[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 214619.
jvikstrom added a comment.

Rebased to master.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64741/new/

https://reviews.llvm.org/D64741

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


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -323,6 +323,57 @@
       $Primitive[[auto]] $Variable[[Form]] = 10.2 + 2 * 4;
       $Primitive[[decltype]]($Variable[[Form]]) $Variable[[F]] = 10;
       auto $Variable[[Fun]] = []()->$Primitive[[void]]{};
+    )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) {
     checkHighlightings(TestCase);
@@ -338,6 +389,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) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -38,6 +38,25 @@
     llvm::sort(Tokens);
     auto Last = std::unique(Tokens.begin(), Tokens.end());
     Tokens.erase(Last, Tokens.end());
+
+    // 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.
+    for (unsigned I = 0; I < Tokens.size(); ++I) {
+      ArrayRef<HighlightingToken> TokRef(Tokens);
+      ArrayRef<HighlightingToken> Conflicting =
+          llvm::ArrayRef<HighlightingToken>(TokRef.begin() + I, TokRef.end())
+              .take_while([&](const HighlightingToken &T) {
+                return T.R == Tokens[I].R;
+              });
+
+      if (Conflicting.size() > 1) {
+        Tokens.erase(Tokens.begin() + I,
+                     Tokens.begin() + I + Conflicting.size());
+        --I;
+      }
+    }
+
     return Tokens;
   }
 
@@ -227,13 +246,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;
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits