https://github.com/Megan0704-1 updated https://github.com/llvm/llvm-project/pull/130866
>From 80e764fcfa1912e9d3771f4edb354569741010b7 Mon Sep 17 00:00:00 2001 From: Megan <megan...@asu.edu> Date: Tue, 11 Mar 2025 17:09:04 -0700 Subject: [PATCH 1/3] [Sema] Diagnose by-value copy constructors in template instantiations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #80963 Previously, Clang skipped diagnosing a constructor if it was implicitly instantiated from a template class (TSK_ImplicitInstantiation). This allowed ill-formed “copy” constructors taking the class by value (e.g. A(A)) to slip through without a diagnostic. However, the C++ standard mandates that copy constructors must take their class type parameter by reference (e.g., A(const A&)). Furthermore, a constructor template that *would* form a copy-by-value signature is not treated as a copy constructor and should never be chosen for copying. This patch replaces the check on TSK_ImplicitInstantiation with a check to see if the constructor is a function template specialization (i.e., isFunctionTemplateSpecialization()). That ensures proper diagnosis of non-template copy-by-value constructors, while still allowing valid template constructors that might appear to have a copy-like signature but should be SFINAEd out or simply not selected as a copy constructor. --- clang/lib/Sema/SemaDeclCXX.cpp | 4 ++-- clang/test/SemaCXX/copy-ctor-template.cpp | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 clang/test/SemaCXX/copy-ctor-template.cpp diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 96aac7871db1e..1c62a551ee732 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -10921,8 +10921,8 @@ void Sema::CheckConstructor(CXXConstructorDecl *Constructor) { // parameters have default arguments. if (!Constructor->isInvalidDecl() && Constructor->hasOneParamOrDefaultArgs() && - Constructor->getTemplateSpecializationKind() != - TSK_ImplicitInstantiation) { + !Constructor->isFunctionTemplateSpecialization() + ) { QualType ParamType = Constructor->getParamDecl(0)->getType(); QualType ClassTy = Context.getTagDeclType(ClassDecl); if (Context.getCanonicalType(ParamType).getUnqualifiedType() == ClassTy) { diff --git a/clang/test/SemaCXX/copy-ctor-template.cpp b/clang/test/SemaCXX/copy-ctor-template.cpp new file mode 100644 index 0000000000000..a46a167038cf7 --- /dev/null +++ b/clang/test/SemaCXX/copy-ctor-template.cpp @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +template<class T, class V> +struct A{ + A(); + A(A&); + A(A<V, T>); // expected-error{{copy constructor must pass its first argument by reference}} +}; + +void f() { + A<int, int> a = A<int, int>(); // expected-note{{in instantiation of template class 'A<int, int>'}} +} + +template<class T, class V> +struct B{ + B(); + template<class U> B(U); // No error (templated constructor) +}; + +void g() { + B<int, int> b = B<int, int>(); // should use implicit copy constructor +} >From 4fe1d934e7a688c346d5218e213decc67b261d70 Mon Sep 17 00:00:00 2001 From: Megan <megan...@asu.edu> Date: Wed, 12 Mar 2025 00:47:56 -0700 Subject: [PATCH 2/3] [Docs] Add release note for #80963 fix --- clang/docs/ReleaseNotes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 004f78f22ac36..8bae0d6e99ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -293,6 +293,7 @@ Bug Fixes to Attribute Support Bug Fixes to C++ Support ^^^^^^^^^^^^^^^^^^^^^^^^ +- Clang now diagnoses copy constructors taking the class by value in template instantiations. (#GH130866) - Clang is now better at keeping track of friend function template instance contexts. (#GH55509) - Clang now prints the correct instantiation context for diagnostics suppressed by template argument deduction. >From 12d9237d7a0650328f6167b1951195cd851f6234 Mon Sep 17 00:00:00 2001 From: Megan <megan...@asu.edu> Date: Wed, 12 Mar 2025 01:24:35 -0700 Subject: [PATCH 3/3] [Sema] Fix test diagnostics for copy-ctor-template --- clang/test/SemaCXX/copy-ctor-template.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/clang/test/SemaCXX/copy-ctor-template.cpp b/clang/test/SemaCXX/copy-ctor-template.cpp index a46a167038cf7..c58bd7c0c5e10 100644 --- a/clang/test/SemaCXX/copy-ctor-template.cpp +++ b/clang/test/SemaCXX/copy-ctor-template.cpp @@ -2,13 +2,20 @@ template<class T, class V> struct A{ - A(); - A(A&); + A(); // expected-note{{candidate constructor not viable: requires 0 arguments, but 1 was provided}} + A(A&); // expected-note{{candidate constructor not viable: expects an lvalue for 1st argument}} A(A<V, T>); // expected-error{{copy constructor must pass its first argument by reference}} }; void f() { - A<int, int> a = A<int, int>(); // expected-note{{in instantiation of template class 'A<int, int>'}} + A<int, int> a = A<int, int>(); // expected-note{{in instantiation of template class 'A<int, int>'}} + A<int, double> a1 = A<double, int>(); // No error (not a copy constructor) +} + +// Test rvalue-to-lvalue conversion in copy constructor +A<int, int> &&a(void); +void g() { + A<int, int> a2 = a(); // expected-error{{no matching constructor}} } template<class T, class V> @@ -17,6 +24,6 @@ struct B{ template<class U> B(U); // No error (templated constructor) }; -void g() { +void h() { B<int, int> b = B<int, int>(); // should use implicit copy constructor } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits