[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 210359.
jvikstrom marked 2 inline comments as done.
jvikstrom added a comment.

Ignore tokens outside of main file. Added testcase for assert.


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
@@ -29,9 +29,11 @@
   return Tokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+void checkHighlightings(llvm::StringRef Code, llvm::StringRef HeaderCode = "") {
   Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+  TestTU TU = TestTU::withCode(Test.code());
+  TU.HeaderCode = HeaderCode;
+  auto AST = TU.build();
   static const std::map<HighlightingKind, std::string> KindToString{
       {HighlightingKind::Variable, "Variable"},
       {HighlightingKind::Function, "Function"},
@@ -186,10 +188,70 @@
       using $Enum[[CD]] = $Enum[[CC]];
       $Enum[[CC]] $Function[[f]]($Class[[B]]);
       $Enum[[CD]] $Function[[f]]($Class[[BB]]);
+    )cpp",
+    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
+      void $Function[[foo]]() {
+        DEF_VAR($Variable[[X]],  123);
+        DEF_VAR_REV(908, $Variable[[XY]]);
+        int CPY( $Variable[[XX]] );
+        DEF_VAR_TYPE($Class[[A]], $Variable[[AA]]);
+        double SOME_NAME;
+        int SOME_NAME_SET;
+        $Variable[[variable]] = 20.1;
+        MACRO_CONCAT(var, 2, float);
+        DEF_VAR_T($Class[[A]], CPY(CPY($Variable[[Nested]])),
+              CPY($Class[[A]]()));
+        INC_VAR($Variable[[variable]]);
+      }
+      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); }
+      int $Variable[[x]];
+      int $Variable[[y]];
+      int $Function[[f]]();
+      void $Function[[foo]]() {
+        assert($Variable[[x]] != $Variable[[y]]);
+        assert($Variable[[x]] != $Function[[f]]());
+      }
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);
   }
+
+  // A separate test for macros in headers.
+  checkHighlightings(R"cpp(
+    DEFINE_Y
+    DXYZ_Y(A);
+  )cpp",
+                     R"cpp(
+    #define DXYZ(X) class X {};
+    #define DXYZ_Y(Y) DXYZ(x##Y)
+    #define DEFINE(X) in 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
@@ -39,6 +39,25 @@
                });
     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;
   }
 
@@ -178,9 +197,19 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
-    if (Loc.isMacroID())
-      // FIXME: skip tokens inside macros for now.
+    if(Loc.isMacroID()) {
+      // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+      if (!SM.isMacroArgExpansion(Loc))
+        return;
+      Loc = SM.getSpellingLoc(Loc);
+    }
+
+    if (!SM.isWrittenInMainFile(Loc)) {
+      // There are cases with macros where the spelling loc will not be in the
+      // main file and highlighting those would be incorrect.
       return;
+    }
+
 
     auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
     if (!R) {

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