This revision was automatically updated to reflect the committed changes.
jvikstrom marked 2 inline comments as done.
Closed by commit rL369275: [clangd] Added highlighting for tokens that are
macro arguments. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D64741?vs=215907&id=215929#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64741/new/
https://reviews.llvm.org/D64741
Files:
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -39,7 +39,27 @@
llvm::sort(Tokens);
auto Last = std::unique(Tokens.begin(), Tokens.end());
Tokens.erase(Last, Tokens.end());
- return Tokens;
+ // Macros can give tokens that have the same source range but conflicting
+ // kinds. In this case all tokens sharing this source range should be
+ // removed.
+ std::vector<HighlightingToken> NonConflicting;
+ NonConflicting.reserve(Tokens.size());
+ for (ArrayRef<HighlightingToken> TokRef = Tokens; !TokRef.empty();) {
+ ArrayRef<HighlightingToken> Conflicting =
+ TokRef.take_while([&](const HighlightingToken &T) {
+ // TokRef is guaranteed at least one element here because otherwise
+ // this predicate would never fire.
+ return T.R == TokRef.front().R;
+ });
+ // If there is exactly one token with this range it's non conflicting and
+ // should be in the highlightings.
+ if (Conflicting.size() == 1)
+ NonConflicting.push_back(TokRef.front());
+ // TokRef[Conflicting.size()] is the next token with a different range (or
+ // the end of the Tokens).
+ TokRef = TokRef.drop_front(Conflicting.size());
+ }
+ return NonConflicting;
}
bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) {
@@ -236,13 +256,18 @@
}
void addToken(SourceLocation Loc, HighlightingKind Kind) {
- if (Loc.isMacroID())
- // FIXME: skip tokens inside macros for now.
- return;
+ if(Loc.isMacroID()) {
+ // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+ if (!SM.isMacroArgExpansion(Loc))
+ return;
+ Loc = SM.getSpellingLoc(Loc);
+ }
// Non top level decls that are included from a header are not filtered by
// topLevelDecls. (example: method declarations being included from another
// file for a class from another file)
+ // There are also cases with macros where the spelling loc will not be in the
+ // main file and the highlighting would be incorrect.
if (!isInsideMainFile(Loc, SM))
return;
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -16,10 +16,6 @@
namespace clang {
namespace clangd {
- void PrintTo(const HighlightingToken &T, ::std::ostream *OS) {
- *OS << "(" << T.R.start.line << ", " << T.R.start.character << ") -> (" << T.R.end.line << ", " << T.R.end.character << "): " << (int)T.Kind;
-}
-
namespace {
std::vector<HighlightingToken>
@@ -380,6 +376,56 @@
$Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $Variable[[GG]];
$Variable[[GG]].$Method[[foo]](&$Variable[[FF]]);
$Class[[A]]<$Function[[foo]]> $Variable[[AA]];
+ )cpp",
+ // 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 {};
+ DEF_MULTIPLE(XYZ);
+ DEF_MULTIPLE(XYZW);
+ DEF_CLASS($Class[[A]])
+ #define MACRO_CONCAT(X, V, T) T foo##X = V
+ #define DEF_VAR(X, V) int X = V
+ #define DEF_VAR_T(T, X, V) T X = V
+ #define DEF_VAR_REV(V, X) DEF_VAR(X, V)
+ #define CPY(X) X
+ #define DEF_VAR_TYPE(X, Y) X Y
+ #define SOME_NAME variable
+ #define SOME_NAME_SET variable2 = 123
+ #define INC_VAR(X) X += 2
+ $Primitive[[void]] $Function[[foo]]() {
+ DEF_VAR($Variable[[X]], 123);
+ DEF_VAR_REV(908, $Variable[[XY]]);
+ $Primitive[[int]] CPY( $Variable[[XX]] );
+ DEF_VAR_TYPE($Class[[A]], $Variable[[AA]]);
+ $Primitive[[double]] SOME_NAME;
+ $Primitive[[int]] SOME_NAME_SET;
+ $Variable[[variable]] = 20.1;
+ MACRO_CONCAT(var, 2, $Primitive[[float]]);
+ DEF_VAR_T($Class[[A]], CPY(CPY($Variable[[Nested]])),
+ CPY($Class[[A]]()));
+ INC_VAR($Variable[[variable]]);
+ }
+ $Primitive[[void]] SOME_NAME();
+ DEF_VAR($Variable[[XYZ]], 567);
+ DEF_VAR_REV(756, $Variable[[AB]]);
+
+ #define CALL_FN(F) F();
+ #define DEF_FN(F) void F ()
+ DEF_FN($Function[[g]]) {
+ CALL_FN($Function[[foo]]);
+ }
+ )cpp",
+ R"cpp(
+ #define fail(expr) expr
+ #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+ $Primitive[[int]] $Variable[[x]];
+ $Primitive[[int]] $Variable[[y]];
+ $Primitive[[int]] $Function[[f]]();
+ $Primitive[[void]] $Function[[foo]]() {
+ assert($Variable[[x]] != $Variable[[y]]);
+ assert($Variable[[x]] != $Function[[f]]());
}
)cpp"};
for (const auto &TestCase : TestCases) {
@@ -396,6 +442,19 @@
int someMethod();
void otherMethod();
)cpp"}});
+
+ // A separate test for macros in headers.
+ checkHighlightings(R"cpp(
+ #include "imp.h"
+ DEFINE_Y
+ DXYZ_Y(A);
+ )cpp",
+ {{"imp.h", R"cpp(
+ #define DXYZ(X) class X {};
+ #define DXYZ_Y(Y) DXYZ(x##Y)
+ #define DEFINE(X) int X;
+ #define DEFINE_Y DEFINE(Y)
+ )cpp"}});
}
TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
