[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

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