ilya-biryukov added inline comments.

================
Comment at: clangd/ClangdUnit.cpp:380
   if (Preamble)
     Diags.insert(Diags.begin(), Preamble->Diags.begin(), 
Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
----------------
Preamble diagnostics seem to be missing the source.
Could we add a test they also have the "source" set? "unresolved include" 
should be the easiest to get.


================
Comment at: clangd/Diagnostics.h:72
+  // Diagnostic enum ID.
+  unsigned ID;
+  // The source of this diagnostic, e.g. 'clang', 'clang-tidy'.
----------------
jkorous wrote:
> Nit - is this really intended to be part of this patch?
+1, why do we need ID?


================
Comment at: clangd/Diagnostics.h:74
+  // The source of this diagnostic, e.g. 'clang', 'clang-tidy'.
+  std::string Source;
   /// Elaborate on the problem, usually pointing to a related piece of code.
----------------
Let's introduce an enum with two values here and convert to string at the LSP 
boundaries.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58600/new/

https://reviews.llvm.org/D58600



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to