arphaman added a comment. In https://reviews.llvm.org/D50814#1224292, @ilya-biryukov wrote:
> Sorry for the delayed response. It seems we absolutely need this if mirroring > libclang is an absolute requirement. > We seem to have multiple implementation options: > > Which classes to use for representing diagnostics? We can: > > 1. Reuse existing hierarchy for diagnostics. (current option) > 2. Have separate hierarchies for "flat" (fixits with notes) and "grouped" > (fix-its are always attached to main diag). "flat" diagnostics can be stored > exactly as they come from clang, with notes and main diags in the same > vector, main diag should lack the "notes" field. I.e. if we choose to go with > "raw" clang mode, we can as well avoid grouping the notes altogether instead > of having code that does something in between the classical libclang and new > clangd approach. > > I think the added separation is worth it for code clarity, but also realize > it's a bunch of boilerplate, so others may disagree. WDYT? I think having separate hierarchies would be more confusing and harder to test well. I'd prefer to use one hierarchy. > Another alternative that we might explore is always producing fix-its > attached to notes at the lowest level (in `Diagnostics.cpp`) and > reconstructing the current diagnostics hierarchy somewhere higher up the > chain, e.g. at the LSPServer level. > This would allow to keep the option at the LSPServer level and would only > require extracting a (seemingly simple) conversion function that can be > called at the LSP Server level. > Having the option handled at the top-level seems like a win, since most of > the code does not need to care about this option.\ That would be my personal preference. I will work on an updated patch that does that. > +@sammccall, who wanted to take a look at this issue. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50814 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
