aaron.ballman added inline comments.
================
Comment at: include/clang/AST/TextNodeDumper.h:28
const comments::FullComment *> {
+ TextTreeStructure &TreeStructure;
raw_ostream &OS;
----------------
This makes me a bit wary because you create a node dumper in the same
situations you make a tree structure object, but now there's a strict ordering
between the two object creations. If you're doing this construction local to a
function, you wind up with a dangling reference unless you're careful (which is
unfortunate, but not the end of the world). If you're doing this construction
as part of a constructor's initializer list, you now have to properly order the
member declarations within the class and that is also unfortunate. Given that
those are the two common scenarios for how I envision constructing an ast dump
of some kind, I worry about the fragility. e.g.,
```
unique_ptr<ASTConsumer> createASTDumper(...) {
TextTreeStructure TreeStructure;
TextNodeDumper NodeDumper(TreeStructure); // Oops, dangling reference
return make_unique<MySuperAwesomeASTDumper>(TreeStructure, NodeDumper, ...);
}
// vs
struct MySuperAwesomeASTDumper : ... {
MySuperAwesomeASTDumper() : TreeStructure(...), NodeDumper(TreeStructure,
...) {}
private:
TextTreeStructure TreeStructure; // This order is now SUPER important
TextNodeDumper NodeDumper;
};
```
There's a part of me that wonders if a better approach is to have this object
passed to the `dumpFoo()` calls as a reference parameter. This way, the caller
is still responsible for creating an object, but the creation order between the
tree and the node dumper isn't as fragile.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55337/new/
https://reviews.llvm.org/D55337
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
