ilya-biryukov added a comment.

Overall LG, just a few quick questions.



================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:65
     SelectionTree ASTSelection;
+    /// The Index being used by ClangdServer.
+    const SymbolIndex *Index = nullptr;
----------------
NIT: Let's avoid mentioning `ClangdServer` here. `ClangdServer` is a "glue" 
that brings things like tweaks, code completion and other code together... 
Mentioning it here is sort of a "reverse" dependency...


================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:66
+    /// The Index being used by ClangdServer.
+    const SymbolIndex *Index = nullptr;
     // FIXME: provide a way to get sources and ASTs for other files.
----------------
NIT: Maybe put it right before `ParsedAST`?


================
Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:71
+  // Index to be passed into Tweak::Selection.
+  const SymbolIndex *Index = nullptr;
+
----------------
How is this index populated? Each test has to mock it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69165



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

Reply via email to