[clang-tools-extra] 002c4b7 - [clangd] Extend CollectMainFileMacros.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 23 04:01:29 PDT 2023
Author: Haojian Wu
Date: 2023-03-23T11:59:11+01:00
New Revision: 002c4b7b955b1fc8825b4d6b46bb079390bce812
URL: https://github.com/llvm/llvm-project/commit/002c4b7b955b1fc8825b4d6b46bb079390bce812
DIFF: https://github.com/llvm/llvm-project/commit/002c4b7b955b1fc8825b4d6b46bb079390bce812.diff
LOG: [clangd] Extend CollectMainFileMacros.
Extend the existing MainFileMacros structure:
- record more information (InConditionalDirective) in MacroOccurrence
- collect macro references inside macro body (fix a long-time FIXME)
So that the MainFileMacros preseve enough information, which allows a
just-in-time convertion to interop with include-cleaner::Macro for
include-cleaer features.
See the context in https://reviews.llvm.org/D146017.
Differential Revision: https://reviews.llvm.org/D146279
Added:
Modified:
clang-tools-extra/clangd/CollectMacros.cpp
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp
index 687f86e0a77eb..c0ed8b68ea481 100644
--- a/clang-tools-extra/clangd/CollectMacros.cpp
+++ b/clang-tools-extra/clangd/CollectMacros.cpp
@@ -9,12 +9,13 @@
#include "CollectMacros.h"
#include "AST.h"
#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
namespace clang {
namespace clangd {
void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,
- bool IsDefinition) {
+ bool IsDefinition, bool InIfCondition) {
if (!InMainFile)
return;
auto Loc = MacroNameTok.getLocation();
@@ -26,9 +27,49 @@ void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,
auto Range = halfOpenToRange(
SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
if (auto SID = getSymbolID(Name, MI, SM))
- Out.MacroRefs[SID].push_back({Range, IsDefinition});
+ Out.MacroRefs[SID].push_back({Range, IsDefinition, InIfCondition});
else
- Out.UnknownMacros.push_back({Range, IsDefinition});
+ Out.UnknownMacros.push_back({Range, IsDefinition, InIfCondition});
+}
+
+void CollectMainFileMacros::FileChanged(SourceLocation Loc, FileChangeReason,
+ SrcMgr::CharacteristicKind, FileID) {
+ InMainFile = isInsideMainFile(Loc, SM);
+}
+void CollectMainFileMacros::MacroExpands(const Token &MacroName,
+ const MacroDefinition &MD,
+ SourceRange Range,
+ const MacroArgs *Args) {
+ add(MacroName, MD.getMacroInfo());
+}
+void CollectMainFileMacros::MacroUndefined(const clang::Token &MacroName,
+ const clang::MacroDefinition &MD,
+ const clang::MacroDirective *Undef) {
+ add(MacroName, MD.getMacroInfo());
+}
+void CollectMainFileMacros::Ifdef(SourceLocation Loc, const Token &MacroName,
+ const MacroDefinition &MD) {
+ add(MacroName, MD.getMacroInfo(), /*IsDefinition=*/false,
+ /*InConditionalDirective=*/true);
+}
+void CollectMainFileMacros::Ifndef(SourceLocation Loc, const Token &MacroName,
+ const MacroDefinition &MD) {
+ add(MacroName, MD.getMacroInfo(), /*IsDefinition=*/false,
+ /*InConditionalDirective=*/true);
+}
+void CollectMainFileMacros::Defined(const Token &MacroName,
+ const MacroDefinition &MD,
+ SourceRange Range) {
+ add(MacroName, MD.getMacroInfo(), /*IsDefinition=*/false,
+ /*InConditionalDirective=*/true);
+}
+void CollectMainFileMacros::SourceRangeSkipped(SourceRange R,
+ SourceLocation EndifLoc) {
+ if (!InMainFile)
+ return;
+ Position Begin = sourceLocToPosition(SM, R.getBegin());
+ Position End = sourceLocToPosition(SM, R.getEnd());
+ Out.SkippedRanges.push_back(Range{Begin, End});
}
class CollectPragmaMarks : public PPCallbacks {
@@ -58,5 +99,24 @@ collectPragmaMarksCallback(const SourceManager &SM,
return std::make_unique<CollectPragmaMarks>(SM, Out);
}
+void CollectMainFileMacros::MacroDefined(const Token &MacroName,
+ const MacroDirective *MD) {
+
+ if (!InMainFile)
+ return;
+ const auto *MI = MD->getMacroInfo();
+ add(MacroName, MD->getMacroInfo(), true);
+ if (MI)
+ for (const auto &Tok : MI->tokens()) {
+ auto *II = Tok.getIdentifierInfo();
+ // Could this token be a reference to a macro? (Not param to this macro).
+ if (!II || !II->hadMacroDefinition() ||
+ llvm::is_contained(MI->params(), II))
+ continue;
+ if (const MacroInfo *MI = PP.getMacroInfo(II))
+ add(Tok, MI);
+ }
+}
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h
index 9d7b478f1c3c7..d5789a2a88912 100644
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -13,6 +13,7 @@
#include "SourceCode.h"
#include "index/SymbolID.h"
#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/DenseMap.h"
#include <string>
@@ -24,6 +25,8 @@ struct MacroOccurrence {
// SourceManager from preamble is not available when we build the AST.
Range Rng;
bool IsDefinition;
+ // True if the occurence is used in a conditional directive, e.g. #ifdef MACRO
+ bool InConditionalDirective;
};
struct MainFileMacros {
@@ -43,56 +46,37 @@ struct MainFileMacros {
/// - collect macros after the preamble of the main file (in ParsedAST.cpp)
class CollectMainFileMacros : public PPCallbacks {
public:
- explicit CollectMainFileMacros(const SourceManager &SM, MainFileMacros &Out)
- : SM(SM), Out(Out) {}
+ explicit CollectMainFileMacros(const Preprocessor &PP, MainFileMacros &Out)
+ : SM(PP.getSourceManager()), PP(PP), Out(Out) {}
void FileChanged(SourceLocation Loc, FileChangeReason,
- SrcMgr::CharacteristicKind, FileID) override {
- InMainFile = isInsideMainFile(Loc, SM);
- }
+ SrcMgr::CharacteristicKind, FileID) override;
- void MacroDefined(const Token &MacroName, const MacroDirective *MD) override {
- add(MacroName, MD->getMacroInfo(), /*IsDefinition=*/true);
- }
+ void MacroDefined(const Token &MacroName, const MacroDirective *MD) override;
void MacroExpands(const Token &MacroName, const MacroDefinition &MD,
- SourceRange Range, const MacroArgs *Args) override {
- add(MacroName, MD.getMacroInfo());
- }
+ SourceRange Range, const MacroArgs *Args) override;
void MacroUndefined(const clang::Token &MacroName,
const clang::MacroDefinition &MD,
- const clang::MacroDirective *Undef) override {
- add(MacroName, MD.getMacroInfo());
- }
+ const clang::MacroDirective *Undef) override;
+ // FIXME: handle C++23 #elifdef, #elifndef
void Ifdef(SourceLocation Loc, const Token &MacroName,
- const MacroDefinition &MD) override {
- add(MacroName, MD.getMacroInfo());
- }
-
+ const MacroDefinition &MD) override;
void Ifndef(SourceLocation Loc, const Token &MacroName,
- const MacroDefinition &MD) override {
- add(MacroName, MD.getMacroInfo());
- }
+ const MacroDefinition &MD) override;
void Defined(const Token &MacroName, const MacroDefinition &MD,
- SourceRange Range) override {
- add(MacroName, MD.getMacroInfo());
- }
-
- void SourceRangeSkipped(SourceRange R, SourceLocation EndifLoc) override {
- if (!InMainFile)
- return;
- Position Begin = sourceLocToPosition(SM, R.getBegin());
- Position End = sourceLocToPosition(SM, R.getEnd());
- Out.SkippedRanges.push_back(Range{Begin, End});
- }
+ SourceRange Range) override;
+
+ void SourceRangeSkipped(SourceRange R, SourceLocation EndifLoc) override;
private:
void add(const Token &MacroNameTok, const MacroInfo *MI,
- bool IsDefinition = false);
+ bool IsDefinition = false, bool InConditionalDirective = false);
const SourceManager &SM;
+ const Preprocessor &PP;
bool InMainFile = true;
MainFileMacros &Out;
};
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 1671eec133b6e..1501a5c5f3c3b 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -610,11 +610,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
Macros = Patch->mainFileMacros();
Marks = Patch->marks();
}
- Clang->getPreprocessor().addPPCallbacks(
- std::make_unique<CollectMainFileMacros>(Clang->getSourceManager(),
- Macros));
+ auto& PP = Clang->getPreprocessor();
+ PP.addPPCallbacks(
+ std::make_unique<CollectMainFileMacros>(
+ PP, Macros));
- Clang->getPreprocessor().addPPCallbacks(
+ PP.addPPCallbacks(
collectPragmaMarksCallback(Clang->getSourceManager(), Marks));
// Copy over the includes from the preamble, then combine with the
@@ -626,10 +627,10 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts());
std::unique_ptr<CommentHandler> IWYUHandler =
collectIWYUHeaderMaps(&CanonIncludes);
- Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
+ PP.addCommentHandler(IWYUHandler.get());
// Collect tokens of the main file.
- syntax::TokenCollector CollectTokens(Clang->getPreprocessor());
+ syntax::TokenCollector CollectTokens(PP);
// To remain consistent with preamble builds, these callbacks must be called
// exactly here, after preprocessor is initialized and BeginSourceFile() was
@@ -660,7 +661,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
// However Action->EndSourceFile() would destroy the ASTContext!
// So just inform the preprocessor of EOF, while keeping everything alive.
- Clang->getPreprocessor().EndSourceFile();
+ PP.EndSourceFile();
// UnitDiagsConsumer is local, we can not store it in CompilerInstance that
// has a longer lifetime.
Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 3b0af0ab50a62..061c67d65f7d8 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -133,6 +133,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
CanonIncludes.addSystemHeadersMapping(CI.getLangOpts());
LangOpts = &CI.getLangOpts();
SourceMgr = &CI.getSourceManager();
+ PP = &CI.getPreprocessor();
Includes.collect(CI);
if (Config::current().Diagnostics.UnusedIncludes ==
Config::IncludesPolicy::Strict ||
@@ -144,11 +145,11 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
}
std::unique_ptr<PPCallbacks> createPPCallbacks() override {
- assert(SourceMgr && LangOpts &&
- "SourceMgr and LangOpts must be set at this point");
+ assert(SourceMgr && LangOpts && PP &&
+ "SourceMgr, LangOpts and PP must be set at this point");
return std::make_unique<PPChainedCallbacks>(
- std::make_unique<CollectMainFileMacros>(*SourceMgr, Macros),
+ std::make_unique<CollectMainFileMacros>(*PP, Macros),
collectPragmaMarksCallback(*SourceMgr, Marks));
}
@@ -215,6 +216,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
std::unique_ptr<CommentHandler> IWYUHandler = nullptr;
const clang::LangOptions *LangOpts = nullptr;
const SourceManager *SourceMgr = nullptr;
+ const Preprocessor *PP = nullptr;
PreambleBuildStats *Stats;
bool ParseForwardingFunctions;
std::function<void(CompilerInstance &)> BeforeExecuteCallback;
@@ -382,7 +384,7 @@ scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) {
PP.addPPCallbacks(
std::make_unique<DirectiveCollector>(PP, SP.TextualDirectives));
PP.addPPCallbacks(collectPragmaMarksCallback(SM, SP.Marks));
- PP.addPPCallbacks(std::make_unique<CollectMainFileMacros>(SM, SP.Macros));
+ PP.addPPCallbacks(std::make_unique<CollectMainFileMacros>(PP, SP.Macros));
if (llvm::Error Err = Action.Execute())
return std::move(Err);
Action.EndSourceFile();
diff --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
index 196ed5cea4693..163a7f1a31707 100644
--- a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -8,12 +8,14 @@
#include "AST.h"
#include "Annotations.h"
#include "CollectMacros.h"
+#include "Matchers.h"
#include "SourceCode.h"
#include "TestTU.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <vector>
namespace clang {
namespace clangd {
@@ -21,19 +23,24 @@ namespace {
using testing::UnorderedElementsAreArray;
+MATCHER_P(rangeIs, R, "") { return arg.Rng == R; }
+MATCHER(isDef, "") { return arg.IsDefinition; }
+MATCHER(inConditionalDirective, "") { return arg.InConditionalDirective; }
+
TEST(CollectMainFileMacros, SelectedMacros) {
// References of the same symbol must have the ranges with the same
// name(integer). If there are N
diff erent symbols then they must be named
// from 1 to N. Macros for which SymbolID cannot be computed must be named
- // "Unknown".
+ // "Unknown". The payload of the annotation describes the extra bit
+ // information of the MacroOccurrence (e.g. $1(def) => IsDefinition).
const char *Tests[] = {
R"cpp(// Macros: Cursor on definition.
- #define $1[[FOO]](x,y) (x + y)
+ #define $1(def)[[FOO]](x,y) (x + y)
int main() { int x = $1[[FOO]]($1[[FOO]](3, 4), $1[[FOO]](5, 6)); }
)cpp",
R"cpp(
- #define $1[[M]](X) X;
- #define $2[[abc]] 123
+ #define $1(def)[[M]](X) X;
+ #define $2(def)[[abc]] 123
int s = $1[[M]]($2[[abc]]);
)cpp",
// FIXME: Locating macro in duplicate definitions doesn't work. Enable
@@ -48,31 +55,50 @@ TEST(CollectMainFileMacros, SelectedMacros) {
// #undef $2[[abc]]
// )cpp",
R"cpp(
- #ifdef $Unknown[[UNDEFINED]]
+ #ifdef $Unknown(condit)[[UNDEFINED]]
+ #endif
+
+ #ifndef $Unknown(condit)[[UNDEFINED]]
+ #endif
+
+ #if defined($Unknown(condit)[[UNDEFINED]])
#endif
)cpp",
R"cpp(
- #ifndef $Unknown[[abc]]
- #define $1[[abc]]
- #ifdef $1[[abc]]
+ #ifndef $Unknown(condit)[[abc]]
+ #define $1(def)[[abc]]
+ #ifdef $1(condit)[[abc]]
#endif
#endif
)cpp",
R"cpp(
// Macros from token concatenations not included.
- #define $1[[CONCAT]](X) X##A()
- #define $2[[PREPEND]](X) MACRO##X()
- #define $3[[MACROA]]() 123
+ #define $1(def)[[CONCAT]](X) X##A()
+ #define $2(def)[[PREPEND]](X) MACRO##X()
+ #define $3(def)[[MACROA]]() 123
int B = $1[[CONCAT]](MACRO);
int D = $2[[PREPEND]](A);
)cpp",
R"cpp(
- // FIXME: Macro names in a definition are not detected.
- #define $1[[MACRO_ARGS2]](X, Y) X Y
- #define $2[[FOO]] BAR
- #define $3[[BAR]] 1
+ #define $1(def)[[MACRO_ARGS2]](X, Y) X Y
+ #define $3(def)[[BAR]] 1
+ #define $2(def)[[FOO]] $3[[BAR]]
int A = $2[[FOO]];
)cpp"};
+ auto ExpectedResults = [](const Annotations &T, StringRef Name) {
+ std::vector<Matcher<MacroOccurrence>> ExpectedLocations;
+ for (const auto &[R, Bits] : T.rangesWithPayload(Name)) {
+ if (Bits == "def")
+ ExpectedLocations.push_back(testing::AllOf(rangeIs(R), isDef()));
+ else if (Bits == "condit")
+ ExpectedLocations.push_back(
+ testing::AllOf(rangeIs(R), inConditionalDirective()));
+ else
+ ExpectedLocations.push_back(testing::AllOf(rangeIs(R)));
+ }
+ return ExpectedLocations;
+ };
+
for (const char *Test : Tests) {
Annotations T(Test);
auto AST = TestTU::withCode(T.code()).build();
@@ -80,13 +106,16 @@ TEST(CollectMainFileMacros, SelectedMacros) {
auto &SM = AST.getSourceManager();
auto &PP = AST.getPreprocessor();
- // Known macros.
- for (int I = 1;; I++) {
- const auto ExpectedRefs = T.ranges(llvm::to_string(I));
- if (ExpectedRefs.empty())
- break;
+ for (const auto &[Name, Ranges] : T.all_ranges()) {
+ if (Name == "Unknown") {
+ EXPECT_THAT(ActualMacroRefs.UnknownMacros,
+ UnorderedElementsAreArray(ExpectedResults(T, "Unknown")))
+ << "Unknown macros doesn't match in " << Test;
+ continue;
+ }
- auto Loc = sourceLocationInMainFile(SM, ExpectedRefs.begin()->start);
+ auto Loc = sourceLocationInMainFile(
+ SM, offsetToPosition(T.code(), Ranges.front().Begin));
ASSERT_TRUE(bool(Loc));
const auto *Id = syntax::spelledIdentifierTouching(*Loc, AST.getTokens());
ASSERT_TRUE(Id);
@@ -94,19 +123,11 @@ TEST(CollectMainFileMacros, SelectedMacros) {
assert(Macro);
auto SID = getSymbolID(Macro->Name, Macro->Info, SM);
- std::vector<Range> Ranges;
- for (const auto &Ref : ActualMacroRefs.MacroRefs[SID])
- Ranges.push_back(Ref.Rng);
- EXPECT_THAT(ExpectedRefs, UnorderedElementsAreArray(Ranges))
- << "Annotation=" << I << ", MacroName=" << Macro->Name
+ EXPECT_THAT(ActualMacroRefs.MacroRefs[SID],
+ UnorderedElementsAreArray(ExpectedResults(T, Name)))
+ << "Annotation=" << Name << ", MacroName=" << Macro->Name
<< ", Test = " << Test;
}
- // Unknown macros.
- std::vector<Range> Ranges;
- for (const auto &Ref : AST.getMacros().UnknownMacros)
- Ranges.push_back(Ref.Rng);
- EXPECT_THAT(Ranges, UnorderedElementsAreArray(T.ranges("Unknown")))
- << "Unknown macros doesn't match in " << Test;
}
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 259efcf54a6b2..975378118b7ad 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -399,7 +399,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
#define $Macro_decl[[MACRO_CONCAT]](X, V, T) T foo##X = V
#define $Macro_decl[[DEF_VAR]](X, V) int X = V
#define $Macro_decl[[DEF_VAR_T]](T, X, V) T X = V
- #define $Macro_decl[[DEF_VAR_REV]](V, X) DEF_VAR(X, V)
+ #define $Macro_decl[[DEF_VAR_REV]](V, X) $Macro[[DEF_VAR]](X, V)
#define $Macro_decl[[CPY]](X) X
#define $Macro_decl[[DEF_VAR_TYPE]](X, Y) X Y
#define $Macro_decl[[SOME_NAME]] variable
@@ -431,7 +431,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
)cpp",
R"cpp(
#define $Macro_decl[[fail]](expr) expr
- #define $Macro_decl[[assert]](COND) if (!(COND)) { fail("assertion failed" #COND); }
+ #define $Macro_decl[[assert]](COND) if (!(COND)) { $Macro[[fail]]("assertion failed" #COND); }
// Preamble ends.
int $Variable_def[[x]];
int $Variable_def[[y]];
More information about the cfe-commits
mailing list