r.stahl added inline comments.

================
Comment at: lib/AST/ASTImporter.cpp:6776
+  // been invalidated to avoid repeatedly calling this.
+  ToContext.invalidateParents();
+
----------------
martong wrote:
> Can an `Expr` has a parent too? If yes, why not invalidate the parents in 
> `Import(Expr*)` ?
> I am also a bit concerned to call the invalidation during the import of all 
> `Stmt` (and possible during the import of `Expr`s too).
> Isn't it enough to invalidate only during `Import(Decl*)`?
The Import(Expr) function just defers to Import(Stmt) so a call there would be 
redundant as Alexey commented.

I don't think it would be enough to invalidate in Import(Decl), because 
Import(Stmt) is part of the public API. I don't know if it makes sense to just 
import Stmts and if there is any user code doing that, but it is theoretically 
possible. In that case we should make Import(Stmt) and Import(Expr) private.

I wouldn't be too concerned about too many calls to invalidate, since the 
function doesn't do anything once the parent info has been released and it will 
only be rebuilt once someone calls ACtx::getParents - which is not the case 
while importing.


https://reviews.llvm.org/D46940



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

Reply via email to