[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 ↗

(On Diff #429213)

Add a comment what this co_return checks.

46 ↗

(On Diff #429213)

Add a comment about what this NullStmt does.

[MaskRay]

clang/lib/Sema/SemaCoroutine.cpp

990

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

clang/test/SemaCXX/coroutine-dep.cpp

1 ↗

(On Diff #429213)

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

55 ↗

(On Diff #429213)

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 ↗

(On Diff #429213)

3 ↗

(On Diff #429213)

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

[urnathan]

update

clang/lib/Sema/SemaCoroutine.cpp

990

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

990

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

[urnathan]

clang/lib/Sema/SemaCoroutine.cpp

990

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();
   }
 
   VarDecl *Promise = FSI->CoroutinePromise;
+  if ((E && E->getType()->isDependentType()) ||
+      Promise->getType()->isDependentType())
+    // It is dependent, do nothing until instantiation time.
+    return new (Context) CoreturnStmt(Loc, E, nullptr, IsImplicit);
+
   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();
 
-  Stmt *Res = new (Context) CoreturnStmt(Loc, E, PCE, IsImplicit);
-  return Res;
+  return new (Context) CoreturnStmt(Loc, E, PCE, IsImplicit);
 }
 
 /// Look up the std::nothrow object.
 static Expr *buildStdNoThrowDeclRef(Sema &S, SourceLocation Loc) {
   NamespaceDecl *Std = S.getStdNamespace();
   assert(Std && "Should already be diagnosed");
 
   LookupResult Result(S, &S.PP.getIdentifierTable().get("nothrow"), Loc,

clang/test/SemaCXX/coroutine-dependent.cpp

+// RUN: %clang_cc1 -std=c++17 -fcoroutines-ts -fsyntax-only %s -verify
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %s -verify
+// expected-no-diagnostics
+
+#include "Inputs/std-coroutine.h"
+
+// Dependent return that ends up void
+namespace Void {
+struct Task {
+  struct promise_type {
+    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
+
+// Dependent return that ends up non-void
+namespace Value {
+struct Task {
+  struct promise_type {
+    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