VitaNuo updated this revision to Diff 508645.
VitaNuo marked 3 inline comments as done.
VitaNuo added a comment.
Address review comments.
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);
@@ -379,15 +378,16 @@
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
