[clang] [Clang][Sema] Fix err_constexpr_virtual_base diagnostic so that it only diagnoses on constructors and destructors (PR #163690)
Shafik Yaghmour via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 16 16:22:58 PDT 2025
https://github.com/shafik updated https://github.com/llvm/llvm-project/pull/163690
>From f6bded3e92814b7ec5f2c3427e9c61b3fa0775f2 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour <shafik.yaghmour at intel.com>
Date: Wed, 15 Oct 2025 20:09:36 -0700
Subject: [PATCH 1/4] [Clang][Sema] Fix err_constexpr_virtual_base diagnostic
so that it only diagnoses on constructors and destructors
When this diagnostic was implemented it fired for all non-static data members.
I added checking so that it only fires for contructors and destructors before
C++26 (which now allows this). Some special care was required for the destructor
case since constexpr constuctors were not allowed in C++11. Modified existing
test to cover the new conditions.
Fixes: https://github.com/llvm/llvm-project/issues/97266
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/lib/Sema/SemaDeclCXX.cpp | 10 ++++++----
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp | 10 ++++++++--
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 40bc7b9a4e45e..0eae2b76ff9b9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3026,7 +3026,7 @@ def warn_cxx17_compat_constexpr_virtual : Warning<
"virtual constexpr functions are incompatible with "
"C++ standards before C++20">, InGroup<CXXPre20Compat>, DefaultIgnore;
def err_constexpr_virtual_base : Error<
- "constexpr %select{member function|constructor}0 not allowed in "
+ "constexpr %select{destructor|constructor}0 not allowed in "
"%select{struct|interface|class}1 with virtual base "
"%plural{1:class|:classes}2">;
def note_non_literal_incomplete : Note<
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index d41ab126c426f..da94b0070dfcb 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1918,8 +1918,10 @@ static bool CheckConstexprMissingReturn(Sema &SemaRef, const FunctionDecl *Dcl);
bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD,
CheckConstexprKind Kind) {
- const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewFD);
- if (MD && MD->isInstance()) {
+ if ((!getLangOpts().CPlusPlus26 && isa<CXXConstructorDecl>(NewFD)) ||
+ ((getLangOpts().CPlusPlus20 && !getLangOpts().CPlusPlus26) &&
+ isa<CXXDestructorDecl>(NewFD))) {
+ const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewFD);
// C++11 [dcl.constexpr]p4:
// The definition of a constexpr constructor shall satisfy the following
// constraints:
@@ -1933,8 +1935,8 @@ bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD,
return false;
Diag(NewFD->getLocation(), diag::err_constexpr_virtual_base)
- << isa<CXXConstructorDecl>(NewFD)
- << getRecordDiagFromTagKind(RD->getTagKind()) << RD->getNumVBases();
+ << isa<CXXConstructorDecl>(NewFD)
+ << getRecordDiagFromTagKind(RD->getTagKind()) << RD->getNumVBases();
for (const auto &I : RD->vbases())
Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here)
<< I.getSourceRange();
diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
index 51990ee4341d2..5a06d686b4c0f 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -84,10 +84,16 @@ struct T3 {
struct U {
constexpr U SelfReturn() const;
constexpr int SelfParam(U) const;
+ constexpr ~U(); // beforecxx20-error {{destructor cannot be declared constexpr}}
};
-struct V : virtual U { // expected-note {{here}}
- constexpr int F() const { return 0; } // expected-error {{constexpr member function not allowed in struct with virtual base class}}
+struct V : virtual U { // expected-note {{here}} //aftercxx20-note {{here}}
+ constexpr V() {} // expected-error {{constexpr constructor not allowed in struct with virtual base class}}
+ constexpr ~V() {} // aftercxx20-error {{constexpr destructor not allowed in struct with virtual base class}}
+ // beforecxx20-error at -1 {{destructor cannot be declared constexpr}}
+ // beforecxx14-error at -2 {{constexpr function's return type 'void' is not a literal type}}
+ // ^ FIXME this is just a bad diagnostic
+ constexpr int f() const { return 0; }
};
// or a compound-statememt that contains only [CXX11]
>From 171fadb5404da240351d95c2cb2dcd4124dc8ef0 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour <shafik.yaghmour at intel.com>
Date: Wed, 15 Oct 2025 22:36:55 -0700
Subject: [PATCH 2/4] Fix test I missed updating and remove FIXME
---
clang/lib/Sema/SemaDeclCXX.cpp | 3 ---
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp | 2 +-
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index da94b0070dfcb..904316ad8ac8d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1926,9 +1926,6 @@ bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD,
// The definition of a constexpr constructor shall satisfy the following
// constraints:
// - the class shall not have any virtual base classes;
- //
- // FIXME: This only applies to constructors and destructors, not arbitrary
- // member functions.
const CXXRecordDecl *RD = MD->getParent();
if (RD->getNumVBases()) {
if (Kind == CheckConstexprKind::CheckValid)
diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
index 48bc8fb426bcb..a3d9d1181fef1 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
@@ -6,7 +6,7 @@
namespace vbase {
struct A {};
struct B : virtual A { // expected-note {{virtual}}
- constexpr ~B() {} // expected-error {{constexpr member function not allowed in struct with virtual base class}}
+ constexpr ~B() {} // expected-error {{constexpr destructor not allowed in struct with virtual base class}}
};
}
>From 7e9ecfcc264cf8f1ffa9214e26619b1eb6314675 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour <shafik.yaghmour at intel.com>
Date: Thu, 16 Oct 2025 16:09:05 -0700
Subject: [PATCH 3/4] Switch from dyn_cast to cast since we assume we will have
a valid pointer.
---
clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 904316ad8ac8d..b9db48df424f1 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1921,7 +1921,7 @@ bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD,
if ((!getLangOpts().CPlusPlus26 && isa<CXXConstructorDecl>(NewFD)) ||
((getLangOpts().CPlusPlus20 && !getLangOpts().CPlusPlus26) &&
isa<CXXDestructorDecl>(NewFD))) {
- const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewFD);
+ const CXXMethodDecl *MD = cast<CXXMethodDecl>(NewFD);
// C++11 [dcl.constexpr]p4:
// The definition of a constexpr constructor shall satisfy the following
// constraints:
>From 0daaba3dcc09230f1f9c471ba40a6de3370ec6be Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour <shafik.yaghmour at intel.com>
Date: Thu, 16 Oct 2025 16:22:35 -0700
Subject: [PATCH 4/4] Add Release note.
---
clang/docs/ReleaseNotes.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4f62a679b8b21..e851667336c5e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -477,6 +477,7 @@ Bug Fixes to C++ Support
- Fix for clang incorrectly rejecting the default construction of a union with
nontrivial member when another member has an initializer. (#GH81774)
- Diagnose unresolved overload sets in non-dependent compound requirements. (#GH51246) (#GH97753)
+- Fix bug where we were diagnosing all constexpr member functions if the class had a virtual bases. (#GH97266)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
More information about the cfe-commits
mailing list