[clang-tools-extra] r369275 - [clangd] Added highlighting for tokens that are macro arguments.

Johan Vikstrom via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 19 09:27:49 PDT 2019


Author: jvikstrom
Date: Mon Aug 19 09:27:49 2019
New Revision: 369275

URL: http://llvm.org/viewvc/llvm-project?rev=369275&view=rev
Log:
[clangd] Added highlighting for tokens that are macro arguments.

Summary:
Adds semantic highlighting for tokens that are a macro argument.
Example:
```
D_V(SomeVar);
```
The "SomeVar" inside the macro is highlighted as a variable now.

Tokens that are in a macro body expansion are ignored in this patch for three reasons.
* The spelling loc is inside the macro "definition" meaning it would highlight inside the macro definition (could probably easily be fixed by using getExpansionLoc instead of getSpellingLoc?)
* If wanting to highlight the macro definition this could create duplicate tokens. And if the tokens are of different types there would be conflicts (tokens in the same range but with different types). Say a macro defines some name and both a variable declaration and a function use this, there would be two tokens in the macro definition but one with Kind "Variable" and the other with Kind "Function".
* Thirdly, macro body expansions could come from a file that is not the main file (easily fixed, just check that the Loc is in the main file and not even a problem if we wanted to highlight the actual macro "invocation")

Reviewers: hokein, sammccall, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D64741

Modified:
    clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
    clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=369275&r1=369274&r2=369275&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Aug 19 09:27:49 2019
@@ -39,7 +39,27 @@ public:
     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 @@ private:
   }
 
   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;
 

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=369275&r1=369274&r2=369275&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Mon Aug 19 09:27:49 2019
@@ -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 @@ TEST(SemanticHighlighting, GetsCorrectTo
         $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 @@ TEST(SemanticHighlighting, GetsCorrectTo
     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) {




More information about the cfe-commits mailing list