steveire marked an inline comment as done.
steveire added inline comments.

================
Comment at: include/clang/AST/ASTDumpTraverser.h:83
+
+  NodeVisitorType &getNodeVisitor() { return getDerived().doGetNodeVisitor(); }
+  Derived &getDerived() { return *static_cast<Derived *>(this); }
----------------
aaron.ballman wrote:
> Given that `ASTDumpTraverser` is itself a visitor of nodes, I wonder if a 
> better name for this would be `getNodeDumper()` or something (and similarly 
> renaming the template parameter)?
I'm not opposed to alternative names, but I do like the current name. We 
`visit` each node before traversing.

`NodeVisitor` names make sense to me because 'dumping' isn't necessarily what 
the delegate class does. It is more consistent with what 'Visitor' usually 
means.

Perhaps a type name of `NodeDelegateType` and an accessor `NodeDelegateType 
&getNodeDelegate()` would also make sense. Then we would have eg:

```
  void Visit(const Attr *A) {
    getNodeDelegate().AddChild([=] {
      getNodeDelegate().Visit(A);
      ConstAttrVisitor<Derived>::Visit(A);
    });
  }
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D57472



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

Reply via email to