[PATCH] D64137: [clangd] Add a hidden tweak to annotate all highlighting tokens of the file.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp:16
+
+llvm::StringRef toTextMateScope(HighlightingKind Kind) {
+  static const auto& TextMateLookupTable = getTextMateScopeLookupTable();
----------------
can we move this to SemanticHighlighting.h, and define getTextMateScopeLookupTable() in terms of it? It seems much more fundamental.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp:53
+Expected<Tweak::Effect> AnnotateHighlightings::apply(const Selection &Inputs) {
+  const auto &BackupScopes = Inputs.AST.getASTContext().getTraversalScope();
+  auto CleanupTask = llvm::make_scope_exit(
----------------
This needs a comment


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp:59
+      {const_cast<Decl *>(InterestedDecl)});
+  auto HighlightingTokens = getSemanticHighlightings(Inputs.AST);
+  auto &SM = Inputs.AST.getSourceManager();
----------------
nit: can you give the overridden traversal scope as narrow a scope as possible? (put it in a block, or just reset explicitly immediately afterwards?) I'm a little paranoid about this scope being reused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64137