-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[Clang][C++23] update constexpr diagnostics for missing return statements per P2448 #94123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang Author: Oleksandr T. (a-tarasyuk) ChangesFixes #92583 Full diff: https://github.com/llvm/llvm-project/pull/94123.diff 3 Files Affected:
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));
|
…statements per P2448
There was a problem hiding this 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
There was a problem hiding this 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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@a-tarasyuk Do you need someone else to merge this? |
@tbaederr Yes, I do. I don't have access to merge… |
Fixes #92583