sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
This seems right to me!
================
Comment at: clang/lib/Sema/TreeTransform.h:7947
// Always rebuild; we don't know if this needs to be injected into a new
// context or if the promise type has changed.
----------------
nridge wrote:
> sammccall wrote:
> > (FWIW I don't know what "injected into a new context" means, but I don't
> > think the promise type can change since DependentCoawaitExpr was added in
> > 20f25cb6dfb3364847f4c570b1914fe51e585def.)
> >
> Is the implication of this that there are some conditions under which we can
> reuse the original CoawaitExpr rather than rebuilding it?
Yes, I think this is how instantiation of non-dependent code works, e.g. in the
following TransformObjCAtTryStmt:
```
// If nothing changed, just retain this statement.
if (!getDerived().AlwaysRebuild() &&
TryBody.get() == S->getTryBody() &&
!AnyCatchChanged &&
Finally.get() == S->getFinallyStmt())
return S;
```
I think missing this opportunity means that a non-dependent co_await will
nevertheless be instantiated into a new node, which in turn means that every
containing node will *also* be instantiated (because its child nodes changed
during instantiation).
So this affects speed and memory usage, but I suppose not correctness.
Anyway I don't know exactly what the conditions are where it's safe to reuse
the node, I suppose we leave this as-is.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115187/new/
https://reviews.llvm.org/D115187
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
