[PATCH] D36176: [clang-diff] Fix some errors and inconsistencies

Alex Lorenz via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 2 05:16:09 PDT 2017


arphaman added inline comments.


================
Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:96
       : TreeImpl(llvm::make_unique<Impl>(this, Node, AST)) {}
+  SyntaxTree(const SyntaxTree &Tree) = delete;
   ~SyntaxTree();
----------------
johannes wrote:
> arphaman wrote:
> > It might be better to add a move constructor which would disable copy constructors.
> ASTDiff::getMapped internally uses the address of the SyntaxTree that is passed as parameter.
> So the tree must be exactly the same as the one that is passed to the constructor of ASTDiff. 
> 
> This is quite bad. I will change ASTDiff::getMapped to accept a boolean or split it into two methods
> Should I still add a move constructor?
Can't you just use the `TreeImpl` pointer value? That should be the same even after you've moved `SyntaxTree` with the move constructor.


https://reviews.llvm.org/D36176





More information about the cfe-commits mailing list