[PATCH] D65996: [clangd] Highlighting auto variables as the deduced type.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:153
+        // "auto" keyword other than using an offset.
+        Loc = Loc.getLocWithOffset(9);
+      Deduced.getTypePtr()->dump();
----------------
The heuristic using here is fragile, (it doesn't handle `decltype  (auto)` correctly).

I'm not quite sure we really want to highlight the inner `auto`, it may introduce inconsistent behavior (see we have `$Primitive[[decltype]]($Variable[[Form]])` in the unittest). I think we could just leave it with the default behavior here...


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:154
+        Loc = Loc.getLocWithOffset(9);
+      Deduced.getTypePtr()->dump();
+      addType(Loc, Deduced.getTypePtr());
----------------
nit: remove the debug log


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:161
 private:
-  void addTypeLoc(SourceLocation Loc, const TypeLoc &TL) {
-    if (const Type *TP = TL.getTypePtr()) {
-      if (const TagDecl *TD = TP->getAsTagDecl())
-        addToken(Loc, TD);
-      if (TP->isBuiltinType())
-        // Builtins must be special cased as they do not have a TagDecl.
-        addToken(Loc, HighlightingKind::Primitive);
+  void addType(SourceLocation Loc, const Type *TP) {
+    if (TP->isBuiltinType())
----------------
assert TP cannot be null, or even move the null-ptr check into this method?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65996