[PATCH] D64863: [clangd] Ignore diags from builtin files

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 19 07:37:52 PDT 2019


ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:128
+    return false;
+  Position StartPos = sourceLocToPosition(SM, IncludeInMainFile);
 
----------------
NIT: inline `StartPos`, it has online a single usage now.


================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
     FillDiagBase(*LastDiag);
-    adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
+    if (!InsideMainFile)
+      LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
----------------
We probably want to **always** set the value of this field to avoid accidentally reading the flag for the previous `LastDiag`


================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:608
+      (!LastDiagWasAdjusted ||
+       // Only report the first diagnostic coming from a header.
        IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
----------------
NIT: maybe be more specific? coming from **each particular** header?


================
Comment at: clang-tools-extra/clangd/Diagnostics.h:148
   llvm::Optional<Diag> LastDiag;
+  bool LastDiagWasAdjusted = false;
   llvm::DenseSet<int> IncludeLinesWithErrors;
----------------
NIT: add a comment that it was `adjustDiagFromHeader` that made the adjustment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64863





More information about the cfe-commits mailing list