[clang-tools-extra] 481f888 - [clangd] Use expansion location for missing include diagnostics.
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 27 07:27:39 PDT 2023
Author: Viktoriia Bakalova
Date: 2023-03-27T14:26:12Z
New Revision: 481f88853685bcf604c79059e890553831588e8b
URL: https://github.com/llvm/llvm-project/commit/481f88853685bcf604c79059e890553831588e8b
DIFF: https://github.com/llvm/llvm-project/commit/481f88853685bcf604c79059e890553831588e8b.diff
LOG: [clangd] Use expansion location for missing include diagnostics.
Differential Revision: https://reviews.llvm.org/D146727
Added:
Modified:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/include-cleaner/lib/Analysis.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index ab7c05eb834c0..8a433b0fe88d7 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -78,7 +78,6 @@ clangd::Range getDiagnosticRange(llvm::StringRef Code, unsigned HashOffset) {
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 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
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 =
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 69b4e07439c38..de45da02bfe87 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -178,27 +178,39 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
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";
@@ -225,6 +237,13 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
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 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
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) {
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index 58402cce0b717..84f1f4cc2cf54 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -36,9 +36,10 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
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));
More information about the cfe-commits
mailing list