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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
