From 7e3c74bc63f97dd8fa354e6f2153ab8b3d88082a Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Tue, 24 Sep 2019 11:14:06 +0000 Subject: [PATCH] [clangd] Collect macros in the preamble region of the main file Summary: - store all macro references in the ParsedAST; - unify the two variants of CollectMainFileMacros; Reviewers: ilya-biryukov Subscribers: MaskRay, jkorous, mgrang, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D67496 llvm-svn: 372725 --- clang-tools-extra/clangd/CodeComplete.cpp | 4 +- clang-tools-extra/clangd/CollectMacros.h | 74 ++++++++++++++++++++++ clang-tools-extra/clangd/ParsedAST.cpp | 52 ++++----------- clang-tools-extra/clangd/ParsedAST.h | 20 +++--- clang-tools-extra/clangd/Preamble.cpp | 57 ++++------------- clang-tools-extra/clangd/Preamble.h | 6 +- clang-tools-extra/clangd/SemanticHighlighting.cpp | 5 +- .../clangd/unittests/ParsedASTTests.cpp | 13 ++-- .../clangd/unittests/SemanticHighlightingTests.cpp | 8 +-- 9 files changed, 126 insertions(+), 113 deletions(-) create mode 100644 clang-tools-extra/clangd/CollectMacros.h diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 2b6c3b4..bc826ba 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1030,8 +1030,8 @@ void loadMainFilePreambleMacros(const Preprocessor &PP, PP.getIdentifierTable().getExternalIdentifierLookup(); if (!PreambleIdentifiers || !PreambleMacros) return; - for (const auto &MacroName : Preamble.MainFileMacros) - if (auto *II = PreambleIdentifiers->get(MacroName)) + for (const auto &MacroName : Preamble.Macros.Names) + if (auto *II = PreambleIdentifiers->get(MacroName.getKey())) if (II->isOutOfDate()) PreambleMacros->updateOutOfDateIdentifier(*II); } diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h new file mode 100644 index 0000000..21227e1 --- /dev/null +++ b/clang-tools-extra/clangd/CollectMacros.h @@ -0,0 +1,74 @@ +//===--- CollectMacros.h -----------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H + +#include "Protocol.h" +#include "SourceCode.h" +#include "clang/Basic/IdentifierTable.h" +#include "clang/Lex/PPCallbacks.h" +#include + +namespace clang { +namespace clangd { + +struct MainFileMacros { + llvm::StringSet<> Names; + // Instead of storing SourceLocation, we have to store the token range because + // SourceManager from preamble is not available when we build the AST. + std::vector Ranges; +}; + +/// Collects macro definitions and expansions in the main file. It is used to: +/// - collect macros in the preamble section of the main file (in Preamble.cpp) +/// - collect macros after the preamble of the main file (in ParsedAST.cpp) +class CollectMainFileMacros : public PPCallbacks { +public: + explicit CollectMainFileMacros(const SourceManager &SM, + const LangOptions &LangOpts, + MainFileMacros &Out) + : SM(SM), LangOpts(LangOpts), Out(Out) {} + + void FileChanged(SourceLocation Loc, FileChangeReason, + SrcMgr::CharacteristicKind, FileID) override { + InMainFile = isInsideMainFile(Loc, SM); + } + + void MacroDefined(const Token &MacroName, const MacroDirective *MD) override { + add(MacroName, MD->getMacroInfo()); + } + + void MacroExpands(const Token &MacroName, const MacroDefinition &MD, + SourceRange Range, const MacroArgs *Args) override { + add(MacroName, MD.getMacroInfo()); + } + +private: + void add(const Token &MacroNameTok, const MacroInfo *MI) { + if (!InMainFile) + return; + auto Loc = MacroNameTok.getLocation(); + if (Loc.isMacroID()) + return; + + if (auto Range = getTokenRange(SM, LangOpts, MacroNameTok.getLocation())) { + Out.Names.insert(MacroNameTok.getIdentifierInfo()->getName()); + Out.Ranges.push_back(*Range); + } + } + const SourceManager &SM; + const LangOptions &LangOpts; + bool InMainFile = true; + MainFileMacros &Out; +}; + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 3619144..370af1d 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -98,33 +98,6 @@ private: std::vector TopLevelDecls; }; -// This collects macro expansions/definitions in the main file. -// (Contrast with CollectMainFileMacros in Preamble.cpp, which collects macro -// *definitions* in the preamble region of the main file). -class CollectMainFileMacros : public PPCallbacks { - const SourceManager &SM; - std::vector &MainFileMacroLocs; - - void addLoc(SourceLocation Loc) { - if (!Loc.isMacroID() && isInsideMainFile(Loc, SM)) - MainFileMacroLocs.push_back(Loc); - } - -public: - CollectMainFileMacros(const SourceManager &SM, - std::vector &MainFileMacroLocs) - : SM(SM), MainFileMacroLocs(MainFileMacroLocs) {} - - void MacroDefined(const Token &MacroNameTok, - const MacroDirective *MD) override { - addLoc(MacroNameTok.getLocation()); - } - void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, - SourceRange Range, const MacroArgs *Args) override { - addLoc(MacroNameTok.getLocation()); - } -}; - // When using a preamble, only preprocessor events outside its bounds are seen. // This is almost what we want: replaying transitive preprocessing wastes time. // However this confuses clang-tidy checks: they don't see any #includes! @@ -362,11 +335,14 @@ ParsedAST::build(std::unique_ptr CI, // (We can't *just* use the replayed includes, they don't have Resolved path). Clang->getPreprocessor().addPPCallbacks( collectIncludeStructureCallback(Clang->getSourceManager(), &Includes)); - // Collect the macro expansions in the main file. - std::vector MainFileMacroExpLocs; + // Copy over the macros in the preamble region of the main file, and combine + // with non-preamble macros below. + MainFileMacros Macros; + if (Preamble) + Macros = Preamble->Macros; Clang->getPreprocessor().addPPCallbacks( std::make_unique(Clang->getSourceManager(), - MainFileMacroExpLocs)); + Clang->getLangOpts(), Macros)); // Copy over the includes from the preamble, then combine with the // non-preamble includes below. @@ -420,9 +396,9 @@ ParsedAST::build(std::unique_ptr CI, Diags.insert(Diags.end(), D.begin(), D.end()); } return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action), - std::move(Tokens), std::move(MainFileMacroExpLocs), - std::move(ParsedDecls), std::move(Diags), - std::move(Includes), std::move(CanonIncludes)); + std::move(Tokens), std::move(Macros), std::move(ParsedDecls), + std::move(Diags), std::move(Includes), + std::move(CanonIncludes)); } ParsedAST::ParsedAST(ParsedAST &&Other) = default; @@ -460,9 +436,7 @@ llvm::ArrayRef ParsedAST::getLocalTopLevelDecls() { return LocalTopLevelDecls; } -llvm::ArrayRef ParsedAST::getMacros() const { - return MacroIdentifierLocs; -} +const MainFileMacros &ParsedAST::getMacros() const { return Macros; } const std::vector &ParsedAST::getDiagnostics() const { return Diags; } @@ -509,15 +483,13 @@ const CanonicalIncludes &ParsedAST::getCanonicalIncludes() const { ParsedAST::ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, - syntax::TokenBuffer Tokens, - std::vector MacroIdentifierLocs, + syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector LocalTopLevelDecls, std::vector Diags, IncludeStructure Includes, CanonicalIncludes CanonIncludes) : Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Tokens(std::move(Tokens)), - MacroIdentifierLocs(std::move(MacroIdentifierLocs)), - Diags(std::move(Diags)), + Macros(std::move(Macros)), Diags(std::move(Diags)), LocalTopLevelDecls(std::move(LocalTopLevelDecls)), Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) { assert(this->Clang); diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index f37e14d..0b4a6ab 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -21,6 +21,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PARSEDAST_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PARSEDAST_H +#include "CollectMacros.h" #include "Compiler.h" #include "Diagnostics.h" #include "Headers.h" @@ -89,10 +90,9 @@ public: const IncludeStructure &getIncludeStructure() const; const CanonicalIncludes &getCanonicalIncludes() const; - /// Gets all macro locations (definition, expansions) present in the main - /// file. - /// NOTE: macros inside the preamble are not included. - llvm::ArrayRef getMacros() const; + /// Gets all macro references (definition, expansions) present in the main + /// file, including those in the preamble region. + const MainFileMacros &getMacros() const; /// Tokens recorded while parsing the main file. /// (!) does not have tokens from the preamble. const syntax::TokenBuffer &getTokens() const { return Tokens; } @@ -101,9 +101,9 @@ private: ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, syntax::TokenBuffer Tokens, - std::vector MainFileMacroExpLocs, - std::vector LocalTopLevelDecls, std::vector Diags, - IncludeStructure Includes, CanonicalIncludes CanonIncludes); + MainFileMacros Macros, std::vector LocalTopLevelDecls, + std::vector Diags, IncludeStructure Includes, + CanonicalIncludes CanonIncludes); // In-memory preambles must outlive the AST, it is important that this member // goes before Clang and Action. @@ -121,10 +121,8 @@ private: /// - Does not have spelled or expanded tokens for files from preamble. syntax::TokenBuffer Tokens; - /// The start locations of all macro definitions/expansions spelled **after** - /// preamble. - /// Does not include locations from inside other macro expansions. - std::vector MacroIdentifierLocs; + /// All macro definitions and expansions in the main file. + MainFileMacros Macros; // Data, stored after parsing. std::vector Diags; // Top-level decls inside the current file. Not that this does not include diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index d68c1ec..41e3c136 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -24,49 +24,14 @@ bool compileCommandsAreEqual(const tooling::CompileCommand &LHS, llvm::makeArrayRef(LHS.CommandLine).equals(RHS.CommandLine); } -// This collects macro definitions in the *preamble region* of the main file. -// (Contrast with CollectMainFileMacroExpansions in ParsedAST.cpp, which -// collects macro *expansions* in the rest of the main file. -class CollectMainFileMacros : public PPCallbacks { -public: - explicit CollectMainFileMacros(const SourceManager &SM, - std::vector *Out) - : SM(SM), Out(Out) {} - - void FileChanged(SourceLocation Loc, FileChangeReason, - SrcMgr::CharacteristicKind, FileID Prev) { - InMainFile = SM.isWrittenInMainFile(Loc); - } - - void MacroDefined(const Token &MacroName, const MacroDirective *MD) { - if (InMainFile) - MainFileMacros.insert(MacroName.getIdentifierInfo()->getName()); - } - - void EndOfMainFile() { - for (const auto &Entry : MainFileMacros) - Out->push_back(Entry.getKey()); - llvm::sort(*Out); - } - -private: - const SourceManager &SM; - bool InMainFile = true; - llvm::StringSet<> MainFileMacros; - std::vector *Out; -}; - class CppFilePreambleCallbacks : public PreambleCallbacks { public: CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback) - : File(File), ParsedCallback(ParsedCallback) { - } + : File(File), ParsedCallback(ParsedCallback) {} IncludeStructure takeIncludes() { return std::move(Includes); } - std::vector takeMainFileMacros() { - return std::move(MainFileMacros); - } + MainFileMacros takeMacros() { return std::move(Macros); } CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); } @@ -79,14 +44,17 @@ public: void BeforeExecute(CompilerInstance &CI) override { CanonIncludes.addSystemHeadersMapping(CI.getLangOpts()); + LangOpts = &CI.getLangOpts(); SourceMgr = &CI.getSourceManager(); } std::unique_ptr createPPCallbacks() override { - assert(SourceMgr && "SourceMgr must be set at this point"); + assert(SourceMgr && LangOpts && + "SourceMgr and LangOpts must be set at this point"); + return std::make_unique( collectIncludeStructureCallback(*SourceMgr, &Includes), - std::make_unique(*SourceMgr, &MainFileMacros)); + std::make_unique(*SourceMgr, *LangOpts, Macros)); } CommentHandler *getCommentHandler() override { @@ -99,20 +67,21 @@ private: PreambleParsedCallback ParsedCallback; IncludeStructure Includes; CanonicalIncludes CanonIncludes; - std::vector MainFileMacros; + MainFileMacros Macros; std::unique_ptr IWYUHandler = nullptr; - SourceManager *SourceMgr = nullptr; + const clang::LangOptions *LangOpts = nullptr; + const SourceManager *SourceMgr = nullptr; }; } // namespace PreambleData::PreambleData(PrecompiledPreamble Preamble, std::vector Diags, IncludeStructure Includes, - std::vector MainFileMacros, + MainFileMacros Macros, std::unique_ptr StatCache, CanonicalIncludes CanonIncludes) : Preamble(std::move(Preamble)), Diags(std::move(Diags)), - Includes(std::move(Includes)), MainFileMacros(std::move(MainFileMacros)), + Includes(std::move(Includes)), Macros(std::move(Macros)), StatCache(std::move(StatCache)), CanonIncludes(std::move(CanonIncludes)) { } @@ -181,7 +150,7 @@ buildPreamble(PathRef FileName, CompilerInvocation &CI, return std::make_shared( std::move(*BuiltPreamble), std::move(Diags), SerializedDeclsCollector.takeIncludes(), - SerializedDeclsCollector.takeMainFileMacros(), std::move(StatCache), + SerializedDeclsCollector.takeMacros(), std::move(StatCache), SerializedDeclsCollector.takeCanonicalIncludes()); } else { elog("Could not build a preamble for file {0}", FileName); diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index c1632ff..55dcce7 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -22,6 +22,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H +#include "CollectMacros.h" #include "Compiler.h" #include "Diagnostics.h" #include "FS.h" @@ -43,8 +44,7 @@ namespace clangd { /// be obtained during parsing must be eagerly captured and stored here. struct PreambleData { PreambleData(PrecompiledPreamble Preamble, std::vector Diags, - IncludeStructure Includes, - std::vector MainFileMacros, + IncludeStructure Includes, MainFileMacros Macros, std::unique_ptr StatCache, CanonicalIncludes CanonIncludes); @@ -57,7 +57,7 @@ struct PreambleData { // Macros defined in the preamble section of the main file. // Users care about headers vs main-file, not preamble vs non-preamble. // These should be treated as main-file entities e.g. for code completion. - std::vector MainFileMacros; + MainFileMacros Macros; // Cache of FS operations performed when building the preamble. // When reusing a preamble, this cache can be consumed to save IO. std::unique_ptr StatCache; diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index ce84614..9790f8b8 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -110,8 +110,9 @@ public: TraverseAST(AST.getASTContext()); // Add highlightings for macro expansions as they are not traversed by the // visitor. - for (SourceLocation Loc : AST.getMacros()) - addToken(Loc, HighlightingKind::Macro); + for (const auto &M : AST.getMacros().Ranges) + Tokens.push_back({HighlightingKind::Macro, M}); + // addToken(Loc, HighlightingKind::Macro); // Initializer lists can give duplicates of tokens, therefore all tokens // must be deduplicated. llvm::sort(Tokens); diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp index 65020bf..53e2f4b 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -229,8 +229,8 @@ TEST(ParsedASTTest, CanBuildInvocationWithUnknownArgs) { TEST(ParsedASTTest, CollectsMainFileMacroExpansions) { Annotations TestCase(R"cpp( - #define MACRO_ARGS(X, Y) X Y - // - premable ends, macros inside preamble are not considered in main file. + #define ^MACRO_ARGS(X, Y) X Y + // - preamble ends ^ID(int A); // Macro arguments included. ^MACRO_ARGS(^MACRO_ARGS(^MACRO_EXP(int), A), ^ID(= 2)); @@ -270,12 +270,11 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) { int D = DEF; )cpp"; ParsedAST AST = TU.build(); - const std::vector &MacroExpansionLocations = AST.getMacros(); std::vector MacroExpansionPositions; - for (const auto &L : MacroExpansionLocations) - MacroExpansionPositions.push_back( - sourceLocToPosition(AST.getSourceManager(), L)); - EXPECT_EQ(MacroExpansionPositions, TestCase.points()); + for (const auto &R : AST.getMacros().Ranges) + MacroExpansionPositions.push_back(R.start); + EXPECT_THAT(MacroExpansionPositions, + testing::UnorderedElementsAreArray(TestCase.points())); } } // namespace diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 50955b3..06c8d3a 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -425,8 +425,8 @@ TEST(SemanticHighlighting, GetsCorrectTokens) { // Tokens that share a source range but have conflicting Kinds are not // highlighted. R"cpp( - #define DEF_MULTIPLE(X) namespace X { class X { int X; }; } - #define DEF_CLASS(T) class T {}; + #define $Macro[[DEF_MULTIPLE]](X) namespace X { class X { int X; }; } + #define $Macro[[DEF_CLASS]](T) class T {}; // Preamble ends. $Macro[[DEF_MULTIPLE]](XYZ); $Macro[[DEF_MULTIPLE]](XYZW); @@ -465,8 +465,8 @@ TEST(SemanticHighlighting, GetsCorrectTokens) { } )cpp", R"cpp( - #define fail(expr) expr - #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); } + #define $Macro[[fail]](expr) expr + #define $Macro[[assert]](COND) if (!(COND)) { fail("assertion failed" #COND); } // Preamble ends. $Primitive[[int]] $Variable[[x]]; $Primitive[[int]] $Variable[[y]]; -- 2.7.4