Author: ioeric Date: Thu Sep 6 02:59:37 2018 New Revision: 341534 URL: http://llvm.org/viewvc/llvm-project?rev=341534&view=rev Log: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.
Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits Differential Revision: https://reviews.llvm.org/D51688 Modified: clang-tools-extra/trunk/clangd/AST.cpp clang-tools-extra/trunk/clangd/AST.h clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Modified: clang-tools-extra/trunk/clangd/AST.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=341534&r1=341533&r2=341534&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/AST.cpp (original) +++ clang-tools-extra/trunk/clangd/AST.cpp Thu Sep 6 02:59:37 2018 @@ -61,5 +61,16 @@ llvm::Optional<SymbolID> getSymbolID(con return SymbolID(USR); } +llvm::Optional<SymbolID> getSymbolID(const IdentifierInfo &II, + const MacroInfo *MI, + const SourceManager &SM) { + if (MI == nullptr) + return None; + llvm::SmallString<128> USR; + if (index::generateUSRForMacro(II.getName(), MI->getDefinitionLoc(), SM, USR)) + return None; + return SymbolID(USR); +} + } // namespace clangd } // namespace clang Modified: clang-tools-extra/trunk/clangd/AST.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=341534&r1=341533&r2=341534&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/AST.h (original) +++ clang-tools-extra/trunk/clangd/AST.h Thu Sep 6 02:59:37 2018 @@ -37,6 +37,17 @@ std::string printQualifiedName(const Nam /// Gets the symbol ID for a declaration, if possible. llvm::Optional<SymbolID> getSymbolID(const Decl *D); +/// Gets the symbol ID for a macro, if possible. +/// Currently, this is an encoded USR of the macro, which incorporates macro +/// locations (e.g. file name, offset in file). +/// FIXME: the USR semantics might not be stable enough as the ID for index +/// macro (e.g. a change in definition offset can result in a different USR). We +/// could change these semantics in the future by reimplementing this funcure +/// (e.g. avoid USR for macros). +llvm::Optional<SymbolID> getSymbolID(const IdentifierInfo &II, + const MacroInfo *MI, + const SourceManager &SM); + } // namespace clangd } // namespace clang Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=341534&r1=341533&r2=341534&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Sep 6 02:59:37 2018 @@ -505,14 +505,15 @@ private: }; // Determine the symbol ID for a Sema code completion result, if possible. -llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R) { +llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R, + const SourceManager &SM) { switch (R.Kind) { case CodeCompletionResult::RK_Declaration: case CodeCompletionResult::RK_Pattern: { return clang::clangd::getSymbolID(R.Declaration); } case CodeCompletionResult::RK_Macro: - // FIXME: Macros do have USRs, but the CCR doesn't contain enough info. + return clang::clangd::getSymbolID(*R.Macro, R.MacroDefInfo, SM); case CodeCompletionResult::RK_Keyword: return None; } @@ -1435,7 +1436,8 @@ private: llvm::DenseSet<const Symbol *> UsedIndexResults; auto CorrespondingIndexResult = [&](const CodeCompletionResult &SemaResult) -> const Symbol * { - if (auto SymID = getSymbolID(SemaResult)) { + if (auto SymID = + getSymbolID(SemaResult, Recorder->CCSema->getSourceManager())) { auto I = IndexResults.find(*SymID); if (I != IndexResults.end()) { UsedIndexResults.insert(&*I); Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=341534&r1=341533&r2=341534&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Thu Sep 6 02:59:37 2018 @@ -385,18 +385,16 @@ bool SymbolCollector::handleMacroOccuren Roles & static_cast<unsigned>(index::SymbolRole::Definition))) return true; - llvm::SmallString<128> USR; - if (index::generateUSRForMacro(Name->getName(), MI->getDefinitionLoc(), SM, - USR)) + auto ID = getSymbolID(*Name, MI, SM); + if (!ID) return true; - SymbolID ID(USR); // Only collect one instance in case there are multiple. - if (Symbols.find(ID) != nullptr) + if (Symbols.find(*ID) != nullptr) return true; Symbol S; - S.ID = std::move(ID); + S.ID = std::move(*ID); S.Name = Name->getName(); S.IsIndexedForCodeCompletion = true; S.SymInfo = index::getSymbolInfoForMacro(*MI); @@ -445,11 +443,9 @@ void SymbolCollector::finish() { if (Opts.CollectMacro) { assert(PP); for (const IdentifierInfo *II : ReferencedMacros) { - llvm::SmallString<128> USR; if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo()) - if (!index::generateUSRForMacro(II->getName(), MI->getDefinitionLoc(), - PP->getSourceManager(), USR)) - IncRef(SymbolID(USR)); + if (auto ID = getSymbolID(*II, MI, PP->getSourceManager())) + IncRef(*ID); } } Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=341534&r1=341533&r2=341534&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Thu Sep 6 02:59:37 2018 @@ -1882,6 +1882,17 @@ TEST(CompletionTest, NoInsertIncludeIfOn AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude())))); } +TEST(CompletionTest, MergeMacrosFromIndexAndSema) { + Symbol Sym; + Sym.Name = "Clangd_Macro_Test"; + Sym.ID = SymbolID("c:foo.cpp@8@macro@Clangd_Macro_Test"); + Sym.SymInfo.Kind = index::SymbolKind::Macro; + Sym.IsIndexedForCodeCompletion = true; + EXPECT_THAT(completions("#define Clangd_Macro_Test\nClangd_Macro_T^", {Sym}) + .Completions, + UnorderedElementsAre(Named("Clangd_Macro_Test"))); +} + } // namespace } // namespace clangd } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
