[clangd] Added highlighting for non type templates.

[jvikstrom]

Summary

Highlights pointers to variables as variables. Function pointers are highlighted as functions. Member function pointers are highlighted as members. Values are highlighted as TemplateParameter.

[ilya-biryukov]

clang-tools-extra/clangd/SemanticHighlighting.cpp

231

Why do we special-case template parameters, but not other kinds of variables?
We definitely need a comment explaining why template parameters are handled in a special way, but variables, parameters, fields are not.

[jvikstrom]

clang-tools-extra/clangd/SemanticHighlighting.cpp

231

Not quite sure what you mean about variables/parameters/fields not being handled in a special way.

The reason for special casing non type templates is because it probably gives more information/is more valuable to highlight a reference/pointer as a variable rather than a normal template parameter (same for methods/functions).

But maybe they all should just be highlighted as with the TemplateParameter kind instead?

[ilya-biryukov]

clang-tools-extra/clangd/SemanticHighlighting.cpp

231

Not quite sure what you mean about variables/parameters/fields not being handled in a special way.

Non-type template parameters are very similar to global and local variables, function parameters, class fields, etc.
We could also match those on type and highlight differently based on the type.

The reason for special casing non type templates is because it probably gives more information/is more valuable to highlight a reference/pointer as a variable rather than a normal template parameter (same for methods/functions).

However, if a global variable has a function pointer type we do not highlight it as a function. Why would this be different?

But maybe they all should just be highlighted as with the TemplateParameter kind instead?

I would personally vouch for this option. The highlighting functionality lets me understand what the name resolves to; if it's a template parameter and it would be highlighted as a variable instead, this would create confusion on my end. If I need to know the type I'll look at completion results or hover.

It is my personal preference and I'm ok with this going in a different direction if you feel the opposite is better. Just asking to put the rationale of this decision (why do this only for template parameters and not other things) in a comment somewhere in the source code.

[jvikstrom]

Highlight as TemplateParameter.

clang-tools-extra/clangd/SemanticHighlighting.cpp

231

I would personally vouch for this option. The highlighting functionality lets me understand what the name resolves to;

Very true actually, was a bit unsure what was the correct way to go but this is very true actually.

Changing to just highlight as TemplateParameter.

[ilya-biryukov]

LGTM

clang-tools-extra/clangd/SemanticHighlighting.cpp

void addToken(SourceLocation Loc, const NamedDecl *D) {
     if (isa<TemplateTemplateParmDecl>(D)) {
       addToken(Loc, HighlightingKind::TemplateParameter);
       return;
     }
     if (isa<TemplateTypeParmDecl>(D)) {
       addToken(Loc, HighlightingKind::TemplateParameter);
       return;
     }
+    if (isa<NonTypeTemplateParmDecl>(D)) {
+      addToken(Loc, HighlightingKind::TemplateParameter);
+      return;
+    }
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
     if (Loc.isMacroID())
       // FIXME: skip tokens inside macros for now.
       return;
 
     // Non top level decls that are included from a header are not filtered by
     // topLevelDecls. (example: method declarations being included from another

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

R"cpp(
       $Enum[[auto]] $Variable[[AE]] = $Enum[[E]]::$EnumConstant[[E]];
       $Class[[auto]] $Variable[[AF]] = $Class[[Foo]]();
       $Class[[decltype]](auto) $Variable[[AF2]] = $Class[[Foo]]();
       $Class[[auto]] *$Variable[[AFP]] = &$Variable[[AF]];
       $Enum[[auto]] &$Variable[[AER]] = $Variable[[AE]];
       $Primitive[[auto]] $Variable[[Form]] = 10.2 + 2 * 4;
       $Primitive[[decltype]]($Variable[[Form]]) $Variable[[F]] = 10;
       auto $Variable[[Fun]] = []()->$Primitive[[void]]{};
+    )cpp",
+    R"cpp(
+      class $Class[[G]] {};
+      template<$Class[[G]] *$TemplateParameter[[U]]>
+      class $Class[[GP]] {};
+      template<$Class[[G]] &$TemplateParameter[[U]]>
+      class $Class[[GR]] {};
+      template<$Primitive[[int]] *$TemplateParameter[[U]]>
+      class $Class[[IP]] {
+        $Primitive[[void]] $Method[[f]]() {
+          *$TemplateParameter[[U]] += 5;
+        }
+      };
+      template<$Primitive[[unsigned]] $TemplateParameter[[U]] = 2>
+      class $Class[[Foo]] {
+        $Primitive[[void]] $Method[[f]]() {
+          for($Primitive[[int]] $Variable[[I]] = 0;
+            $Variable[[I]] < $TemplateParameter[[U]];) {}
+        }
+      };
+
+      $Class[[G]] $Variable[[L]];
+      $Primitive[[void]] $Function[[f]]() {
+        $Class[[Foo]]<123> $Variable[[F]];
+        $Class[[GP]]<&$Variable[[L]]> $Variable[[LL]];
+        $Class[[GR]]<$Variable[[L]]> $Variable[[LLL]];
+      }
+    )cpp",
+    R"cpp(
+      template<typename $TemplateParameter[[T]], 
+        $Primitive[[void]] (T::*$TemplateParameter[[method]])($Primitive[[int]])>
+      struct $Class[[G]] {
+        $Primitive[[void]] $Method[[foo]](
+            $TemplateParameter[[T]] *$Variable[[O]]) {
+          ($Variable[[O]]->*$TemplateParameter[[method]])(10);
+        }
+      };
+      struct $Class[[F]] {
+        $Primitive[[void]] $Method[[f]]($Primitive[[int]]);
+      };
+      template<$Primitive[[void]] (*$TemplateParameter[[Func]])()>
+      struct $Class[[A]] {
+        $Primitive[[void]] $Method[[f]]() {
+          (*$TemplateParameter[[Func]])();
+        }
+      };
+
+      $Primitive[[void]] $Function[[foo]]() {
+        $Class[[F]] $Variable[[FF]];
+        $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $Variable[[GG]];
+        $Variable[[GG]].$Method[[foo]](&$Variable[[FF]]);
+        $Class[[A]]<$Function[[foo]]> $Variable[[AA]];
+      }
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);
   }
 
   checkHighlightings(R"cpp(
     class $Class[[A]] {
       #include "imp.h"