[clang] [clang CodeGen] Restrict return statements nested in return statements. (PR #94282)

[Eli Friedman via cfe-commits]

https://github.com/efriedma-quic created https://github.com/llvm/llvm-project/pull/94282

The construct has unclear semantics in general, and it's not really interesting to try to support. So diagnose it.

Some subset of the cases this rejects might be possible to handle, if this shows up in practice.  But I don't think it's worth the effort without a specific use-case.

Fixes #91761

>From cd63cdce2d27217a87d423a5e696486cdb528acd Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Mon, 3 Jun 2024 14:30:10 -0700
Subject: [PATCH] [clang CodeGen] Restrict return statements nested in return
 statements.

The construct has unclear semantics in general, and it's not really
interesting to try to support. So diagnose it.

Some subset of the cases this rejects might be possible to handle, if
this shows up in practice.  But I don't think it's worth the effort
without a specific use-case.

Fixes #91761
---
 clang/lib/CodeGen/CGStmt.cpp            | 16 ++++++++++++++++
 clang/test/CodeGenCXX/nested-return.cpp | 22 ++++++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 clang/test/CodeGenCXX/nested-return.cpp

diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 99daaa14cf3fe..caf6a64c559ed 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -1455,6 +1455,22 @@ static bool isSwiftAsyncCallee(const CallExpr *CE) {
 /// if the function returns void, or may be missing one if the function returns
 /// non-void.  Fun stuff :).
 void CodeGenFunction::EmitReturnStmt(const ReturnStmt &S) {
+  if (RetExpr) {
+    if (RetExpr->getType()->isRecordType() &&
+        RetExpr->getType().isDestructedType()) {
+      // This is a return of a class inside the return expression of another
+      // return statement.
+      //
+      // Generating correct code for a nested return of a class is hard: we need
+      // to destroy any temporaries which have already been stored in the return
+      // value slot, and it's not clear when that's supposed to happen. In some
+      // cases, there's no possible consistent order.  So reject for now.
+      //
+      // Currently, this can only show up with statement expressions.
+      CGM.ErrorUnsupported(&S, "nested return statement");
+    }
+  }
+
   if (requiresReturnValueCheck()) {
     llvm::Constant *SLoc = EmitCheckSourceLocation(S.getBeginLoc());
     auto *SLocPtr =
diff --git a/clang/test/CodeGenCXX/nested-return.cpp b/clang/test/CodeGenCXX/nested-return.cpp
new file mode 100644
index 0000000000000..7a84fbf852d89
--- /dev/null
+++ b/clang/test/CodeGenCXX/nested-return.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify -emit-llvm-only %s
+
+// Reject this function; there's no way to correctly destroy the temporary
+struct S { S(); ~S(); };
+struct S2 { S s1, s2; };
+S2 f() {
+    return {S(), ({
+        while (true) {
+            return {S(), ({break; S();})}; // expected-error {{cannot compile this nested return statement yet}}
+        }
+        S();})};
+}
+
+// This variant doesn't have any temporaries, so it's allowed.
+struct Simple { int s1, s2; };
+Simple f2() {
+    return {1, ({
+        while (true) {
+            return {2, ({break; 3;})};
+        }
+        3;})};
+}