[clang] dependent co_return fix

[urnathan]

Summary

This addresses an issue with dependent co_return handling, and
preparation for PR55406's fix.

If the co_return type is not void, we'll look for return_value, but
if it's void, we'll look for return_void. Usually the promise type is
dependent, and no lookup is actually done. But, if it is
non-dependent, we'll do the lookup for return_value even when the type
is T. And then possibly emit an incorrect diagnostic.

We should only do the lookup when the types are non-dependent. This
is the only coroutine place where the name we look for depends on an
expression type.

[MaskRay]

clang/test/SemaCXX/coroutine-dep.cpp

45

Add a comment what this co_return checks.

46

Add a comment about what this NullStmt does.

[MaskRay]

clang/lib/Sema/SemaCoroutine.cpp

983

The condition Promise->getType()->isDependentType() is not covered by a test. Is it needed?

clang/test/SemaCXX/coroutine-dep.cpp

1

Other tests use *dependent* to indicate dependent types. It's unneeded to abbreviate it into dep.

55

Delete unneeded public:

[ChuanqiXu]

clang/lib/Sema/SemaCoroutine.cpp

980

It looks better to follow the style in https://github.com/llvm/llvm-project/blob/9b519f416b70db0fa3bba020732988d3555a3625/clang/lib/Sema/SemaCoroutine.cpp#L936-L939

clang/test/SemaCXX/coroutine-dep.cpp

1

3

nit: it could save some typings by #include "Inputs/std-coroutine.h"

[urnathan]

update

clang/lib/Sema/SemaCoroutine.cpp

983

existing tests cover that.

[ChuanqiXu]

What's the reason you removed me as a reviewer? I don't think I did any thing bad.

clang/lib/Sema/SemaCoroutine.cpp

983

No. It doesn't. I've tried to remove the check and all the tests still pass.

[urnathan]

clang/lib/Sema/SemaCoroutine.cpp

983

correct.

clang/lib/Sema/SemaCoroutine.cpp

StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E,
 
   if (E && E->hasPlaceholderType() &&
       !E->hasPlaceholderType(BuiltinType::Overload)) {
     ExprResult R = CheckPlaceholderExpr(E);
     if (R.isInvalid()) return StmtError();
     E = R.get();
   }
 
+  Expr *PCE = nullptr;
   VarDecl *Promise = FSI->CoroutinePromise;
+  if ((E && E->getType()->isDependentType()) ||
+      Promise->getType()->isDependentType()) {
+    // It is dependent, do nothing until instantiation time.
+  } else {
     ExprResult PC;
     if (E && (isa<InitListExpr>(E) || !E->getType()->isVoidType())) {
       getNamedReturnInfo(E, SimplerImplicitMoveMode::ForceOn);
       PC = buildPromiseCall(*this, Promise, Loc, "return_value", E);
     } else {
       E = MakeFullDiscardedValueExpr(E).get();
       PC = buildPromiseCall(*this, Promise, Loc, "return_void", None);
     }
     if (PC.isInvalid())
       return StmtError();
 
-  Expr *PCE = ActOnFinishFullExpr(PC.get(), /*DiscardedValue*/ false).get();
+    PCE = ActOnFinishFullExpr(PC.get(), /*DiscardedValue*/ false).get();
+  }
 
   Stmt *Res = new (Context) CoreturnStmt(Loc, E, PCE, IsImplicit);
   return Res;
 }
 
 /// Look up the std::nothrow object.
 static Expr *buildStdNoThrowDeclRef(Sema &S, SourceLocation Loc) {
   NamespaceDecl *Std = S.getStdNamespace();

clang/test/SemaCXX/coroutine-dep.cpp

+// RUN: %clang_cc1 -std=c++17 -fcoroutines-ts -fsyntax-only %s
+
+namespace std {
+template <typename _Result, typename...>
+struct coroutine_traits {
+  using promise_type = typename _Result::promise_type;
+};
+
+template <typename _Promise = void>
+struct coroutine_handle;
+
+template <>
+struct coroutine_handle<void> {
+  static coroutine_handle from_address(void *__a) noexcept;
+  void resume() const noexcept;
+  void destroy() const noexcept;
+};
+
+template <typename _Promise>
+struct coroutine_handle : coroutine_handle<> {};
+
+struct suspend_always {
+  bool await_ready() const noexcept;
+  void await_suspend(coroutine_handle<>) const noexcept;
+  void await_resume() const noexcept;
+};
+} // namespace std
+
+namespace Void {
+class Task {
+public:
+  struct promise_type {
+  public:
+    std::suspend_always initial_suspend() noexcept;
+    std::suspend_always final_suspend() noexcept;
+
+    Task get_return_object() noexcept;
+    void unhandled_exception() noexcept;
+    void return_void() noexcept;
+  };
+};
+
+template <typename T>
+Task Foo() noexcept {
+  co_return T(0);
+  ;
+}
+template Task Foo<void>() noexcept;
+} // namespace Void
+
+namespace Value {
+class Task {
+public:
+  struct promise_type {
+  public:
+    std::suspend_always initial_suspend() noexcept;
+    std::suspend_always final_suspend() noexcept;
+
+    Task get_return_object() noexcept;
+    void unhandled_exception() noexcept;
+    void return_value(int value) noexcept;
+  };
+};
+
+template <typename T>
+Task Foo() noexcept {
+  co_return T(0);
+}
+template Task Foo<int>() noexcept;
+} // namespace Value