Skip to content

Conversation

@yronglin
Copy link
Contributor

Implement P2748R5 "Disallow Binding a Returned Glvalue to a Temporary" https://wg21.link/P2748R5

…emporary"

Signed-off-by: yronglin <yronglin777@gmail.com>
@yronglin yronglin requested a review from Endilll as a code owner April 24, 2024 15:38
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 24, 2024
@yronglin yronglin changed the title [Clang] Implement P2748R5 "Disallow Binding a Returned Glvalue to a Temporary" [Clang] Implement C++26 P2748R5 "Disallow Binding a Returned Glvalue to a Temporary" Apr 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 24, 2024

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

Implement P2748R5 "Disallow Binding a Returned Glvalue to a Temporary" https://wg21.link/P2748R5


Full diff: https://github.com/llvm/llvm-project/pull/89942.diff

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3-1)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
  • (modified) clang/lib/Sema/SemaInit.cpp (+11-2)
  • (modified) clang/test/CXX/drs/cwg650.cpp (+1-1)
  • (added) clang/test/CXX/stmt.stmt/stmt.return/p6.cpp (+18)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 64526ed6d06f55..5e07000198d63a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -129,7 +129,9 @@ C++2c Feature Support
 
 - Implemented `P2662R3 Pack Indexing <https://wg21.link/P2662R3>`_.
 
-- Implemented `P2573R2: = delete("should have a reason"); <https://wg21.link/P2573R2>`_
+- Implemented `P2573R2: = delete("should have a reason"); <https://wg21.link/P2573R2>`_.
+
+- Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary <https://wg21.link/P2748R5>`_.
 
 
 Resolutions to C++ Defect Reports
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6732a1a98452ad..7342215db9cc3d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9950,6 +9950,8 @@ def warn_ret_stack_addr_ref : Warning<
 def warn_ret_local_temp_addr_ref : Warning<
   "returning %select{address of|reference to}0 local temporary object">,
   InGroup<ReturnStackAddress>;
+def err_ret_local_temp_addr_ref : Error<
+  "returning %select{address of|reference to}0 local temporary object">;
 def warn_ret_addr_label : Warning<
   "returning address of label, which is local">,
   InGroup<ReturnStackAddress>;
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 793e16df178914..003c4c34810e1f 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -8340,8 +8340,17 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
             << Entity.getType()->isReferenceType() << CLE->getInitializer() << 2
             << DiagRange;
       } else {
-        Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
-         << Entity.getType()->isReferenceType() << DiagRange;
+        // P2748R5: Disallow Binding a Returned Glvalue to a Temporary.
+        // [stmt.return]/p6: In a function whose return type is a reference,
+        // other than an invented function for std::is_convertible ([meta.rel]),
+        // a return statement that binds the returned reference to a temporary
+        // expression ([class.temporary]) is ill-formed.
+        if (getLangOpts().CPlusPlus26)
+          Diag(DiagLoc, diag::err_ret_local_temp_addr_ref)
+              << Entity.getType()->isReferenceType() << DiagRange;
+        else
+          Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
+              << Entity.getType()->isReferenceType() << DiagRange;
       }
       break;
     }
diff --git a/clang/test/CXX/drs/cwg650.cpp b/clang/test/CXX/drs/cwg650.cpp
index dcb844095b0598..01a841b04b42d3 100644
--- a/clang/test/CXX/drs/cwg650.cpp
+++ b/clang/test/CXX/drs/cwg650.cpp
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
 // RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
 // RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
-// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// Since C++26, P2748R5 "Disallow Binding a Returned Glvalue to a Temporary". Therefore we do not test this issue after C++26.
 
 #if __cplusplus == 199711L
 #define NOTHROW throw()
diff --git a/clang/test/CXX/stmt.stmt/stmt.return/p6.cpp b/clang/test/CXX/stmt.stmt/stmt.return/p6.cpp
new file mode 100644
index 00000000000000..682d3a8a075d4e
--- /dev/null
+++ b/clang/test/CXX/stmt.stmt/stmt.return/p6.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++26 -fsyntax-only -verify %s
+
+auto&& f1() {
+  return 42; // expected-error{{returning reference to local temporary object}}
+}
+const double& f2() {
+  static int x = 42;
+  return x; // expected-error{{returning reference to local temporary object}}
+}
+auto&& id(auto&& r) {
+  return static_cast<decltype(r)&&>(r);
+}
+auto&& f3() {
+  return id(42);        // OK, but probably a bug
+}
+
+static_assert(__is_convertible(int, const int &));
+static_assert(__is_nothrow_convertible(int, const int &));

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. you will also need to modify cxx_status.html

Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DR changes look good.
@erichkeane mind giving CWG650 test a quick look?

Signed-off-by: yronglin <yronglin777@gmail.com>
@yronglin
Copy link
Contributor Author

DR changes look good. @erichkeane mind giving CWG650 test a quick look?

Thanks for your review!

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 Nit, else LGTM, the CWG issue change seems sensible, as does the rest.

…l temporary object

Signed-off-by: yronglin <yronglin777@gmail.com>
Signed-off-by: yronglin <yronglin777@gmail.com>
@github-actions
Copy link

github-actions bot commented Apr 26, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo comment to shorten + test to add
Thanks a lot for this PR!

Signed-off-by: yronglin <yronglin777@gmail.com>
@yronglin
Copy link
Contributor Author

LGTM modulo comment to shorten + test to add Thanks a lot for this PR!

Thanks for your review and help!

@yronglin yronglin merged commit e718403 into llvm:main Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants