VitaNuo updated this revision to Diff 508648.
VitaNuo added a comment.

Format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146727

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -33,9 +33,10 @@
   tooling::stdlib::Recognizer Recognizer;
   for (auto *Root : ASTRoots) {
     walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
-      if (!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)))
+      auto FID = SM.getFileID(SM.getSpellingLoc(Loc));
+      if (FID != SM.getMainFileID() && FID != SM.getPreambleFileID())
         return;
-      // FIXME: Most of the work done here is repetative. It might be useful to
+      // FIXME: Most of the work done here is repetitive. It might be useful to
       // have a cache/batching.
       SymbolReference SymRef{Loc, ND, RT};
       return CB(SymRef, headersForSymbol(ND, SM, PI));
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -177,27 +177,39 @@
   WithContextValue Ctx(Config::Key, std::move(Cfg));
   Annotations MainFile(R"cpp(
 #include "a.h"
+#include "all.h"
 $insert_b[[]]#include "baz.h"
 #include "dir/c.h"
-$insert_d[[]]#include "fuzz.h"
+$insert_d[[]]$insert_foo[[]]#include "fuzz.h"
 #include "header.h"
 $insert_foobar[[]]#include <e.h>
 $insert_f[[]]$insert_vector[[]]
 
-    void foo() {
-      $b[[b]]();
+#define DEF(X) const Foo *X;
+#define BAZ(X) const X x
 
-      ns::$bar[[Bar]] bar;
-      bar.d();
-      $f[[f]](); 
+  void foo() {
+    $b[[b]]();
 
-      // this should not be diagnosed, because it's ignored in the config
-      buzz(); 
+    ns::$bar[[Bar]] bar;
+    bar.d();
+    $f[[f]](); 
 
-      $foobar[[foobar]]();
+    // this should not be diagnosed, because it's ignored in the config
+    buzz(); 
 
-      std::$vector[[vector]] v;
-    })cpp");
+    $foobar[[foobar]]();
+
+    std::$vector[[vector]] v;
+
+    int var = $FOO[[FOO]];
+
+    $DEF[[DEF]](a);
+
+    $BAR[[BAR]](b);
+
+    BAZ($Foo[[Foo]]);
+})cpp");
 
   TestTU TU;
   TU.Filename = "foo.cpp";
@@ -224,6 +236,13 @@
   namespace std { class vector {}; }
   )cpp");
 
+  TU.AdditionalFiles["all.h"] = guard("#include \"foo.h\"");
+  TU.AdditionalFiles["foo.h"] = guard(R"cpp(
+    #define BAR(x) Foo *x
+    #define FOO 1
+    struct Foo{}; 
+  )cpp");
+
   TU.Code = MainFile.code();
   ParsedAST AST = TU.build();
 
@@ -253,7 +272,23 @@
               Diag(MainFile.range("vector"),
                    "No header providing \"std::vector\" is directly included"),
               withFix(Fix(MainFile.range("insert_vector"),
-                          "#include <vector>\n", "#include <vector>")))));
+                          "#include <vector>\n", "#include <vector>"))),
+          AllOf(Diag(MainFile.range("FOO"),
+                     "No header providing \"FOO\" is directly included"),
+                withFix(Fix(MainFile.range("insert_foo"),
+                            "#include \"foo.h\"\n", "#include \"foo.h\""))),
+          AllOf(Diag(MainFile.range("DEF"),
+                     "No header providing \"Foo\" is directly included"),
+                withFix(Fix(MainFile.range("insert_foo"),
+                            "#include \"foo.h\"\n", "#include \"foo.h\""))),
+          AllOf(Diag(MainFile.range("BAR"),
+                     "No header providing \"BAR\" is directly included"),
+                withFix(Fix(MainFile.range("insert_foo"),
+                            "#include \"foo.h\"\n", "#include \"foo.h\""))),
+          AllOf(Diag(MainFile.range("Foo"),
+                     "No header providing \"Foo\" is directly included"),
+                withFix(Fix(MainFile.range("insert_foo"),
+                            "#include \"foo.h\"\n", "#include \"foo.h\"")))));
 }
 
 TEST(IncludeCleaner, IWYUPragmas) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -78,7 +78,6 @@
   return Result;
 }
 
-
 bool isFilteredByConfig(const Config &Cfg, llvm::StringRef HeaderPath) {
   // Convert the path to Unix slashes and try to match against the filter.
   llvm::SmallString<64> NormalizedPath(HeaderPath);
@@ -315,7 +314,6 @@
 }
 } // namespace
 
-
 std::vector<const Inclusion *>
 getUnused(ParsedAST &AST,
           const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
@@ -379,15 +377,17 @@
             Ref.RT != include_cleaner::RefType::Explicit)
           return;
 
-        auto &Tokens = AST.getTokens();
-        auto SpelledForExpanded =
-            Tokens.spelledForExpanded(Tokens.expandedTokens(Ref.RefLocation));
-        if (!SpelledForExpanded)
-          return;
-
-        auto Range = syntax::Token::range(SM, SpelledForExpanded->front(),
-                                          SpelledForExpanded->back());
-        MissingIncludeDiagInfo DiagInfo{Ref.Target, Range, Providers};
+        // We actually always want to map usages to their spellings, but
+        // spelling locations can point into preamble section. Using these
+        // offsets could lead into crashes in presence of stale preambles. Hence
+        // we use "getFileLoc" instead to make sure it always points into main
+        // file.
+        // FIXME: Use presumed locations to map such usages back to patched
+        // locations safely.
+        auto Loc = SM.getFileLoc(Ref.RefLocation);
+        const auto *Token = AST.getTokens().spelledTokenAt(Loc);
+        MissingIncludeDiagInfo DiagInfo{Ref.Target, Token->range(SM),
+                                        Providers};
         MissingIncludes.push_back(std::move(DiagInfo));
       });
   std::vector<const Inclusion *> UnusedIncludes =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to