[clang-tools-extra] ec6c344 - [include-cleaner] Respect langopts when analyzing macro names (#123634)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 21 05:48:11 PST 2025
Author: kadir çetinkaya
Date: 2025-01-21T14:48:08+01:00
New Revision: ec6c3448d31056db5d63d7aed3e9f207edb49321
URL: https://github.com/llvm/llvm-project/commit/ec6c3448d31056db5d63d7aed3e9f207edb49321
DIFF: https://github.com/llvm/llvm-project/commit/ec6c3448d31056db5d63d7aed3e9f207edb49321.diff
LOG: [include-cleaner] Respect langopts when analyzing macro names (#123634)
Fixes https://github.com/llvm/llvm-project/issues/113926.
Fixes https://github.com/llvm/llvm-project/issues/63976.
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 5e136d0e76ece7..3ab3d890305204 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -1193,12 +1193,13 @@ void maybeAddSymbolProviders(ParsedAST &AST, HoverInfo &HI,
include_cleaner::Symbol Sym) {
trace::Span Tracer("Hover::maybeAddSymbolProviders");
- const SourceManager &SM = AST.getSourceManager();
llvm::SmallVector<include_cleaner::Header> RankedProviders =
- include_cleaner::headersForSymbol(Sym, SM, &AST.getPragmaIncludes());
+ include_cleaner::headersForSymbol(Sym, AST.getPreprocessor(),
+ &AST.getPragmaIncludes());
if (RankedProviders.empty())
return;
+ const SourceManager &SM = AST.getSourceManager();
std::string Result;
include_cleaner::Includes ConvertedIncludes = convertIncludes(AST);
for (const auto &P : RankedProviders) {
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 6d0af20e31260c..1de7faf81746ee 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -888,7 +888,7 @@ void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation DefLoc,
// might run while parsing, rather than at the end of a translation unit.
// Hence we see more and more redecls over time.
SymbolProviders[S.ID] =
- include_cleaner::headersForSymbol(Sym, SM, Opts.PragmaIncludes);
+ include_cleaner::headersForSymbol(Sym, *PP, Opts.PragmaIncludes);
}
llvm::StringRef getStdHeader(const Symbol *S, const LangOptions &LangOpts) {
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index 46ca3c9d080740..c3241763237d14 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -90,7 +90,7 @@ std::string fixIncludes(const AnalysisResults &Results,
/// Returned headers are sorted by relevance, first element is the most
/// likely provider for the symbol.
llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
- const SourceManager &SM,
+ const Preprocessor &PP,
const PragmaIncludes *PI);
} // namespace include_cleaner
} // namespace clang
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index e3a4834cb19aeb..a1781f4e24f2e8 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -64,7 +64,7 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
// FIXME: Most of the work done here is repetitive. It might be useful to
// have a cache/batching.
SymbolReference SymRef{ND, Loc, RT};
- return CB(SymRef, headersForSymbol(ND, SM, PI));
+ return CB(SymRef, headersForSymbol(ND, PP, PI));
});
}
for (const SymbolReference &MacroRef : MacroRefs) {
@@ -72,7 +72,7 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)) ||
shouldIgnoreMacroReference(PP, MacroRef.Target.macro()))
continue;
- CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI));
+ CB(MacroRef, headersForSymbol(MacroRef.Target, PP, PI));
}
}
diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
index cd796c2da7b805..7d170fd15014d2 100644
--- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
@@ -25,6 +25,8 @@
#include "clang-include-cleaner/Analysis.h"
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include <vector>
@@ -57,13 +59,14 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
const PragmaIncludes *PI);
/// A set of locations that provides the declaration.
-std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S);
+std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S,
+ const LangOptions &LO);
/// Write an HTML summary of the analysis to the given stream.
void writeHTMLReport(FileID File, const Includes &,
llvm::ArrayRef<Decl *> Roots,
llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx,
- const HeaderSearch &HS, PragmaIncludes *PI,
+ const Preprocessor &PP, PragmaIncludes *PI,
llvm::raw_ostream &OS);
} // namespace include_cleaner
diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index 7b28d1c252d715..b96d9a70728c23 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,6 +18,7 @@
#include "clang/Basic/FileEntry.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
@@ -239,8 +240,9 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
}
llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
- const SourceManager &SM,
+ const Preprocessor &PP,
const PragmaIncludes *PI) {
+ const auto &SM = PP.getSourceManager();
// Get headers for all the locations providing Symbol. Same header can be
// reached through
diff erent traversals, deduplicate those into a single
// Header by merging their hints.
@@ -248,7 +250,7 @@ llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
if (auto SpecialHeaders = headersForSpecialSymbol(S, SM, PI)) {
Headers = std::move(*SpecialHeaders);
} else {
- for (auto &Loc : locateSymbol(S))
+ for (auto &Loc : locateSymbol(S, PP.getLangOpts()))
Headers.append(applyHints(findHeaders(Loc, SM, PI), Loc.Hint));
}
// If two Headers probably refer to the same file (e.g. Verbatim(foo.h) and
diff --git a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
index bbe8bc230c6e20..92c7c554ca50c9 100644
--- a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -21,6 +21,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/raw_ostream.h"
@@ -135,7 +136,7 @@ class Reporter {
llvm::raw_ostream &OS;
const ASTContext &Ctx;
const SourceManager &SM;
- const HeaderSearch &HS;
+ const Preprocessor &PP;
const include_cleaner::Includes &Includes;
const PragmaIncludes *PI;
FileID MainFile;
@@ -170,9 +171,9 @@ class Reporter {
void fillTarget(Ref &R) {
// Duplicates logic from walkUsed(), which doesn't expose SymbolLocations.
- for (auto &Loc : locateSymbol(R.Sym))
+ for (auto &Loc : locateSymbol(R.Sym, Ctx.getLangOpts()))
R.Locations.push_back(Loc);
- R.Headers = headersForSymbol(R.Sym, SM, PI);
+ R.Headers = headersForSymbol(R.Sym, PP, PI);
for (const auto &H : R.Headers) {
R.Includes.append(Includes.match(H));
@@ -189,14 +190,15 @@ class Reporter {
R.Includes.end());
if (!R.Headers.empty())
- R.Insert = spellHeader({R.Headers.front(), HS, MainFE});
+ R.Insert =
+ spellHeader({R.Headers.front(), PP.getHeaderSearchInfo(), MainFE});
}
public:
- Reporter(llvm::raw_ostream &OS, ASTContext &Ctx, const HeaderSearch &HS,
+ Reporter(llvm::raw_ostream &OS, ASTContext &Ctx, const Preprocessor &PP,
const include_cleaner::Includes &Includes, const PragmaIncludes *PI,
FileID MainFile)
- : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), HS(HS),
+ : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), PP(PP),
Includes(Includes), PI(PI), MainFile(MainFile),
MainFE(SM.getFileEntryForID(MainFile)) {}
@@ -498,9 +500,9 @@ class Reporter {
void writeHTMLReport(FileID File, const include_cleaner::Includes &Includes,
llvm::ArrayRef<Decl *> Roots,
llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx,
- const HeaderSearch &HS, PragmaIncludes *PI,
+ const Preprocessor &PP, PragmaIncludes *PI,
llvm::raw_ostream &OS) {
- Reporter R(OS, Ctx, HS, Includes, PI, File);
+ Reporter R(OS, Ctx, PP, Includes, PI, File);
const auto& SM = Ctx.getSourceManager();
for (Decl *Root : Roots)
walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
diff --git a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
index 78e783a62eb27f..b7433305152f95 100644
--- a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
+++ b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
@@ -54,20 +54,24 @@ std::vector<Hinted<SymbolLocation>> locateDecl(const Decl &D) {
return Result;
}
-std::vector<Hinted<SymbolLocation>> locateMacro(const Macro &M) {
+std::vector<Hinted<SymbolLocation>> locateMacro(const Macro &M,
+ const tooling::stdlib::Lang L) {
// FIXME: Should we also provide physical locations?
- if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName()))
+ if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName(), L))
return {{*SS, Hints::CompleteSymbol}};
return {{M.Definition, Hints::CompleteSymbol}};
}
} // namespace
-std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S) {
+std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S,
+ const LangOptions &LO) {
+ const auto L = !LO.CPlusPlus && LO.C99 ? tooling::stdlib::Lang::C
+ : tooling::stdlib::Lang::CXX;
switch (S.kind()) {
case Symbol::Declaration:
return locateDecl(S.declaration());
case Symbol::Macro:
- return locateMacro(S.macro());
+ return locateMacro(S.macro(), L);
}
llvm_unreachable("Unknown Symbol::Kind enum");
}
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index f85dbc0e0c31f2..1d9458ffc4d327 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -216,10 +216,9 @@ class Action : public clang::ASTFrontendAction {
++Errors;
return;
}
- writeHTMLReport(
- AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
- PP.MacroReferences, *AST.Ctx,
- getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), &PI, OS);
+ writeHTMLReport(AST.Ctx->getSourceManager().getMainFileID(), PP.Includes,
+ AST.Roots, PP.MacroReferences, *AST.Ctx,
+ getCompilerInstance().getPreprocessor(), &PI, OS);
}
};
class ActionFactory : public tooling::FrontendActionFactory {
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index 84e02e1d0d621b..0ac243937e6e4f 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -306,7 +306,7 @@ class HeadersForSymbolTest : public FindHeadersTest {
if (!V.Out)
ADD_FAILURE() << "Couldn't find any decls named " << Name << ".";
assert(V.Out);
- return headersForSymbol(*V.Out, AST->sourceManager(), &PI);
+ return headersForSymbol(*V.Out, AST->preprocessor(), &PI);
}
llvm::SmallVector<Header> headersForFoo() { return headersFor("foo"); }
};
@@ -611,13 +611,12 @@ TEST_F(HeadersForSymbolTest, AmbiguousStdSymbolsUsingShadow) {
Visitor V;
V.TraverseDecl(AST->context().getTranslationUnitDecl());
ASSERT_TRUE(V.Out) << "Couldn't find a DeclRefExpr!";
- EXPECT_THAT(headersForSymbol(*(V.Out->getFoundDecl()),
- AST->sourceManager(), &PI),
- UnorderedElementsAre(
- Header(*tooling::stdlib::Header::named("<cstdio>"))));
+ EXPECT_THAT(
+ headersForSymbol(*(V.Out->getFoundDecl()), AST->preprocessor(), &PI),
+ UnorderedElementsAre(
+ Header(*tooling::stdlib::Header::named("<cstdio>"))));
}
-
TEST_F(HeadersForSymbolTest, StandardHeaders) {
Inputs.Code = R"cpp(
#include "stdlib_internal.h"
@@ -636,6 +635,30 @@ TEST_F(HeadersForSymbolTest, StandardHeaders) {
tooling::stdlib::Header::named("<assert.h>")));
}
+TEST_F(HeadersForSymbolTest, StdlibLangForMacros) {
+ Inputs.Code = R"cpp(
+ #define EOF 0
+ void foo() { EOF; }
+ )cpp";
+ {
+ buildAST();
+ const Macro Eof{AST->preprocessor().getIdentifierInfo("EOF"), {}};
+ EXPECT_THAT(
+ headersForSymbol(Eof, AST->preprocessor(), nullptr),
+ UnorderedElementsAre(tooling::stdlib::Header::named("<cstdio>"),
+ tooling::stdlib::Header::named("<stdio.h>")));
+ }
+
+ {
+ Inputs.ExtraArgs.push_back("-xc");
+ buildAST();
+ const Macro Eof{AST->preprocessor().getIdentifierInfo("EOF"), {}};
+ EXPECT_THAT(headersForSymbol(Eof, AST->preprocessor(), nullptr),
+ UnorderedElementsAre(tooling::stdlib::Header::named(
+ "<stdio.h>", tooling::stdlib::Lang::C)));
+ }
+}
+
TEST_F(HeadersForSymbolTest, ExporterNoNameMatch) {
Inputs.Code = R"cpp(
#include "exporter/foo.h"
diff --git a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
index 756757cfd0f09d..1e7baf142a75ac 100644
--- a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Testing/TestAST.h"
@@ -96,6 +97,8 @@ struct LocateExample {
Results.emplace_back(SM.getComposedLoc(FID, Offset));
return Results;
}
+
+ const LangOptions &langOpts() { return AST.preprocessor().getLangOpts(); }
};
TEST(LocateSymbol, Decl) {
@@ -110,7 +113,7 @@ TEST(LocateSymbol, Decl) {
for (auto &Case : Cases) {
SCOPED_TRACE(Case);
LocateExample Test(Case);
- EXPECT_THAT(locateSymbol(Test.findDecl("foo")),
+ EXPECT_THAT(locateSymbol(Test.findDecl("foo"), Test.langOpts()),
ElementsAreArray(Test.points()));
}
}
@@ -119,12 +122,12 @@ TEST(LocateSymbol, Stdlib) {
{
LocateExample Test("namespace std { struct vector; }");
EXPECT_THAT(
- locateSymbol(Test.findDecl("vector")),
+ locateSymbol(Test.findDecl("vector"), Test.langOpts()),
ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector")));
}
{
LocateExample Test("#define assert(x)\nvoid foo() { assert(true); }");
- EXPECT_THAT(locateSymbol(Test.findMacro("assert")),
+ EXPECT_THAT(locateSymbol(Test.findMacro("assert"), Test.langOpts()),
ElementsAre(*tooling::stdlib::Symbol::named("", "assert")));
}
}
@@ -132,7 +135,7 @@ TEST(LocateSymbol, Stdlib) {
TEST(LocateSymbol, Macros) {
// Make sure we preserve the last one.
LocateExample Test("#define FOO\n#undef FOO\n#define ^FOO");
- EXPECT_THAT(locateSymbol(Test.findMacro("FOO")),
+ EXPECT_THAT(locateSymbol(Test.findMacro("FOO"), Test.langOpts()),
ElementsAreArray(Test.points()));
}
@@ -143,7 +146,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
{
// stdlib symbols are always complete.
LocateExample Test("namespace std { struct vector; }");
- EXPECT_THAT(locateSymbol(Test.findDecl("vector")),
+ EXPECT_THAT(locateSymbol(Test.findDecl("vector"), Test.langOpts()),
ElementsAre(HintedSymbol(
*tooling::stdlib::Symbol::named("std::", "vector"),
Hints::CompleteSymbol)));
@@ -151,7 +154,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
{
// macros are always complete.
LocateExample Test("#define ^FOO");
- EXPECT_THAT(locateSymbol(Test.findMacro("FOO")),
+ EXPECT_THAT(locateSymbol(Test.findMacro("FOO"), Test.langOpts()),
ElementsAre(HintedSymbol(Test.points().front(),
Hints::CompleteSymbol)));
}
@@ -165,7 +168,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
for (auto &Case : Cases) {
SCOPED_TRACE(Case);
LocateExample Test(Case);
- EXPECT_THAT(locateSymbol(Test.findDecl("foo")),
+ EXPECT_THAT(locateSymbol(Test.findDecl("foo"), Test.langOpts()),
ElementsAre(HintedSymbol(Test.points().front(), Hints::None),
HintedSymbol(Test.points().back(),
Hints::CompleteSymbol)));
@@ -181,7 +184,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
for (auto &Case : Cases) {
SCOPED_TRACE(Case);
LocateExample Test(Case);
- EXPECT_THAT(locateSymbol(Test.findDecl("foo")),
+ EXPECT_THAT(locateSymbol(Test.findDecl("foo"), Test.langOpts()),
Each(Field(&Hinted<SymbolLocation>::Hint,
Eq(Hints::CompleteSymbol))));
}
More information about the cfe-commits
mailing list