[clangd] Expose CoawaitExpr's operand in the AST

[nridge]

Summary

Previously the Expr returned by getOperand() was actually the
subexpression common to the "ready", "suspend", and "resume"
expressions, which often isn't just the operand but e.g.
await_transform() called on the operand.

It's important for the AST to expose the operand as written
in the source for traversals and tools like clangd to work
correctly.

Fixes https://github.com/clangd/clangd/issues/939

[sammccall]

FWIW I agree the "store" option would be more reliable here and also simpler - I think it's just storing one more pointer per co_await, and one we have easy access to.
(I would be worried about what happens to that expression in template instantiation, but dependent CoroutineSuspendExprs only store the written form anyway, so it probably actually becomes cleaner)

A change that probably goes along with this one is having RecursiveASTVisitor traverse the syntactic form instead of the semantic one if traversal of implicit code is off.

[nridge]

Rework to store operand instead of digging it out from the common-expr

[nridge]

(Keeping this in the WIP state for now, there's some crashiness I need to sort out)

[nridge]

Add clang test

[nridge]

Can't seem to reproduce the crashiness any more, so I wrote a clang test and I'm submitting this for review.

I'm happy to take guidance on what the clang test should look like. What I have currently is a FileCheck test to verify that a CoawaitExpr has the expected subexpressions. I added a new file for this because the existing coroutine-related tests that I could find did not use FileCheck (they were just testing for expected-errors and such).

[nridge]

In D115187#3192034, @sammccall wrote:

A change that probably goes along with this one is having RecursiveASTVisitor traverse the syntactic form instead of the semantic one if traversal of implicit code is off.

I believe that's already the behaviour that falls out of the current implementation:

DEF_TRAVERSE_STMT(CoawaitExpr, {
  if (!getDerived().shouldVisitImplicitCode()) {
    TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getOperand());
    ShouldVisitChildren = false;
  }
})

If we are not visiting implicit code, we only visit the operand, otherwise we visit all subexpressions.

[sammccall]

This generally looks really good, I'm worried about the separate transformation in the template case though.

clang/include/clang/AST/ExprCXX.h

4746

I think a comment here like // The syntactic operand written in the code or so would help clarify the distinction between this and the common/ready/suspend/resume family.

I'm just thinking of all the times working on tooling when you're trying to understand how to handle every node type without deep-diving into the semantics of each one :-)

Also maybe group getOperand(), getKeywordLoc(), getBeginLoc(), getEndLoc() together?

clang/lib/Sema/SemaCoroutine.cpp

716

I'm not sure Suspend.get() here is the most appropriate value for Operand.

It's a little abstract, since we're talking about the syntactic operand of an implicit call :-)
But for consistency with the explicit case, I think the operand should be the result of initial_suspend() etc, *before* we call operator co_await.

i.e. the value of Suspend.get() a couple of lines above.

839

This has become confusing I think. The old code was was changing the meaning of the E, because we didn't need the original anymore. Rather than introduce a new variable for the original meaning, I'd prefer to introduce one for the changed meaning:

// line 848
auto *Transformed = E;
if (lookupMember(...)) {
  ...
  Transformed = R.get();
}
buildOperatorCoAwaitCall(..., Transformed);

(if you also want to rename E -> Operand, that makes sense to me too!)

858–859

(aside, this variable name is really unfortunate: I think E here is the awaitable, and Awaitable is the _awaiter_. If this is right, feel free to change it or not...)

866–867

Not really your fault, but having two Exprs Operand and E is terribly confusing in isolation. I think E is Awaiter in standardese. Again feel free to change or not.

clang/lib/Sema/TreeTransform.h

7967–7971

Ah, this doesn't *sound* right - it's going to create duplicate subexprs I think, and we should really try to have the operand still be a subexpr of the commonexpr like in the non-instantiated case.

TransformInitListExpr() is a fairly similar case, and it just discards the semantic form and rebuilds from the syntactic one. TransformPseudoObjectExpr seems to be similar.

I suppose the analogous thing to do here would be to have RebuildCoawaitExpr call BuildUnresolvedCoawaitExpr with the operand only, but this is a large change! (And suggests maybe we should stop building the semantic forms of dependent coawaits entirely, though that probably would regress diagnostics).
Maybe worth giving this a spin anyway?

@rsmith, care to weight in here?

7983

(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.)

[nridge]

Address review comments

[nridge]

clang/lib/Sema/SemaCoroutine.cpp

858–859

I believe you're right, changed it

866–867

Agreed, changed this as well

clang/lib/Sema/TreeTransform.h

7967–7971

I suppose the analogous thing to do here would be to have RebuildCoawaitExpr call BuildUnresolvedCoawaitExpr with the operand only,

I gave this a try.

7977

Note, I copied this comment from here.

7983

Is the implication of this that there are some conditions under which we can reuse the original CoawaitExpr rather than rebuilding it?

[sammccall]

This seems right to me!

clang/lib/Sema/TreeTransform.h

7983

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.

[nridge]

Update on this: I'm working on getting the failing tests to pass. Some of them just need expected AST dumps updated to reflect the new node, but some others suggest the patch actually causes semantic regressions.

[nridge]

Here is a reduced testcase for the semantic regression:

namespace std {
template <class PromiseType = void>
struct coroutine_handle {
  static coroutine_handle from_address(void *) noexcept;
};
template <class Ret, class... Args>
struct coroutine_traits;
} // end of namespace std

template <typename Promise> struct coro {};
template <typename Promise, typename... Ps>
struct std::coroutine_traits<coro<Promise>, Ps...> {
  using promise_type = Promise;
};

struct awaitable {
  bool await_ready() noexcept;
  template <typename F>
  void await_suspend(F) noexcept;
  void await_resume() noexcept;
} a;

struct transform_awaitable {};

struct transform_promise {
  coro<transform_promise> get_return_object();
  awaitable initial_suspend();
  awaitable final_suspend() noexcept;
  awaitable await_transform(transform_awaitable);
  void unhandled_exception();
  void return_void();
};

template <typename T, typename U>
coro<T> await_template(U t) {
  co_await t;
}

template coro<transform_promise> await_template<transform_promise>(transform_awaitable);

Without my patch, this compiles fine. With it, I clang gives the following errors:

coroutines.cpp:35:9: error: no viable conversion from 'awaitable' to 'transform_awaitable'
coro<T> await_template(U t) {
        ^~~~~~~~~~~~~~
coroutines.cpp:39:34: note: in instantiation of function template specialization 'await_template<transform_promise, transform_awaitable>' requested here
template coro<transform_promise> await_template<transform_promise>(transform_awaitable);
                                 ^
coroutines.cpp:23:8: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'awaitable' to 'const transform_awaitable &' for 1st argument
struct transform_awaitable {};
       ^
coroutines.cpp:23:8: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'awaitable' to 'transform_awaitable &&' for 1st argument
struct transform_awaitable {};
       ^
coroutines.cpp:29:48: note: passing argument to parameter here
  awaitable await_transform(transform_awaitable);
                                               ^
coroutines.cpp:35:9: note: call to 'await_transform' implicitly required by 'co_await' here
coro<T> await_template(U t) {
        ^~~~~~~~~~~~~~
1 error generated.

[sammccall]

In D115187#3471003, @nridge wrote:

Without my patch, this compiles fine. With it, I clang gives the following errors:

coroutines.cpp:35:9: error: no viable conversion from 'awaitable' to 'transform_awaitable'
coro<T> await_template(U t) {
        ^~~~~~~~~~~~~~
coroutines.cpp:39:34: note: in instantiation of function template specialization 'await_template<transform_promise, transform_awaitable>' requested here
template coro<transform_promise> await_template<transform_promise>(transform_awaitable);

Poked at this a bit locally.
From the message we see there's a call to BuildUnresolvedCoawaitExpr with an awaitable rather than transform_awaitable, so the await_transform call fails.

It turns out this is the initial_suspend call (CXXMemberCallExpr). We're not supposed to transform initial/final suspends, but the code is now doing so.

This seems to be a consequence of switching from BuildResolvedCoawaitExpr (which does not apply the transform) to BuildUnresolvedCoawaitExpr (which does) during template instantiation.

[nridge]

Handle implicit coawait-exprs properly in RebuildCoawaitExpr, other test fixes

[nridge]

In D115187#3471261, @sammccall wrote:

We're not supposed to transform initial/final suspends, but the code is now doing so.

This seems to be a consequence of switching from BuildResolvedCoawaitExpr (which does not apply the transform) to BuildUnresolvedCoawaitExpr (which does) during template instantiation.

Thanks, this helped me get onto the right track for a fix.

Indeed, we have two different codepaths by which CoawaitExprs are built: explicit ones use BuildUnresolvedCoawaitExpr, while the implicit ones for the suspends use BuildResolvedCoawaitExpr with a bit of extra logic before (to apply operator coawait (but not await_transform) if appropriate).

Thankfully, CoawaitExpr remembers which kind it is in isImplicit(), so it's straightforward to get TreeTransform to do the right thing based on this flag.

[sammccall]

Looks good to me - this still seems hairy and abstract to me so I'd "expect the unexpected" in terms of bot or downstream breakage...

clang/include/clang/Sema/Sema.h

10393

nit: update param names here

clang/lib/Sema/SemaCoroutine.cpp

824

this name is a bit confusing - the result isn't (necessarily) unresolved, it's more like the inputs are resolved.

I don't actually think we should rename it, but maybe add a comment like Attempts to resolve and build a CoAwaitExpr from "raw" inputs, bailing out to DependentCoawaitExpr if needed

(Not caused by this patch, but I feel like the only time to add comments to make this less confusing is when we've been digging into it)

clang/lib/Sema/TreeTransform.h

1476–1494

This is essentially inlining BuildUnresolvedCoawaitExpr, with the await_transform logic dropped, right? I wonder how that compares to adding a bool Transform to that function.

Such a param would make everything less direct, but it also seems possible that the two copies could unexpectedly diverge either now (e.g. could the "placeholder type" logic from BuildUnresolved be relevant here?) or more likely in the future.

Up to you, I don't feel strongly here, it just seems a little surprising.

clang/test/AST/coroutine-locals-cleanup-exp-namespace.cpp

88–89

nit: I think it'd be nice to include the type here for consistency

[nridge]

Address review comments

clang/lib/Sema/TreeTransform.h

1476–1494

This is essentially inlining BuildUnresolvedCoawaitExpr, with the await_transform logic dropped, right?

Not exactly. Rather, it's attempting to replicate the logic here, which is where implicit CoawaitExpr's are built in the first place.

(The only reason I didn't factor out those few lines into a function of their own is that here we already have the UnresolvedLookupExpr for the operator coawait, whereas there it's calling a helper that builds the lookup-expr followed by calling Sema::BuildOperatorCoawaitCall().)

So, precisely because BuildUnresolvedCoawaitExpr does a few extra things like the "placeholder type" logic that the code we're trying to replicate doesn't, I think it's safer not to try and reuse that here.

clang/test/AST/coroutine-locals-cleanup-exp-namespace.cpp

88–89

A full line of example output here is:

CXXBindTemporaryExpr 0x1898248 <col:14, col:18> 'Task' (CXXTemporary 0x1898248)

not sure what type you have in mind besides 'Task', which is already there.

[nridge]

There is one remaining test failure, related to a duplicate diagnostic. I've reduced it to the following:

namespace std {

template <class Ret, class... Args>
struct coroutine_traits {};
} // end of namespace std

struct awaitable {
  bool await_ready() noexcept;
  template <typename F>
  void await_suspend(F) noexcept;
  void await_resume() noexcept;
} a;

struct suspend_always {
  bool await_ready() noexcept { return false; }
  template <typename F>
  void await_suspend(F) noexcept;
  void await_resume() noexcept {}
};

struct yielded_thing { const char *p; short a, b; };

struct promise {
  void get_return_object();
  suspend_always initial_suspend();
  suspend_always final_suspend() noexcept;
  awaitable yield_value(int);
  awaitable yield_value(yielded_thing);
  void return_value(int);
  void unhandled_exception();
};

template <typename... T>
struct std::coroutine_traits<void, T...> { using promise_type = promise; };

namespace std {
template <class PromiseType = void>
struct coroutine_handle {
  static coroutine_handle from_address(void *) noexcept;
};
template <>
struct coroutine_handle<void> {
  template <class PromiseType>
  coroutine_handle(coroutine_handle<PromiseType>) noexcept;
  static coroutine_handle from_address(void *) noexcept;
};
} // namespace std

void yield() {
  co_yield 0;
  co_yield {1e100}; // expected-error {{cannot be narrowed}} expected-note {{explicit cast}} expected-warning {{implicit conversion}} expected-warning {{braces around scalar}}
  co_yield {"foo", __LONG_LONG_MAX__}; // expected-error {{cannot be narrowed}} expected-note {{explicit cast}} expected-warning {{changes value}}
}

The last two lines each trigger an "implicit conversion" warning (among other diagnostics), which is expected, but with my patch the "implicit conversion" warnings appear twice each.

[nridge]

I think the issue is related to this loop in AnalyzeImplicitConversions(), which iterates over Expr::children(), and adds each child to a list of expressions to be checked for implicit conversions.

CoroutineSuspendExpr now has the operand as an extra child, and an implicit conversion in the operand gets diagnosed both when processing the operand, and the common-expr.

I'm guessing this will need an explicit carve-out in AnalyzeImplicitConversions. There is already some special handling of other expressions types there, including at least one whose purpose is to avoid duplicate diagnostics.

[sammccall]

In D115187#3500098, @nridge wrote:

I think the issue is related to this loop in AnalyzeImplicitConversions(), which iterates over Expr::children(), and adds each child to a list of expressions to be checked for implicit conversions.

CoroutineSuspendExpr now has the operand as an extra child, and an implicit conversion in the operand gets diagnosed both when processing the operand, and the common-expr.

I'm guessing this will need an explicit carve-out in AnalyzeImplicitConversions. There is already some special handling of other expressions types there, including at least one whose purpose is to avoid duplicate diagnostics.

Yeah, I agree.
The general issue here is that storing alternate representations that share subexpressions can cause duplicated traversal. I guess having RecursiveASTVisitor do the right thing will cover most cases.

PseudoObjectExpr has this pattern (for objc property access), InitListExpr kind of does this (syntactic vs semantic forms). For both, I don't fully understand the mechanism, and it seems to require hacks in lots of places :-(
FWIW PseudoObjectExpr avoids duplicated diagnostics here by wrapping all its semantic/derived subexpressions inside OpaqueValueExpr, the special case you found... I don't know what the implications of trying to reuse that mechanism would be. The explicit carve-out seems simpler.

clang/lib/Sema/TreeTransform.h

1476–1494

Thank you, that makes a lot of sense.
Could you add to the comment something like:
// This mirrors how the implicit CoAwaitExpr is originally created in Sema::ActOnCoroutineBodyStart?

clang/test/AST/coroutine-locals-cleanup-exp-namespace.cpp

88–89

I'm not sure what I meant either, sorry! Looks good.

[nridge]

Fix last test case and review comment

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Code = R"cpp(
 
     template <[[Fooable]]<int> T>
     void bar(T t);
   )cpp";
   EXPECT_DECLS("ConceptSpecializationExpr",
                {"template <typename T, typename U> concept Fooable = true"});
 }
 
+TEST_F(TargetDeclTest, Coroutine) {
+  Flags.push_back("-std=c++20");
+
+  Code = R"cpp(
+    namespace std::experimental {
+    template <typename, typename...> struct coroutine_traits;
+    template <typename> struct coroutine_handle {
+      template <typename U>
+      coroutine_handle(coroutine_handle<U>&&) noexcept;
+      static coroutine_handle from_address(void* __addr) noexcept;
+    };
+    } // namespace std::experimental
+
+    struct executor {};
+    struct awaitable {};
+    struct awaitable_frame {
+      awaitable get_return_object();
+      void return_void();
+      void unhandled_exception();
+      struct result_t {
+        ~result_t();
+        bool await_ready() const noexcept;
+        void await_suspend(std::experimental::coroutine_handle<void>) noexcept;
+        void await_resume() const noexcept;
+      };
+      result_t initial_suspend() noexcept;
+      result_t final_suspend() noexcept;
+      result_t await_transform(executor) noexcept;
+    };
+
+    namespace std::experimental {
+    template <>
+    struct coroutine_traits<awaitable> {
+      typedef awaitable_frame promise_type;
+    };
+    } // namespace std::experimental
+
+    awaitable foo() {
+      co_await [[executor]]();
+    }
+  )cpp";
+  EXPECT_DECLS("RecordTypeLoc", "struct executor");
+}
+
 TEST_F(TargetDeclTest, FunctionTemplate) {
   Code = R"cpp(
     // Implicit specialization.
     template<typename T> bool foo(T) { return false; };
     bool x = [[foo]](42);
   )cpp";
   EXPECT_DECLS("DeclRefExpr",
                {"template<> bool foo<int>(int)", Rel::TemplateInstantiation},

clang/include/clang/AST/ExprCXX.h

 /// If the coroutine is not suspended, or when it is resumed, the 'resume'
 /// expression is evaluated, and its result is the result of the overall
 /// expression.
 class CoroutineSuspendExpr : public Expr {
   friend class ASTStmtReader;
 
   SourceLocation KeywordLoc;
 
-  enum SubExpr { Common, Ready, Suspend, Resume, Count };
+  enum SubExpr { Operand, Common, Ready, Suspend, Resume, Count };
 
   Stmt *SubExprs[SubExpr::Count];
   OpaqueValueExpr *OpaqueValue = nullptr;
 
 public:
-  CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, Expr *Common,
-                       Expr *Ready, Expr *Suspend, Expr *Resume,
+  CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, Expr *Operand,
+                       Expr *Common, Expr *Ready, Expr *Suspend, Expr *Resume,
                        OpaqueValueExpr *OpaqueValue)
       : Expr(SC, Resume->getType(), Resume->getValueKind(),
              Resume->getObjectKind()),
         KeywordLoc(KeywordLoc), OpaqueValue(OpaqueValue) {
+    SubExprs[SubExpr::Operand] = Operand;
     SubExprs[SubExpr::Common] = Common;
     SubExprs[SubExpr::Ready] = Ready;
     SubExprs[SubExpr::Suspend] = Suspend;
     SubExprs[SubExpr::Resume] = Resume;
     setDependence(computeDependence(this));
   }
 
   CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, QualType Ty,
-                       Expr *Common)
+                       Expr *Operand, Expr *Common)
       : Expr(SC, Ty, VK_PRValue, OK_Ordinary), KeywordLoc(KeywordLoc) {
     assert(Common->isTypeDependent() && Ty->isDependentType() &&
            "wrong constructor for non-dependent co_await/co_yield expression");
+    SubExprs[SubExpr::Operand] = Operand;
     SubExprs[SubExpr::Common] = Common;
     SubExprs[SubExpr::Ready] = nullptr;
     SubExprs[SubExpr::Suspend] = nullptr;
     SubExprs[SubExpr::Resume] = nullptr;
     setDependence(computeDependence(this));
   }
 
   CoroutineSuspendExpr(StmtClass SC, EmptyShell Empty) : Expr(SC, Empty) {
+    SubExprs[SubExpr::Operand] = nullptr;
     SubExprs[SubExpr::Common] = nullptr;
     SubExprs[SubExpr::Ready] = nullptr;
     SubExprs[SubExpr::Suspend] = nullptr;
     SubExprs[SubExpr::Resume] = nullptr;
   }
 
-  SourceLocation getKeywordLoc() const { return KeywordLoc; }
-
   Expr *getCommonExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Common]);
   }
 
   /// getOpaqueValue - Return the opaque value placeholder.
   OpaqueValueExpr *getOpaqueValue() const { return OpaqueValue; }
 
   Expr *getReadyExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Ready]);
   }
 
   Expr *getSuspendExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Suspend]);
   }
 
   Expr *getResumeExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Resume]);
   }
 
+  // The syntactic operand written in the code
+  Expr *getOperand() const {
+    return static_cast<Expr *>(SubExprs[SubExpr::Operand]);
+  }
+
+  SourceLocation getKeywordLoc() const { return KeywordLoc; }
+
   SourceLocation getBeginLoc() const LLVM_READONLY { return KeywordLoc; }
 
   SourceLocation getEndLoc() const LLVM_READONLY {
-    return getCommonExpr()->getEndLoc();
+    return getOperand()->getEndLoc();
   }
 
   child_range children() {
     return child_range(SubExprs, SubExprs + SubExpr::Count);
   }
 
   const_child_range children() const {
     return const_child_range(SubExprs, SubExprs + SubExpr::Count);
   }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == CoawaitExprClass ||
            T->getStmtClass() == CoyieldExprClass;
   }
 };
 
 /// Represents a 'co_await' expression.
 class CoawaitExpr : public CoroutineSuspendExpr {
   friend class ASTStmtReader;
 
 public:
-  CoawaitExpr(SourceLocation CoawaitLoc, Expr *Operand, Expr *Ready,
-              Expr *Suspend, Expr *Resume, OpaqueValueExpr *OpaqueValue,
-              bool IsImplicit = false)
-      : CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Operand, Ready,
-                             Suspend, Resume, OpaqueValue) {
+  CoawaitExpr(SourceLocation CoawaitLoc, Expr *Operand, Expr *Common,
+              Expr *Ready, Expr *Suspend, Expr *Resume,
+              OpaqueValueExpr *OpaqueValue, bool IsImplicit = false)
+      : CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Operand, Common,
+                             Ready, Suspend, Resume, OpaqueValue) {
     CoawaitBits.IsImplicit = IsImplicit;
   }
 
   CoawaitExpr(SourceLocation CoawaitLoc, QualType Ty, Expr *Operand,
-              bool IsImplicit = false)
-      : CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Ty, Operand) {
+              Expr *Common, bool IsImplicit = false)
+      : CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Ty, Operand,
+                             Common) {
     CoawaitBits.IsImplicit = IsImplicit;
   }
 
   CoawaitExpr(EmptyShell Empty)
       : CoroutineSuspendExpr(CoawaitExprClass, Empty) {}
 
-  Expr *getOperand() const {
-    // FIXME: Dig out the actual operand or store it.
-    return getCommonExpr();
-  }
-
   bool isImplicit() const { return CoawaitBits.IsImplicit; }
   void setIsImplicit(bool value = true) { CoawaitBits.IsImplicit = value; }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == CoawaitExprClass;
   }
 };
 
public:
   }
 };
 
 /// Represents a 'co_yield' expression.
 class CoyieldExpr : public CoroutineSuspendExpr {
   friend class ASTStmtReader;
 
 public:
-  CoyieldExpr(SourceLocation CoyieldLoc, Expr *Operand, Expr *Ready,
-              Expr *Suspend, Expr *Resume, OpaqueValueExpr *OpaqueValue)
-      : CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Operand, Ready,
-                             Suspend, Resume, OpaqueValue) {}
-  CoyieldExpr(SourceLocation CoyieldLoc, QualType Ty, Expr *Operand)
-      : CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Ty, Operand) {}
+  CoyieldExpr(SourceLocation CoyieldLoc, Expr *Operand, Expr *Common,
+              Expr *Ready, Expr *Suspend, Expr *Resume,
+              OpaqueValueExpr *OpaqueValue)
+      : CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Operand, Common,
+                             Ready, Suspend, Resume, OpaqueValue) {}
+  CoyieldExpr(SourceLocation CoyieldLoc, QualType Ty, Expr *Operand,
+              Expr *Common)
+      : CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Ty, Operand,
+                             Common) {}
   CoyieldExpr(EmptyShell Empty)
       : CoroutineSuspendExpr(CoyieldExprClass, Empty) {}
 
-  Expr *getOperand() const {
-    // FIXME: Dig out the actual operand or store it.
-    return getCommonExpr();
-  }
-
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == CoyieldExprClass;
   }
 };
 
 /// Represents a C++2a __builtin_bit_cast(T, v) expression. Used to implement
 /// std::bit_cast. These can sometimes be evaluated as part of a constant
 /// expression, but otherwise CodeGen to a simple memcpy in general.

clang/include/clang/Sema/Sema.h

   // C++ Coroutines TS
   //
   bool ActOnCoroutineBodyStart(Scope *S, SourceLocation KwLoc,
                                StringRef Keyword);
   ExprResult ActOnCoawaitExpr(Scope *S, SourceLocation KwLoc, Expr *E);
   ExprResult ActOnCoyieldExpr(Scope *S, SourceLocation KwLoc, Expr *E);
   StmtResult ActOnCoreturnStmt(Scope *S, SourceLocation KwLoc, Expr *E);
 
-  ExprResult BuildResolvedCoawaitExpr(SourceLocation KwLoc, Expr *E,
-                                      bool IsImplicit = false);
-  ExprResult BuildUnresolvedCoawaitExpr(SourceLocation KwLoc, Expr *E,
-                                        UnresolvedLookupExpr* Lookup);
+  ExprResult BuildOperatorCoawaitLookupExpr(Scope *S, SourceLocation Loc);
+  ExprResult BuildOperatorCoawaitCall(SourceLocation Loc, Expr *E,
+                                      UnresolvedLookupExpr *Lookup);
+  ExprResult BuildResolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
+                                      Expr *Awaiter, bool IsImplicit = false);
+  ExprResult BuildUnresolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
+                                        UnresolvedLookupExpr *Lookup);
   ExprResult BuildCoyieldExpr(SourceLocation KwLoc, Expr *E);
   StmtResult BuildCoreturnStmt(SourceLocation KwLoc, Expr *E,
                                bool IsImplicit = false);
   StmtResult BuildCoroutineBodyStmt(CoroutineBodyStmt::CtorArgs);
   bool buildCoroutineParameterMoves(SourceLocation Loc);
   VarDecl *buildCoroutinePromise(SourceLocation Loc);
   void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body);
   /// Lookup 'coroutine_traits' in std namespace and std::experimental

clang/lib/Sema/SemaChecking.cpp

static void AnalyzeImplicitConversions(
   CC = E->getExprLoc();
   BinaryOperator *BO = dyn_cast<BinaryOperator>(E);
   bool IsLogicalAndOperator = BO && BO->getOpcode() == BO_LAnd;
   for (Stmt *SubStmt : E->children()) {
     Expr *ChildExpr = dyn_cast_or_null<Expr>(SubStmt);
     if (!ChildExpr)
       continue;
 
+    if (auto *CSE = dyn_cast<CoroutineSuspendExpr>(E))
+      if (ChildExpr == CSE->getOperand())
+        // Do not recurse over a CoroutineSuspendExpr's operand.
+        // The operand is also a subexpression of getCommonExpr(), and
+        // recursing into it directly would produce duplicate diagnostics.
+        continue;
+
     if (IsLogicalAndOperator &&
         isa<StringLiteral>(ChildExpr->IgnoreParenImpCasts()))
       // Ignore checking string literals that are in logical and operators.
       // This is a common pattern for asserts.
       continue;
     WorkList.push_back({ChildExpr, CC, IsListInit});
   }
 

clang/lib/Sema/SemaCoroutine.cpp

static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
   // The parameter-declaration-clause of the coroutine shall not terminate with
   // an ellipsis that is not part of a parameter-declaration.
   if (FD->isVariadic())
     DiagInvalid(DiagVarargs);
 
   return !Diagnosed;
 }
 
-static ExprResult buildOperatorCoawaitLookupExpr(Sema &SemaRef, Scope *S,
-                                                 SourceLocation Loc) {
-  DeclarationName OpName =
-      SemaRef.Context.DeclarationNames.getCXXOperatorName(OO_Coawait);
-  LookupResult Operators(SemaRef, OpName, SourceLocation(),
-                         Sema::LookupOperatorName);
-  SemaRef.LookupName(Operators, S);
-
-  assert(!Operators.isAmbiguous() && "Operator lookup cannot be ambiguous");
-  const auto &Functions = Operators.asUnresolvedSet();
-  bool IsOverloaded =
-      Functions.size() > 1 ||
-      (Functions.size() == 1 && isa<FunctionTemplateDecl>(*Functions.begin()));
-  Expr *CoawaitOp = UnresolvedLookupExpr::Create(
-      SemaRef.Context, /*NamingClass*/ nullptr, NestedNameSpecifierLoc(),
-      DeclarationNameInfo(OpName, Loc), /*RequiresADL*/ true, IsOverloaded,
-      Functions.begin(), Functions.end());
-  assert(CoawaitOp);
-  return CoawaitOp;
-}
-
 /// Build a call to 'operator co_await' if there is a suitable operator for
 /// the given expression.
-static ExprResult buildOperatorCoawaitCall(Sema &SemaRef, SourceLocation Loc,
-                                           Expr *E,
-                                           UnresolvedLookupExpr *Lookup) {
+ExprResult Sema::BuildOperatorCoawaitCall(SourceLocation Loc, Expr *E,
   UnresolvedSet<16> Functions;
   Functions.append(Lookup->decls_begin(), Lookup->decls_end());
-  return SemaRef.CreateOverloadedUnaryOp(Loc, UO_Coawait, Functions, E);
+  return CreateOverloadedUnaryOp(Loc, UO_Coawait, Functions, E);
 }
 
 static ExprResult buildOperatorCoawaitCall(Sema &SemaRef, Scope *S,
                                            SourceLocation Loc, Expr *E) {
-  ExprResult R = buildOperatorCoawaitLookupExpr(SemaRef, S, Loc);
+  ExprResult R = SemaRef.BuildOperatorCoawaitLookupExpr(S, Loc);
   if (R.isInvalid())
     return ExprError();
-  return buildOperatorCoawaitCall(SemaRef, Loc, E,
+  return SemaRef.BuildOperatorCoawaitCall(Loc, E,
                                           cast<UnresolvedLookupExpr>(R.get()));
 }
 
 static ExprResult buildCoroutineHandle(Sema &S, QualType PromiseType,
                                        SourceLocation Loc) {
   QualType CoroHandleType = lookupCoroutineHandleType(S, PromiseType, Loc);
   if (CoroHandleType.isNull())
     return ExprError();
 
if (!ScopeInfo->NeedsCoroutineSuspends)
     return true;
 
   ScopeInfo->setNeedsCoroutineSuspends(false);
 
   auto *Fn = cast<FunctionDecl>(CurContext);
   SourceLocation Loc = Fn->getLocation();
   // Build the initial suspend point
   auto buildSuspends = [&](StringRef Name) mutable -> StmtResult {
-    ExprResult Suspend =
+    ExprResult Operand =
         buildPromiseCall(*this, ScopeInfo->CoroutinePromise, Loc, Name, None);
-    if (Suspend.isInvalid())
+    if (Operand.isInvalid())
       return StmtError();
-    Suspend = buildOperatorCoawaitCall(*this, SC, Loc, Suspend.get());
+    ExprResult Suspend =
+        buildOperatorCoawaitCall(*this, SC, Loc, Operand.get());
     if (Suspend.isInvalid())
       return StmtError();
-    Suspend = BuildResolvedCoawaitExpr(Loc, Suspend.get(),
+    Suspend = BuildResolvedCoawaitExpr(Loc, Operand.get(), Suspend.get(),
                                        /*IsImplicit*/ true);
     Suspend = ActOnFinishFullExpr(Suspend.get(), /*DiscardedValue*/ false);
     if (Suspend.isInvalid()) {
       Diag(Loc, diag::note_coroutine_promise_suspend_implicitly_required)
           << ((Name == "initial_suspend") ? 0 : 1);
       Diag(KWLoc, diag::note_declared_coroutine_here) << Keyword;
       return StmtError();
     }
ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) {
 
   checkSuspensionContext(*this, Loc, "co_await");
 
   if (E->hasPlaceholderType()) {
     ExprResult R = CheckPlaceholderExpr(E);
     if (R.isInvalid()) return ExprError();
     E = R.get();
   }
-  ExprResult Lookup = buildOperatorCoawaitLookupExpr(*this, S, Loc);
+  ExprResult Lookup = BuildOperatorCoawaitLookupExpr(S, Loc);
   if (Lookup.isInvalid())
     return ExprError();
   return BuildUnresolvedCoawaitExpr(Loc, E,
                                    cast<UnresolvedLookupExpr>(Lookup.get()));
 }
 
-ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *E,
+ExprResult Sema::BuildOperatorCoawaitLookupExpr(Scope *S, SourceLocation Loc) {
+  DeclarationName OpName =
+      Context.DeclarationNames.getCXXOperatorName(OO_Coawait);
+  LookupResult Operators(*this, OpName, SourceLocation(),
+                         Sema::LookupOperatorName);
+  LookupName(Operators, S);
+
+  assert(!Operators.isAmbiguous() && "Operator lookup cannot be ambiguous");
+  const auto &Functions = Operators.asUnresolvedSet();
+  bool IsOverloaded =
+      Functions.size() > 1 ||
+      (Functions.size() == 1 && isa<FunctionTemplateDecl>(*Functions.begin()));
+  Expr *CoawaitOp = UnresolvedLookupExpr::Create(
+      Context, /*NamingClass*/ nullptr, NestedNameSpecifierLoc(),
+      DeclarationNameInfo(OpName, Loc), /*RequiresADL*/ true, IsOverloaded,
+      Functions.begin(), Functions.end());
+  assert(CoawaitOp);
+  return CoawaitOp;
+}
+
+// Attempts to resolve and build a CoawaitExpr from "raw" inputs, bailing out to
+// DependentCoawaitExpr if needed.
+ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand,
                                             UnresolvedLookupExpr *Lookup) {
   auto *FSI = checkCoroutineContext(*this, Loc, "co_await");
   if (!FSI)
     return ExprError();
 
-  if (E->hasPlaceholderType()) {
-    ExprResult R = CheckPlaceholderExpr(E);
+  if (Operand->hasPlaceholderType()) {
+    ExprResult R = CheckPlaceholderExpr(Operand);
     if (R.isInvalid())
       return ExprError();
-    E = R.get();
+    Operand = R.get();
   }
 
   auto *Promise = FSI->CoroutinePromise;
   if (Promise->getType()->isDependentType()) {
-    Expr *Res =
-        new (Context) DependentCoawaitExpr(Loc, Context.DependentTy, E, Lookup);
+    Expr *Res = new (Context)
+        DependentCoawaitExpr(Loc, Context.DependentTy, Operand, Lookup);
     return Res;
   }
 
   auto *RD = Promise->getType()->getAsCXXRecordDecl();
+  auto *Transformed = Operand;
   if (lookupMember(*this, "await_transform", RD, Loc)) {
-    ExprResult R = buildPromiseCall(*this, Promise, Loc, "await_transform", E);
+    ExprResult R =
+        buildPromiseCall(*this, Promise, Loc, "await_transform", Operand);
     if (R.isInvalid()) {
       Diag(Loc,
            diag::note_coroutine_promise_implicit_await_transform_required_here)
-          << E->getSourceRange();
+          << Operand->getSourceRange();
       return ExprError();
     }
-    E = R.get();
+    Transformed = R.get();
   }
-  ExprResult Awaitable = buildOperatorCoawaitCall(*this, Loc, E, Lookup);
-  if (Awaitable.isInvalid())
+  ExprResult Awaiter = BuildOperatorCoawaitCall(Loc, Transformed, Lookup);
+  if (Awaiter.isInvalid())
     return ExprError();
 
-  return BuildResolvedCoawaitExpr(Loc, Awaitable.get());
+  return BuildResolvedCoawaitExpr(Loc, Operand, Awaiter.get());
 }
 
-ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *E,
-                                  bool IsImplicit) {
+ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *Operand,
+                                          Expr *Awaiter, bool IsImplicit) {
   auto *Coroutine = checkCoroutineContext(*this, Loc, "co_await", IsImplicit);
   if (!Coroutine)
     return ExprError();
 
-  if (E->hasPlaceholderType()) {
-    ExprResult R = CheckPlaceholderExpr(E);
+  if (Awaiter->hasPlaceholderType()) {
+    ExprResult R = CheckPlaceholderExpr(Awaiter);
     if (R.isInvalid()) return ExprError();
-    E = R.get();
+    Awaiter = R.get();
   }
 
-  if (E->getType()->isDependentType()) {
+  if (Awaiter->getType()->isDependentType()) {
     Expr *Res = new (Context)
-        CoawaitExpr(Loc, Context.DependentTy, E, IsImplicit);
+        CoawaitExpr(Loc, Context.DependentTy, Operand, Awaiter, IsImplicit);
     return Res;
   }
 
   // If the expression is a temporary, materialize it as an lvalue so that we
   // can use it multiple times.
-  if (E->isPRValue())
-    E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
+  if (Awaiter->isPRValue())
+    Awaiter = CreateMaterializeTemporaryExpr(Awaiter->getType(), Awaiter, true);
 
   // The location of the `co_await` token cannot be used when constructing
   // the member call expressions since it's before the location of `Expr`, which
   // is used as the start of the member call expression.
-  SourceLocation CallLoc = E->getExprLoc();
+  SourceLocation CallLoc = Awaiter->getExprLoc();
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS = buildCoawaitCalls(
-      *this, Coroutine->CoroutinePromise, CallLoc, E);
+  ReadySuspendResumeResult RSS =
+      buildCoawaitCalls(*this, Coroutine->CoroutinePromise, CallLoc, Awaiter);
   if (RSS.IsInvalid)
     return ExprError();
 
-  Expr *Res =
-      new (Context) CoawaitExpr(Loc, E, RSS.Results[0], RSS.Results[1],
-                                RSS.Results[2], RSS.OpaqueValue, IsImplicit);
+  Expr *Res = new (Context)
+      CoawaitExpr(Loc, Operand, Awaiter, RSS.Results[0], RSS.Results[1],
 
   return Res;
 }
 
 ExprResult Sema::ActOnCoyieldExpr(Scope *S, SourceLocation Loc, Expr *E) {
   if (!ActOnCoroutineBodyStart(S, Loc, "co_yield")) {
     CorrectDelayedTyposInExpr(E);
     return ExprError();
if (!Coroutine)
     return ExprError();
 
   if (E->hasPlaceholderType()) {
     ExprResult R = CheckPlaceholderExpr(E);
     if (R.isInvalid()) return ExprError();
     E = R.get();
   }
 
+  Expr *Operand = E;
+
   if (E->getType()->isDependentType()) {
-    Expr *Res = new (Context) CoyieldExpr(Loc, Context.DependentTy, E);
+    Expr *Res = new (Context) CoyieldExpr(Loc, Context.DependentTy, Operand, E);
     return Res;
   }
 
   // If the expression is a temporary, materialize it as an lvalue so that we
   // can use it multiple times.
   if (E->isPRValue())
     E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
   ReadySuspendResumeResult RSS = buildCoawaitCalls(
       *this, Coroutine->CoroutinePromise, Loc, E);
   if (RSS.IsInvalid)
     return ExprError();
 
   Expr *Res =
-      new (Context) CoyieldExpr(Loc, E, RSS.Results[0], RSS.Results[1],
+      new (Context) CoyieldExpr(Loc, Operand, E, RSS.Results[0], RSS.Results[1],
                                 RSS.Results[2], RSS.OpaqueValue);
 
   return Res;
 }
 
 StmtResult Sema::ActOnCoreturnStmt(Scope *S, SourceLocation Loc, Expr *E) {
   if (!ActOnCoroutineBodyStart(S, Loc, "co_return")) {
     CorrectDelayedTyposInExpr(E);

clang/lib/Sema/TreeTransform.h

StmtResult RebuildCoreturnStmt(SourceLocation CoreturnLoc, Expr *Result,
                                  bool IsImplicit) {
     return getSema().BuildCoreturnStmt(CoreturnLoc, Result, IsImplicit);
   }
 
   /// Build a new co_await expression.
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildCoawaitExpr(SourceLocation CoawaitLoc, Expr *Result,
+  ExprResult RebuildCoawaitExpr(SourceLocation CoawaitLoc, Expr *Operand,
+                                UnresolvedLookupExpr *OpCoawaitLookup,
                                 bool IsImplicit) {
-    return getSema().BuildResolvedCoawaitExpr(CoawaitLoc, Result, IsImplicit);
+    // This function rebuilds a coawait-expr given its operator.
+    // For an explicit coawait-expr, the rebuild involves the full set
+    // of transformations performed by BuildUnresolvedCoawaitExpr(),
+    // including calling await_transform().
+    // For an implicit coawait-expr, we need to rebuild the "operator
+    // coawait" but not await_transform(), so use BuildResolvedCoawaitExpr().
+    // This mirrors how the implicit CoawaitExpr is originally created
+    // in Sema::ActOnCoroutineBodyStart().
+    if (IsImplicit) {
+      ExprResult Suspend = getSema().BuildOperatorCoawaitCall(
+          CoawaitLoc, Operand, OpCoawaitLookup);
+      if (Suspend.isInvalid())
+        return ExprError();
+      return getSema().BuildResolvedCoawaitExpr(CoawaitLoc, Operand,
+                                                Suspend.get(), true);
+    }
+
+    return getSema().BuildUnresolvedCoawaitExpr(CoawaitLoc, Operand,
+                                                OpCoawaitLookup);
   }
 
   /// Build a new co_await expression.
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildDependentCoawaitExpr(SourceLocation CoawaitLoc,
                                          Expr *Result,
if (Result.isInvalid())
     return StmtError();
 
   // Always rebuild; we don't know if this needs to be injected into a new
   // context or if the promise type has changed.
   return getDerived().RebuildCoreturnStmt(S->getKeywordLoc(), Result.get(),
                                           S->isImplicit());
 }
 
-template<typename Derived>
-ExprResult
-TreeTransform<Derived>::TransformCoawaitExpr(CoawaitExpr *E) {
-  ExprResult Result = getDerived().TransformInitializer(E->getOperand(),
-                                                        /*NotCopyInit*/false);
-  if (Result.isInvalid())
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformCoawaitExpr(CoawaitExpr *E) {
+  ExprResult Operand = getDerived().TransformInitializer(E->getOperand(),
+                                                         /*NotCopyInit*/ false);
+  if (Operand.isInvalid())
     return ExprError();
 
+  // Rebuild the common-expr from the operand rather than transforming it
+  // separately.
+
+  // FIXME: getCurScope() should not be used during template instantiation.
+  // We should pick up the set of unqualified lookup results for operator
+  // co_await during the initial parse.
+  ExprResult Lookup = getSema().BuildOperatorCoawaitLookupExpr(
+      getSema().getCurScope(), E->getKeywordLoc());
+
   // Always rebuild; we don't know if this needs to be injected into a new
   // context or if the promise type has changed.
-  return getDerived().RebuildCoawaitExpr(E->getKeywordLoc(), Result.get(),
-                                         E->isImplicit());
+  return getDerived().RebuildCoawaitExpr(
+      E->getKeywordLoc(), Operand.get(),
+      cast<UnresolvedLookupExpr>(Lookup.get()), E->isImplicit());
 }
 
 template <typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformDependentCoawaitExpr(DependentCoawaitExpr *E) {
   ExprResult OperandResult = getDerived().TransformInitializer(E->getOperand(),
                                                         /*NotCopyInit*/ false);
   if (OperandResult.isInvalid())

clang/test/AST/coroutine-locals-cleanup-exp-namespace.cpp

Task bar() {
   }
 }
 
 // CHECK-LABEL: FunctionDecl {{.*}} bar 'Task ()'
 // CHECK:         SwitchStmt
 // CHECK:           CaseStmt
 // CHECK:             ExprWithCleanups {{.*}} 'void'
 // CHECK-NEXT:          CoawaitExpr
-// CHECK-NEXT:            MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
+// CHECK-NEXT:            CXXBindTemporaryExpr {{.*}} 'Task' (CXXTemporary {{.*}})
+// CHECK:                 MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
 // CHECK:                 ExprWithCleanups {{.*}} 'bool'
 // CHECK-NEXT:              CXXMemberCallExpr {{.*}} 'bool'
 // CHECK-NEXT:                MemberExpr {{.*}} .await_ready
 // CHECK:                 CallExpr {{.*}} 'void'
 // CHECK-NEXT:              ImplicitCastExpr {{.*}} 'void (*)(void *)'
 // CHECK-NEXT:                DeclRefExpr {{.*}} '__builtin_coro_resume' 'void (void *)'
 // CHECK-NEXT:              ExprWithCleanups {{.*}} 'void *'
 
 // CHECK:           CaseStmt
 // CHECK:             ExprWithCleanups {{.*}} 'void'
 // CHECK-NEXT:          CoawaitExpr
-// CHECK-NEXT:            MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
+// CHECK-NEXT:            CXXBindTemporaryExpr {{.*}} 'Task' (CXXTemporary {{.*}})
+// CHECK:                 MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
 // CHECK:                 ExprWithCleanups {{.*}} 'bool'
 // CHECK-NEXT:              CXXMemberCallExpr {{.*}} 'bool'
 // CHECK-NEXT:                MemberExpr {{.*}} .await_ready
 // CHECK:                 CallExpr {{.*}} 'void'
 // CHECK-NEXT:              ImplicitCastExpr {{.*}} 'void (*)(void *)'
 // CHECK-NEXT:                DeclRefExpr {{.*}} '__builtin_coro_resume' 'void (void *)'
 // CHECK-NEXT:              ExprWithCleanups {{.*}} 'void *'

clang/test/AST/coroutine-locals-cleanup.cpp

Task bar() {
   }
 }
 
 // CHECK-LABEL: FunctionDecl {{.*}} bar 'Task ()'
 // CHECK:         SwitchStmt
 // CHECK:           CaseStmt
 // CHECK:             ExprWithCleanups {{.*}} 'void'
 // CHECK-NEXT:          CoawaitExpr
-// CHECK-NEXT:            MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
+// CHECK-NEXT:            CXXBindTemporaryExpr {{.*}} 'Task' (CXXTemporary {{.*}})
+// CHECK:                 MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
 // CHECK:                 ExprWithCleanups {{.*}} 'bool'
 // CHECK-NEXT:              CXXMemberCallExpr {{.*}} 'bool'
 // CHECK-NEXT:                MemberExpr {{.*}} .await_ready
 // CHECK:                 CallExpr {{.*}} 'void'
 // CHECK-NEXT:              ImplicitCastExpr {{.*}} 'void (*)(void *)'
 // CHECK-NEXT:                DeclRefExpr {{.*}} '__builtin_coro_resume' 'void (void *)'
 // CHECK-NEXT:              ExprWithCleanups {{.*}} 'void *'
 
 // CHECK:           CaseStmt
 // CHECK:             ExprWithCleanups {{.*}} 'void'
 // CHECK-NEXT:          CoawaitExpr
-// CHECK-NEXT:            MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
+// CHECK-NEXT:            CXXBindTemporaryExpr {{.*}} 'Task' (CXXTemporary {{.*}})
+// CHECK:                 MaterializeTemporaryExpr {{.*}} 'Task::Awaiter':'Task::Awaiter'
 // CHECK:                 ExprWithCleanups {{.*}} 'bool'
 // CHECK-NEXT:              CXXMemberCallExpr {{.*}} 'bool'
 // CHECK-NEXT:                MemberExpr {{.*}} .await_ready
 // CHECK:                 CallExpr {{.*}} 'void'
 // CHECK-NEXT:              ImplicitCastExpr {{.*}} 'void (*)(void *)'
 // CHECK-NEXT:                DeclRefExpr {{.*}} '__builtin_coro_resume' 'void (void *)'
 // CHECK-NEXT:              ExprWithCleanups {{.*}} 'void *'

clang/test/AST/coroutine-source-location-crash-exp-namespace.cpp

 };
 
 // {{0x[0-9a-fA-F]+}} <line:[[@LINE+1]]:1, col:36>
 // CHECK-LABEL: FunctionDecl {{.*}} f 'coro_t (int)'
 coro_t f(int n) {
   A a{};
   // CHECK: CoawaitExpr {{0x[0-9a-fA-F]+}} <col:3, col:12>
   // CHECK-NEXT: DeclRefExpr {{0x[0-9a-fA-F]+}} <col:12>
+  // CHECK-NEXT: DeclRefExpr {{0x[0-9a-fA-F]+}} <col:12>
   // CHECK-NEXT: CXXMemberCallExpr {{0x[0-9a-fA-F]+}} <col:12>
   // CHECK-NEXT: MemberExpr {{0x[0-9a-fA-F]+}} <col:12>
   co_await a;
 }

clang/test/AST/coroutine-source-location-crash.cpp

 };
 
 // {{0x[0-9a-fA-F]+}} <line:[[@LINE+1]]:1, col:36>
 // CHECK-LABEL: FunctionDecl {{.*}} f 'coro_t (int)'
 coro_t f(int n) {
   A a{};
   // CHECK: CoawaitExpr {{0x[0-9a-fA-F]+}} <col:3, col:12>
   // CHECK-NEXT: DeclRefExpr {{0x[0-9a-fA-F]+}} <col:12>
+  // CHECK-NEXT: DeclRefExpr {{0x[0-9a-fA-F]+}} <col:12>
   // CHECK-NEXT: CXXMemberCallExpr {{0x[0-9a-fA-F]+}} <col:12>
   // CHECK-NEXT: MemberExpr {{0x[0-9a-fA-F]+}} <col:12>
   co_await a;
 }

clang/test/SemaCXX/co_await-ast.cpp

+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -ast-dump -ast-dump-filter=foo %s | FileCheck %s --strict-whitespace
+
+namespace std {
+template <typename, typename...> struct coroutine_traits;
+template <typename> struct coroutine_handle {
+  template <typename U>
+  coroutine_handle(coroutine_handle<U> &&) noexcept;
+  static coroutine_handle from_address(void *__addr) noexcept;
+};
+} // namespace std
+
+struct executor {};
+struct awaitable {};
+struct awaitable_frame {
+  awaitable get_return_object();
+  void return_void();
+  void unhandled_exception();
+  struct result_t {
+    ~result_t();
+    bool await_ready() const noexcept;
+    void await_suspend(std::coroutine_handle<void>) noexcept;
+    void await_resume() const noexcept;
+  };
+  result_t initial_suspend() noexcept;
+  result_t final_suspend() noexcept;
+  result_t await_transform(executor) noexcept;
+};
+
+namespace std {
+template <>
+struct coroutine_traits<awaitable> {
+  typedef awaitable_frame promise_type;
+};
+} // namespace std
+
+awaitable foo() {
+  co_await executor();
+}
+
+// Check that CoawaitExpr contains the correct subexpressions, including
+// the operand expression as written in the source.
+
+// CHECK-LABEL: Dumping foo:
+// CHECK: FunctionDecl {{.*}} foo 'awaitable ()'
+// CHECK: `-CoroutineBodyStmt {{.*}}
+// CHECK:   |-CompoundStmt {{.*}}
+// CHECK:   | `-ExprWithCleanups {{.*}} 'void'
+// CHECK:   |   `-CoawaitExpr {{.*}} 'void'
+// CHECK:   |     |-CXXTemporaryObjectExpr {{.*}} 'executor' 'void () noexcept' zeroing
+// CHECK:   |     |-MaterializeTemporaryExpr {{.*}} 'awaitable_frame::result_t' lvalue
+// CHECK:   |     | `-CXXBindTemporaryExpr {{.*}} 'awaitable_frame::result_t' (CXXTemporary {{.*}})
+// CHECK:   |     |   `-CXXMemberCallExpr {{.*}} 'awaitable_frame::result_t'
+// CHECK:   |     |     |-MemberExpr {{.*}} '<bound member function type>' .await_transform {{.*}}
+// CHECK:   |     |     | `-DeclRefExpr {{.*}} 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame' lvalue Var {{.*}} '__promise' 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame'
+// CHECK:   |     |     `-CXXTemporaryObjectExpr {{.*}} 'executor' 'void () noexcept' zeroing
+// CHECK:   |     |-ExprWithCleanups {{.*}} 'bool'
+// CHECK:   |     | `-CXXMemberCallExpr {{.*}} 'bool'
+// CHECK:   |     |   `-MemberExpr {{.*}} '<bound member function type>' .await_ready {{.*}}
+// CHECK:   |     |     `-ImplicitCastExpr {{.*}} 'const awaitable_frame::result_t' lvalue <NoOp>
+// CHECK:   |     |       `-OpaqueValueExpr {{.*}} 'awaitable_frame::result_t' lvalue
+// CHECK:   |     |         `-MaterializeTemporaryExpr {{.*}} 'awaitable_frame::result_t' lvalue
+// CHECK:   |     |           `-CXXBindTemporaryExpr {{.*}} 'awaitable_frame::result_t' (CXXTemporary {{.*}})
+// CHECK:   |     |             `-CXXMemberCallExpr {{.*}} 'awaitable_frame::result_t'
+// CHECK:   |     |               |-MemberExpr {{.*}} '<bound member function type>' .await_transform {{.*}}
+// CHECK:   |     |               | `-DeclRefExpr {{.*}} 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame' lvalue Var {{.*}} '__promise' 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame'
+// CHECK:   |     |               `-CXXTemporaryObjectExpr {{.*}} 'executor' 'void () noexcept' zeroing
+// CHECK:   |     |-ExprWithCleanups {{.*}} 'void'
+// CHECK:   |     | `-CXXMemberCallExpr {{.*}} 'void'
+// CHECK:   |     |   |-MemberExpr {{.*}} '<bound member function type>' .await_suspend {{.*}}
+// CHECK:   |     |   | `-OpaqueValueExpr {{.*}} 'awaitable_frame::result_t' lvalue
+// CHECK:   |     |   |   `-MaterializeTemporaryExpr {{.*}} 'awaitable_frame::result_t' lvalue
+// CHECK:   |     |   |     `-CXXBindTemporaryExpr {{.*}} 'awaitable_frame::result_t' (CXXTemporary {{.*}})
+// CHECK:   |     |   |       `-CXXMemberCallExpr {{.*}} 'awaitable_frame::result_t'
+// CHECK:   |     |   |         |-MemberExpr {{.*}} '<bound member function type>' .await_transform {{.*}}
+// CHECK:   |     |   |         | `-DeclRefExpr {{.*}} 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame' lvalue Var {{.*}} '__promise' 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame'
+// CHECK:   |     |   |         `-CXXTemporaryObjectExpr {{.*}} 'executor' 'void () noexcept' zeroing
+// CHECK:   |     |   `-ImplicitCastExpr {{.*}} 'std::coroutine_handle<void>':'std::coroutine_handle<void>' <ConstructorConversion>
+// CHECK:   |     |     `-CXXConstructExpr {{.*}} 'std::coroutine_handle<void>':'std::coroutine_handle<void>' 'void (coroutine_handle<awaitable_frame> &&) noexcept'
+// CHECK:   |     |       `-MaterializeTemporaryExpr {{.*}} 'std::coroutine_handle<awaitable_frame>' xvalue
+// CHECK:   |     |         `-CallExpr {{.*}} 'std::coroutine_handle<awaitable_frame>'
+// CHECK:   |     |           |-ImplicitCastExpr {{.*}} 'std::coroutine_handle<awaitable_frame> (*)(void *) noexcept' <FunctionToPointerDecay>
+// CHECK:   |     |           | `-DeclRefExpr {{.*}} 'std::coroutine_handle<awaitable_frame> (void *) noexcept' lvalue CXXMethod {{.*}} 'from_address' 'std::coroutine_handle<awaitable_frame> (void *) noexcept'
+// CHECK:   |     |           `-CallExpr {{.*}} 'void *'
+// CHECK:   |     |             `-ImplicitCastExpr {{.*}} 'void *(*)() noexcept' <FunctionToPointerDecay>
+// CHECK:   |     |               `-DeclRefExpr {{.*}} 'void *() noexcept' lvalue Function {{.*}} '__builtin_coro_frame' 'void *() noexcept'
+// CHECK:   |     `-CXXMemberCallExpr {{.*}} 'void'
+// CHECK:   |       `-MemberExpr {{.*}} '<bound member function type>' .await_resume {{.*}}
+// CHECK:   |         `-ImplicitCastExpr {{.*}} 'const awaitable_frame::result_t' lvalue <NoOp>
+// CHECK:   |           `-OpaqueValueExpr {{.*}} 'awaitable_frame::result_t' lvalue
+// CHECK:   |             `-MaterializeTemporaryExpr {{.*}} 'awaitable_frame::result_t' lvalue
+// CHECK:   |               `-CXXBindTemporaryExpr {{.*}} 'awaitable_frame::result_t' (CXXTemporary {{.*}})
+// CHECK:   |                 `-CXXMemberCallExpr {{.*}} 'awaitable_frame::result_t'
+// CHECK:   |                   |-MemberExpr {{.*}} '<bound member function type>' .await_transform {{.*}}
+// CHECK:   |                   | `-DeclRefExpr {{.*}} 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame' lvalue Var {{.*}} '__promise' 'std::coroutine_traits<awaitable>::promise_type':'awaitable_frame'
+// CHECK:   |                   `-CXXTemporaryObjectExpr {{.*}} <col:12, col:21> 'executor' 'void () noexcept' zeroing
+
+// Rest of the generated coroutine statements omitted.