diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/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); @@ -390,15 +389,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 UnusedIncludes = diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -178,27 +178,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 $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"; @@ -225,6 +237,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(); @@ -254,7 +273,23 @@ Diag(MainFile.range("vector"), "No header providing \"std::vector\" is directly included"), withFix(Fix(MainFile.range("insert_vector"), - "#include \n", "#include "))))); + "#include \n", "#include "))), + 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) { diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -36,9 +36,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));