[PATCH] D65101: [clangd] SelectionTree treats TranslationUnitDecl consistently with other containers.

Tue Jul 23 01:14:04 PDT 2019

hokein added inline comments.

================
Comment at: clang-tools-extra/clangd/Selection.h:100
   };
   // The most specific common ancestor of all the selected nodes.
+  const Node &commonAncestor() const;
----------------
I think it would be nice to mention "what means no selection" (TUDecl with partial)?

And it seems that we are missing some documentation describing the selection tree behavior on the preprocessor elements e.g. `#incl^ude`, header guard. 

================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp:47
   // Narrow the traversal scope to the selected node.
   Inputs.AST.getASTContext().setTraversalScope(
       {const_cast<Decl *>(InterestedDecl)});
----------------
we'll get a TUDecl if the whole file is selected, however this tweak doesn't want to traversal the whole TU (otherwise we will generate highlighting tokens for #included files), we only interested in the top level decls of the main file. 

================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp:38
          N = N->Parent)
-      if (dumpable(N->ASTNode))
+      if (N->Parent && dumpable(N->ASTNode))
         Node = N->ASTNode;
----------------
Is the `N->Parent` intended?  this seems like we'd exclude the TUDecl?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65101