Skip to content
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

[Clang][Sema] Do not add implicit 'const' when matching constexpr function template explicit specializations after C++14 #92449

Merged
merged 2 commits into from
May 20, 2024

Conversation

sdkrystian
Copy link
Member

Clang incorrectly accepts the following when using C++14 or later:

struct A {
  template<typename T>
  void f() const;

  template<>
  constexpr void f<int>();
};

Non-static member functions declared constexpr are only implicitly const in C++11. This patch makes clang reject the explicit specialization of f when using language standards after C++11.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 16, 2024
@sdkrystian sdkrystian requested a review from Endilll May 16, 2024 20:17
@llvmbot
Copy link
Collaborator

llvmbot commented May 16, 2024

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Clang incorrectly accepts the following when using C++14 or later:

struct A {
  template&lt;typename T&gt;
  void f() const;

  template&lt;&gt;
  constexpr void f&lt;int&gt;();
};

Non-static member functions declared constexpr are only implicitly const in C++11. This patch makes clang reject the explicit specialization of f when using language standards after C++11.


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

3 Files Affected:

  • (modified) clang/lib/Sema/SemaTemplate.cpp (+10-5)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp (+10-3)
  • (added) clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp (+70)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a5350ceb59cb7..420bd2de88686 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -10282,15 +10282,20 @@ bool Sema::CheckFunctionTemplateSpecialization(
                                 Ovl->getDeclContext()->getRedeclContext()))
         continue;
 
+      QualType FT = FD->getType();
+      // C++11 [dcl.constexpr]p8:
+      //   A constexpr specifier for a non-static member function that is not
+      //   a constructor declares that member function to be const.
+      //
       // When matching a constexpr member function template specialization
       // against the primary template, we don't yet know whether the
       // specialization has an implicit 'const' (because we don't know whether
       // it will be a static member function until we know which template it
-      // specializes), so adjust it now assuming it specializes this template.
-      QualType FT = FD->getType();
-      if (FD->isConstexpr()) {
-        CXXMethodDecl *OldMD =
-          dyn_cast<CXXMethodDecl>(FunTmpl->getTemplatedDecl());
+      // specializes). This rule was removed in C++14.
+      if (auto *NewMD = dyn_cast<CXXMethodDecl>(FD);
+          !getLangOpts().CPlusPlus14 && NewMD && NewMD->isConstexpr() &&
+          !isa<CXXConstructorDecl, CXXDestructorDecl>(NewMD)) {
+        auto *OldMD = dyn_cast<CXXMethodDecl>(FunTmpl->getTemplatedDecl());
         if (OldMD && OldMD->isConst()) {
           const FunctionProtoType *FPT = FT->castAs<FunctionProtoType>();
           FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
index a28a5f91c4775..2712f203bbd76 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
@@ -89,6 +89,9 @@ struct S {
   template<typename T> constexpr T f(); // expected-warning 0-1{{C++14}} expected-note 0-1{{candidate}}
   template <typename T>
   T g() const; // expected-note-re {{candidate template ignored: could not match 'T (){{( __attribute__\(\(thiscall\)\))?}} const' against 'char (){{( __attribute__\(\(thiscall\)\))?}}'}}
+#if __cplusplus >= 201402L
+  // expected-note@-2 {{candidate template ignored: could not match 'T () const' against 'int ()'}}
+#endif
 };
 
 // explicit specialization can differ in constepxr
@@ -100,13 +103,17 @@ template <> notlit S::f() const { return notlit(); }
 #if __cplusplus >= 201402L
 // expected-error@-2 {{no function template matches}}
 #endif
-template <> constexpr int S::g() { return 0; } // expected-note {{previous}}
+template <> constexpr int S::g() { return 0; }
 #if __cplusplus < 201402L
 // expected-warning@-2 {{C++14}}
+// expected-note@-3 {{previous}}
 #else
-// expected-error@-4 {{does not match any declaration in 'S'}}
+// expected-error@-5 {{no function template matches function template specialization 'g'}}
+#endif
+template <> int S::g() const;
+#if __cplusplus < 201402L
+// expected-error@-2 {{non-constexpr declaration of 'g<int>' follows constexpr declaration}}
 #endif
-template <> int S::g() const; // expected-error {{non-constexpr declaration of 'g<int>' follows constexpr declaration}}
 // specializations can drop the 'constexpr' but not the implied 'const'.
 template <> char S::g() { return 0; } // expected-error {{no function template matches}}
 template <> double S::g() const { return 0; } // ok
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp
new file mode 100644
index 0000000000000..2a57489083695
--- /dev/null
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify=expected,cxx11 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify=expected,since-cxx14 %s
+
+struct A {
+  template<typename T>
+  void f0();
+
+  template<>
+  constexpr void f0<short>(); // cxx11-error {{conflicting types for 'f0'}}
+                              // cxx11-note@-1 {{previous declaration is here}}
+                              // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+
+  template<typename T>
+  void f1() const; // since-cxx14-note 2{{candidate template ignored: could not match 'void () const' against 'void ()'}}
+
+  template<>
+  constexpr void f1<short>(); // since-cxx14-error {{no function template matches function template specialization 'f1'}}
+                              // cxx11-warning@-1 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+};
+
+template<>
+constexpr void A::f0<long>(); // cxx11-error {{conflicting types for 'f0'}}
+                              // cxx11-note@-1 {{previous declaration is here}}
+                              // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+
+template<>
+constexpr void A::f1<long>(); // since-cxx14-error {{no function template matches function template specialization 'f1'}}
+                              // cxx11-warning@-1 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+
+// FIXME: It's unclear whether [temp.expl.spec]p12 is intended to apply to
+// members of a class template explicitly specialized for an implicitly
+// instantiated specialization of that template.
+template<typename T>
+struct B {
+  void g0(); // since-cxx14-note {{previous declaration is here}}
+             // cxx11-note@-1 {{member declaration does not match because it is not const qualified}}
+
+  void g1() const; // since-cxx14-note {{member declaration does not match because it is const qualified}}
+                   // cxx11-note@-1 {{previous declaration is here}}
+
+  template<typename U>
+  void h0(); // since-cxx14-note {{previous declaration is here}}
+
+  template<typename U>
+  void h1() const; // cxx11-note {{previous declaration is here}}
+};
+
+template<>
+constexpr void B<short>::g0(); // since-cxx14-error {{constexpr declaration of 'g0' follows non-constexpr declaration}}
+                               // cxx11-error@-1 {{out-of-line declaration of 'g0' does not match any declaration in 'B<short>'}}
+                               // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+
+template<>
+constexpr void B<short>::g1(); // since-cxx14-error {{out-of-line declaration of 'g1' does not match any declaration in 'B<short>'}}
+                               // cxx11-error@-1 {{constexpr declaration of 'g1' follows non-constexpr declaration}}
+                               // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+
+template<>
+template<typename U>
+constexpr void B<long>::h0(); // since-cxx14-error {{constexpr declaration of 'h0' follows non-constexpr declaration}}
+                              // cxx11-error@-1 {{out-of-line declaration of 'h0' does not match any declaration in 'B<long>'}}
+                              // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+
+template<>
+template<typename U>
+constexpr void B<long>::h1(); // since-cxx14-error {{out-of-line declaration of 'h1' does not match any declaration in 'B<long>'}}
+                              // cxx11-error@-1 {{constexpr declaration of 'h1' follows non-constexpr declaration}}
+                              // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+
+

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.

Release note, else LGTM

@sdkrystian sdkrystian merged commit e75b58c into llvm:main May 20, 2024
4 of 5 checks passed
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.

None yet

3 participants