bnbarham added a subscriber: yaxunl. bnbarham added a comment. > ! In D108268#2961735 <https://reviews.llvm.org/D108268#2961735>, @vsapsai > wrote: > This might be a stupid idea and a bridge too far but what if delayed > diagnostic was storing `PartialDiagnostic` and not three strings? This looks > like a better API but I haven't tried it myself and concerned we might not > have diag allocator in all required places.
It seems like it should be possible to not have the "only one in-flight diagnostic" restriction at all. Removing that would be nice since 1. we could just use DiagnosticError and 2. there would be no need to check for an existing diagnostic. @yaxunl I see you did a bunch of work extracting out the DiagnosticStorage and StreamingDiagnostic classes. Any thoughts on this? I feel like I'm missing something obvious here. > Another idea is to replace DiagnosedError with something like > ThreeStringError (please don't use this name). I can be wrong but I find it > easier to understand an error representing diagnostics compared to a marker > that diagnostic was emitted-or-scheduled earlier. There's only a few places that call `SetDelayedDiagnostic`, but none of them would have an allocator (ie. `DiagnosticIDs` and `ContentCache`). I may fallback to this instead (could just call it `ASTReadError`/something similar). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108268/new/ https://reviews.llvm.org/D108268 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits