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]]; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits