johannes 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();
----------------
arphaman wrote:
> 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.
ok perfect


https://reviews.llvm.org/D36176



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

Reply via email to