[PATCH] D58600: [clangd] Emit source to Diagnostic.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 5 01:38:27 PST 2019


hokein added a comment.

add @kadircet as a reviewer, since @ilya-biryukov is OOO.



================
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),
----------------
ilya-biryukov wrote:
> 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.
good catch, I missed this.


================
Comment at: clangd/Diagnostics.cpp:381
     LastDiag = Diag();
+    LastDiag->ID = Info.getID();
     FillDiagBase(*LastDiag);
----------------
jkorous wrote:
> Nit - is this really intended to be part of this patch?
See my comment above.


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


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58600





More information about the cfe-commits mailing list