diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h --- 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 @@ -24,6 +25,8 @@ // 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 @@ /// - 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/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp --- 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 @@ 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 @@ return std::make_unique(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/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -610,11 +610,12 @@ Macros = Patch->mainFileMacros(); Marks = Patch->marks(); } - Clang->getPreprocessor().addPPCallbacks( - std::make_unique(Clang->getSourceManager(), - Macros)); + auto& PP = Clang->getPreprocessor(); + PP.addPPCallbacks( + std::make_unique( + 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 @@ CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts()); std::unique_ptr 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 @@ // 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 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -133,6 +133,7 @@ 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 @@ } std::unique_ptr 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( - std::make_unique(*SourceMgr, Macros), + std::make_unique(*PP, Macros), collectPragmaMarksCallback(*SourceMgr, Marks)); } @@ -215,6 +216,7 @@ std::unique_ptr IWYUHandler = nullptr; const clang::LangOptions *LangOpts = nullptr; const SourceManager *SourceMgr = nullptr; + const Preprocessor *PP = nullptr; PreambleBuildStats *Stats; bool ParseForwardingFunctions; std::function BeforeExecuteCallback; @@ -382,7 +384,7 @@ PP.addPPCallbacks( std::make_unique(PP, SP.TextualDirectives)); PP.addPPCallbacks(collectPragmaMarksCallback(SM, SP.Marks)); - PP.addPPCallbacks(std::make_unique(SM, SP.Macros)); + PP.addPPCallbacks(std::make_unique(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 --- 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 namespace clang { namespace clangd { @@ -21,19 +23,24 @@ 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 different 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 @@ // #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> 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 @@ 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 @@ assert(Macro); auto SID = getSymbolID(Macro->Name, Macro->Info, SM); - std::vector 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 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 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -399,7 +399,7 @@ #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 @@ )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]];