Skip to content

Conversation

a-tarasyuk
Copy link
Member

Fixes #92583

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 1, 2024

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #92583


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

3 Files Affected:

  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+22-12)
  • (modified) clang/test/SemaCXX/constant-expression-cxx14.cpp (+1-1)
  • (added) clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp (+7)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 631fd4e354927..d4401a427282c 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1806,6 +1806,7 @@ static unsigned getRecordDiagFromTagKind(TagTypeKind Tag) {
 static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
                                        Stmt *Body,
                                        Sema::CheckConstexprKind Kind);
+static bool CheckConstexprMissingReturn(Sema &SemaRef, const FunctionDecl *Dcl);
 
 // Check whether a function declaration satisfies the requirements of a
 // constexpr function definition or a constexpr constructor definition. If so,
@@ -2411,20 +2412,9 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
     }
   } else {
     if (ReturnStmts.empty()) {
-      // C++1y doesn't require constexpr functions to contain a 'return'
-      // statement. We still do, unless the return type might be void, because
-      // otherwise if there's no return statement, the function cannot
-      // be used in a core constant expression.
-      bool OK = SemaRef.getLangOpts().CPlusPlus14 &&
-                (Dcl->getReturnType()->isVoidType() ||
-                 Dcl->getReturnType()->isDependentType());
       switch (Kind) {
       case Sema::CheckConstexprKind::Diagnose:
-        SemaRef.Diag(Dcl->getLocation(),
-                     OK ? diag::warn_cxx11_compat_constexpr_body_no_return
-                        : diag::err_constexpr_body_no_return)
-            << Dcl->isConsteval();
-        if (!OK)
+        if (!CheckConstexprMissingReturn(SemaRef, Dcl))
           return false;
         break;
 
@@ -2487,6 +2477,26 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
   return true;
 }
 
+static bool CheckConstexprMissingReturn(Sema &SemaRef,
+                                        const FunctionDecl *Dcl) {
+  bool IsVoidOrDependentType = Dcl->getReturnType()->isVoidType() ||
+                               Dcl->getReturnType()->isDependentType();
+
+  if (SemaRef.getLangOpts().CPlusPlus23 && !IsVoidOrDependentType)
+    return true;
+
+  // C++1y doesn't require constexpr functions to contain a 'return'
+  // statement. We still do, unless the return type might be void, because
+  // otherwise if there's no return statement, the function cannot
+  // be used in a core constant expression.
+  bool OK = SemaRef.getLangOpts().CPlusPlus14 && IsVoidOrDependentType;
+  SemaRef.Diag(Dcl->getLocation(),
+               OK ? diag::warn_cxx11_compat_constexpr_body_no_return
+                  : diag::err_constexpr_body_no_return)
+      << Dcl->isConsteval();
+  return OK;
+}
+
 bool Sema::CheckImmediateEscalatingFunctionDefinition(
     FunctionDecl *FD, const sema::FunctionScopeInfo *FSI) {
   if (!getLangOpts().CPlusPlus20 || !FD->isImmediateEscalating())
diff --git a/clang/test/SemaCXX/constant-expression-cxx14.cpp b/clang/test/SemaCXX/constant-expression-cxx14.cpp
index 80a7a2dd31531..70ab5dcd357c1 100644
--- a/clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -82,7 +82,7 @@ constexpr void k() {
 
 // If the return type is not 'void', no return statements => never a constant
 // expression, so still diagnose that case.
-[[noreturn]] constexpr int fn() { // expected-error {{no return statement in constexpr function}}
+[[noreturn]] constexpr int fn() { // cxx14_20-error {{no return statement in constexpr function}}
   fn();
 }
 
diff --git a/clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp b/clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp
new file mode 100644
index 0000000000000..91a8bb656b317
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -Wno-return-type -std=c++23 -fsyntax-only -verify %s
+// expected-no-diagnostics
+constexpr int f() { }
+static_assert(__is_same(decltype([] constexpr -> int { }( )), int));
+
+consteval int g() { }
+static_assert(__is_same(decltype([] consteval -> int { }( )), int));

@a-tarasyuk a-tarasyuk changed the title feat(92583): [C++23] "no return statement in constexpr function" no longer an error with P2448 feat(92583): [C++23] update constexpr diagnostics for missing return statements per P2448 Jun 3, 2024
@cor3ntin cor3ntin changed the title feat(92583): [C++23] update constexpr diagnostics for missing return statements per P2448 [Clang][C++23] update constexpr diagnostics for missing return statements per P2448 Jun 3, 2024
Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

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

I wonder if we should always emit an error when a function with non-void return type doesn't have a return statement? In any case it is a serious yet dumb bug that will lead to problems and perhaps some time spent debugging it.
gcc rejects the case for C++23 as well https://godbolt.org/z/8xq8TdEGE .
cc @AaronBallman

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!

@a-tarasyuk a-tarasyuk requested a review from AaronBallman June 4, 2024 19:04
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM

@tbaederr
Copy link
Contributor

@a-tarasyuk Do you need someone else to merge this?

@a-tarasyuk
Copy link
Member Author

@tbaederr Yes, I do. I don't have access to merge…

@tbaederr tbaederr merged commit ae9d89d into llvm:main Jun 10, 2024
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 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.

[C++23] "no return statement in constexpr function" no longer an error with P2448
5 participants