[PATCH] D115187: [clangd] Expose CoawaitExpr's operand in the AST
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 20 02:26:38 PDT 2022
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.
> 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() &&
Finally.get() == S->getFinallyStmt())
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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits