[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

[Dmitri Gribenko via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:227-254
+namespace llvm {
+template <> struct DenseMapInfo<clang::NestedNameSpecifierLoc> {
+  using FirstInfo = DenseMapInfo<clang::NestedNameSpecifier *>;
+  using SecondInfo = DenseMapInfo<void *>;
+
+  static inline NestedNameSpecifierLoc getEmptyKey() {
+    return NestedNameSpecifierLoc(FirstInfo::getEmptyKey(),
----------------
eduucaldas wrote:
> Inpired on the definition of: 
> * `template<typename T> struct DenseMapInfo<T*>`
> * `template<typename T, typename U> struct DenseMapInfo<std::pair<T,U>>`
> 
> Please tell me if this is all silly.
This is the right way to use DenseMapInfo, just move the specialization into the file that defines NNSLoc.


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:892
 
-  syntax::NestedNameSpecifier *
-  BuildNestedNameSpecifier(const NestedNameSpecifierLoc &QualifierLoc) {
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc QualifierLoc) {
     if (!QualifierLoc)
----------------
Please add a comment that explains why we override Traverse for NNSLoc instead of WalkUpFrom.

Something like:

To build syntax tree nodes for NestedNameSpecifierLoc we override Traverse instead of WalkUpFrom because we want to traverse the children ourselves and build a list instead of a nested tree of name specifier prefixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85439