usaxena95 updated this revision to Diff 228869.
usaxena95 marked 2 inline comments as done.
usaxena95 added a comment.
Herald added a subscriber: mgorny.
Added tests for CollectMacros.h
Addressed comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70008/new/
https://reviews.llvm.org/D70008
Files:
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2050,6 +2050,32 @@
} // namespace ns
int main() { [[^ns]]::Foo foo; }
)cpp",
+
+ R"cpp(// Macros
+ #define [[FOO]](x,y) (x + y)
+ // FIXME: No references for nested macros.
+ #define BAR(x,y,z) (FOO(x, y) * FOO(y, z))
+ int main() {
+ int x = [[FOO]](1, 2);
+ int y = [[FOO]]([[FO^O]](x, 1), [[FOO]](1, 1));
+ int z = BAR(1, 2, 3);
+ }
+ #define FOO(x) (x + 1)
+ )cpp",
+
+ R"cpp(// Macros: Cursor on definition.
+ #define [[F^OO]](x,y) (x + y)
+ int main() { int x = [[FOO]](1, 2); }
+ )cpp",
+
+ R"cpp( // Choose correct definiton in case of multiple definitions.
+ #define abc 1
+ #undef abc
+
+ #define [[abc]] 2
+ int main() { int a = [[abc]];}
+ #undef [[ab^c]]
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -260,6 +260,15 @@
// Includes macro expansions in arguments that are expressions
^assert(0 <= ^BAR);
}
+
+ #ifdef ^UNDEFINED
+ #endif
+
+ #define ^MULTIPLE_DEFINITION 1
+ #undef ^MULTIPLE_DEFINITION
+
+ #define ^MULTIPLE_DEFINITION 2
+ #undef ^MULTIPLE_DEFINITION
)cpp");
auto TU = TestTU::withCode(TestCase.code());
TU.HeaderCode = R"cpp(
@@ -274,7 +283,11 @@
)cpp";
ParsedAST AST = TU.build();
std::vector<Position> MacroExpansionPositions;
- for (const auto &R : AST.getMacros().Ranges)
+ for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
+ for (const auto &R : SIDToRefs.second)
+ MacroExpansionPositions.push_back(R.start);
+ }
+ for (const auto &R : AST.getMacros().UnknownMacros)
MacroExpansionPositions.push_back(R.start);
EXPECT_THAT(MacroExpansionPositions,
testing::UnorderedElementsAreArray(TestCase.points()));
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -0,0 +1,86 @@
+#include "Annotations.h"
+#include "CollectMacros.h"
+#include "Matchers.h"
+#include "TestTU.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::UnorderedElementsAreArray;
+
+// Assumes that the ranges are named from 1 to N for some N.
+Matcher<std::vector<std::vector<Range>>>
+AreMacroRefsFrom(const Annotations &Test) {
+ std::vector<Matcher<std::vector<Range>>> Expected;
+ for (int I = 1;; I++) {
+ const auto Refs = Test.ranges(llvm::to_string(I));
+ if (Refs.empty())
+ break;
+ Expected.push_back(UnorderedElementsAreArray(Refs));
+ }
+ return UnorderedElementsAreArray(Expected);
+}
+
+std::vector<std::vector<Range>> collecknownReferences(MainFileMacros M) {
+ std::vector<std::vector<Range>> Result;
+ for (const auto &SIDToRef : M.MacroRefs) {
+ Result.emplace_back(SIDToRef.second.begin(), SIDToRef.second.end());
+ }
+ return Result;
+}
+
+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".
+ const char *Tests[] = {
+ R"cpp(// Macros: Cursor on definition.
+ #define $1[[FOO]](x,y) (x + y)
+ int main() { int x = $1[[FOO]]($1[[FOO]](3, 4), $1[[FOO]](5, 6)); }
+ )cpp",
+ R"cpp( // Multiple definitions.
+ #define $1[[abc]] 1
+ int func1() { int a = $1[[abc]];}
+ #undef $1[[abc]]
+
+ #define $2[[abc]] 2
+ int func2() { int a = $2[[abc]];}
+ #undef $2[[abc]]
+ )cpp",
+ R"cpp(
+ #ifdef $Unknown[[UNDEFINED]]
+ #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
+ 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
+ int A = $2[[FOO]];
+ )cpp"};
+ for (const char *Test : Tests) {
+ Annotations T(Test);
+ auto AST = TestTU::withCode(T.code()).build();
+ EXPECT_THAT(AST.getMacros().UnknownMacros,
+ UnorderedElementsAreArray(T.ranges("Unknown")))
+ << Test;
+ EXPECT_THAT(collecknownReferences(AST.getMacros()), AreMacroRefsFrom(T))
+ << Test;
+ }
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -30,6 +30,7 @@
ClangdTests.cpp
CodeCompleteTests.cpp
CodeCompletionStringsTests.cpp
+ CollectMacrosTests.cpp
ContextTests.cpp
DexTests.cpp
DiagnosticsTests.cpp
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -21,6 +21,7 @@
#include "index/Merge.h"
#include "index/Relation.h"
#include "index/SymbolCollector.h"
+#include "index/SymbolID.h"
#include "index/SymbolLocation.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
@@ -47,6 +48,7 @@
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
+#include <set>
namespace clang {
namespace clangd {
@@ -891,7 +893,28 @@
}
auto Loc = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
- // TODO: should we handle macros, too?
+
+ // Handle macros.
+ if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
+ if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
+ const auto &IDToRefs = AST.getMacros().MacroRefs;
+ auto Refs = IDToRefs.find(*MacroSID);
+ if (Refs != IDToRefs.end()) {
+ auto URIMainFile =
+ URIForFile::canonicalize(*MainFilePath, *MainFilePath);
+ for (const auto Ref : Refs->second) {
+ Location Loc;
+ Loc.uri = URIMainFile;
+ Loc.range = Ref;
+ Results.push_back(Loc);
+ }
+ }
+ }
+ if (Results.size() > Limit)
+ Results.resize(Limit);
+ return Results;
+ }
+
// We also show references to the targets of using-decls, so we include
// DeclRelation::Underlying.
DeclRelationSet Relations = DeclRelation::TemplatePattern |
@@ -911,8 +934,7 @@
MainFileRefs.end());
for (const auto &Ref : MainFileRefs) {
if (auto Range =
- getTokenRange(AST.getASTContext().getSourceManager(),
- AST.getASTContext().getLangOpts(), Ref.Loc)) {
+ getTokenRange(SM, AST.getASTContext().getLangOpts(), Ref.Loc)) {
Location Result;
Result.range = *Range;
Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -312,8 +312,13 @@
Builder.addToken(R.NameLoc, *Kind);
});
// Add highlightings for macro expansions.
- for (const auto &M : AST.getMacros().Ranges)
+ for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
+ for (const auto &M : SIDToRefs.second)
+ Builder.addToken({HighlightingKind::Macro, M});
+ }
+ for (const auto &M : AST.getMacros().UnknownMacros)
Builder.addToken({HighlightingKind::Macro, M});
+
return std::move(Builder).collect();
}
Index: clang-tools-extra/clangd/CollectMacros.h
===================================================================
--- clang-tools-extra/clangd/CollectMacros.h
+++ clang-tools-extra/clangd/CollectMacros.h
@@ -9,10 +9,13 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
+#include "AST.h"
#include "Protocol.h"
#include "SourceCode.h"
+#include "index/SymbolID.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Lex/PPCallbacks.h"
+#include "llvm/ADT/DenseMap.h"
#include <string>
namespace clang {
@@ -22,7 +25,11 @@
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<Range> Ranges;
+ llvm::DenseMap<SymbolID, std::set<Range>> MacroRefs;
+ // Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a
+ // reference to an undefined macro. Store them separately, e.g. for semantic
+ // highlighting.
+ std::vector<Range> UnknownMacros;
};
/// Collects macro references (e.g. definitions, expansions) in the main file.
@@ -81,7 +88,11 @@
if (auto Range = getTokenRange(SM, LangOpts, Loc)) {
Out.Names.insert(MacroNameTok.getIdentifierInfo()->getName());
- Out.Ranges.push_back(*Range);
+ if (auto SID =
+ getSymbolID(MacroNameTok.getIdentifierInfo()->getName(), MI, SM))
+ Out.MacroRefs[*SID].insert(*Range);
+ else
+ Out.UnknownMacros.push_back(*Range);
}
}
const SourceManager &SM;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
