[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-24 Thread Richard Smith via cfe-commits

zygoloid wrote:

This error is [produced for the following 
code](https://github.com/carbon-language/carbon-lang/pull/5170):
```c++
template 
class SetView {
  SetView(SetView, KeyContextT> other_view)
requires(!std::same_as>);
};
```
... which is never an eligible copy constructor. Is that intended? If this is 
in line with the language rules, perhaps we should file a core issue.

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-15 Thread via cfe-commits

Megan0704-1 wrote:

@cor3ntin 
Thank you for flagging the test failures! 

I’ve investigated them, and here’s what I found: 
Failed Tests in constructor-template.cpp: 
- The failures occurred because the test wasn’t annotated to expect the new 
diagnostics introduced by the fix (e.g., A instantiations now emit 
errors for invalid by-value copy constructors). 

Fix it by updating the test to: 
- Add expected-error to lines 142, 159, 172 (invalid by-value constructors when 
T = U). 
- Add expected-note to lines 147, 164, 176 (template instantiation points). 
- Remove an incorrect expected-error from line 76 (valid template 
specialization). 

Please Let me know if further adjustments are needed~

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-13 Thread via cfe-commits


@@ -10921,8 +10921,8 @@ void Sema::CheckConstructor(CXXConstructorDecl 
*Constructor) {
   //   parameters have default arguments.
   if (!Constructor->isInvalidDecl() &&
   Constructor->hasOneParamOrDefaultArgs() &&
-  Constructor->getTemplateSpecializationKind() !=
-  TSK_ImplicitInstantiation) {
+  !Constructor->isFunctionTemplateSpecialization()
+  ) {

cor3ntin wrote:

I missed it too, sorry.

yes, feel free to submit a new pr. thanks 

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread Shafik Yaghmour via cfe-commits


@@ -10921,8 +10921,8 @@ void Sema::CheckConstructor(CXXConstructorDecl 
*Constructor) {
   //   parameters have default arguments.
   if (!Constructor->isInvalidDecl() &&
   Constructor->hasOneParamOrDefaultArgs() &&
-  Constructor->getTemplateSpecializationKind() !=
-  TSK_ImplicitInstantiation) {
+  !Constructor->isFunctionTemplateSpecialization()
+  ) {

shafik wrote:

Did you run clang-format on this? I am surprised it would leave the `)` on the 
line like that but maybe I am off.

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff c2ed840ed94d3412c7c0bdd9ed84cac6fe0afb57 
790d151975c9ce4f5f823484d100d9460077b971 --extensions cpp -- 
clang/test/SemaCXX/copy-ctor-template.cpp clang/lib/Sema/SemaDeclCXX.cpp 
clang/test/SemaTemplate/constructor-template.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 1c62a551ee..00b4006b5e 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -10921,8 +10921,7 @@ void Sema::CheckConstructor(CXXConstructorDecl 
*Constructor) {
   //   parameters have default arguments.
   if (!Constructor->isInvalidDecl() &&
   Constructor->hasOneParamOrDefaultArgs() &&
-  !Constructor->isFunctionTemplateSpecialization()
-  ) {
+  !Constructor->isFunctionTemplateSpecialization()) {
 QualType ParamType = Constructor->getParamDecl(0)->getType();
 QualType ClassTy = Context.getTagDeclType(ClassDecl);
 if (Context.getCanonicalType(ParamType).getUnqualifiedType() == ClassTy) {

``




https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

github-actions[bot] wrote:



@Megan0704-1 Congratulations on having your first Pull Request (PR) merged into 
the LLVM Project!

Your changes will be combined with recent changes from other authors, then 
tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a 
problem with a build, you may receive a report in an email or a comment on this 
PR.

Please check whether problems have been caused by your change specifically, as 
the builds can include changes from many authors. It is not uncommon for your 
change to be included in a build that fails due to someone else's changes, or 
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail 
[here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).

If your change does cause a problem, it may be reverted, or you can revert it 
yourself. This is a normal part of [LLVM 
development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy).
 You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are 
working as expected, well done!


https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

https://github.com/cor3ntin closed 
https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits


@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct A{
+A();
+A(A&);
+A(A); // expected-error{{copy constructor must pass its first 
argument by reference}}
+};
+
+void f() {
+A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+}
+
+template
+struct B{
+B();
+template B(U); // No error (templated constructor)
+};
+
+void g() {
+B b = B(); // should use implicit copy constructor
+}

cor3ntin wrote:

Can you add a test for @hubert-reinterpretcast comment here
https://github.com/llvm/llvm-project/issues/80963#issuecomment-2716322960

Can you add tests for something like
`A a = A();` // not a copy constructor


https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

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 
Date: Tue, 11 Mar 2025 17:09:04 -0700
Subject: [PATCH 1/4] [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 0..a46a167038cf7
--- /dev/null
+++ b/clang/test/SemaCXX/copy-ctor-template.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct A{
+A();
+A(A&);
+A(A); // expected-error{{copy constructor must pass its first 
argument by reference}}
+};
+
+void f() {
+A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+}
+
+template
+struct B{
+B();
+template B(U); // No error (templated constructor)
+};
+
+void g() {
+B b = B(); // should use implicit copy constructor
+}

>From 4fe1d934e7a688c346d5218e213decc67b261d70 Mon Sep 17 00:00:00 2001
From: Megan 
Date: Wed, 12 Mar 2025 00:47:56 -0700
Subject: [PATCH 2/4] [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 
Date: Wed, 12 Mar 2025 01:24:35 -0700
Subject: [PATCH 3/4] [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
 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); // expected-error{{copy constructor must pass its first 
argument by reference}}
 };
 
 void f() {
-A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+A a = A(); // expected-note{{i

[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

cor3ntin wrote:

@Megan0704-1 Can you look into the test failures? thanks

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

Megan0704-1 wrote:

Thank you for reviewing and approving the PR!
Yes, please merge it for me. Thanks again for all your help!

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

Megan0704-1 wrote:

Thanks for your feedback and review!! I’ve updated the changelog and the test 
file to include the rvalue-to-lvalue case and templated constructor scenario. 
Please let me know if there’s anything else!

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

https://github.com/cor3ntin approved this pull request.

LGTM. Thanks for fixing this issue!

Will you need me to merge this for you?

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

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 
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 0..a46a167038cf7
--- /dev/null
+++ b/clang/test/SemaCXX/copy-ctor-template.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct A{
+A();
+A(A&);
+A(A); // expected-error{{copy constructor must pass its first 
argument by reference}}
+};
+
+void f() {
+A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+}
+
+template
+struct B{
+B();
+template B(U); // No error (templated constructor)
+};
+
+void g() {
+B b = B(); // should use implicit copy constructor
+}

>From 4fe1d934e7a688c346d5218e213decc67b261d70 Mon Sep 17 00:00:00 2001
From: Megan 
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 5ea66a6461de5890edd8ae7e5790278eec9aecea Mon Sep 17 00:00:00 2001
From: Megan 
Date: Wed, 12 Mar 2025 01:15:12 -0700
Subject: [PATCH 3/3] [Sema] Fix test diagnostics for copy-ctor-template

---
 clang/test/SemaCXX/copy-ctor-template.cpp | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/clang/test/SemaCXX/copy-ctor-template.cpp 
b/clang/test/SemaCXX/copy-ctor-template.cpp
index a46a167038cf7..92dea6208a167 100644
--- a/clang/test/SemaCXX/copy-ctor-template.cpp
+++ b/clang/test/SemaCXX/copy-ctor-template.cpp
@@ -2,13 +2,21 @@
 
 template
 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); // expected-error{{copy constructor must pass its first 
argument by reference}}
 };
 
 void f() {
-A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+A a = A(); // expected-note{{

[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

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 
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 0..a46a167038cf7
--- /dev/null
+++ b/clang/test/SemaCXX/copy-ctor-template.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct A{
+A();
+A(A&);
+A(A); // expected-error{{copy constructor must pass its first 
argument by reference}}
+};
+
+void f() {
+A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+}
+
+template
+struct B{
+B();
+template B(U); // No error (templated constructor)
+};
+
+void g() {
+B b = B(); // should use implicit copy constructor
+}

>From 4fe1d934e7a688c346d5218e213decc67b261d70 Mon Sep 17 00:00:00 2001
From: Megan 
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 
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
 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); // expected-error{{copy constructor must pass its first 
argument by reference}}
 };
 
 void f() {
-A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+A a = A(); // expected-note{{i

[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits


@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct A{
+A();
+A(A&);
+A(A); // expected-error{{copy constructor must pass its first 
argument by reference}}
+};
+
+void f() {
+A a = A(); // expected-note{{in instantiation of 
template class 'A'}}
+}
+
+template
+struct B{
+B();
+template B(U); // No error (templated constructor)
+};
+
+void g() {
+B b = B(); // should use implicit copy constructor
+}

Megan0704-1 wrote:

Thanks for the review! I’ll add tests for the A case and update 
the PR shortly.

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

https://github.com/cor3ntin commented:

Thanks for working on this 

Can you add a changelog entry in clang/docs/ReleaseNotes.rst (in the bug fixes 
to c++ section)?

Thanks

https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)

2025-03-12 Thread via cfe-commits

https://github.com/cor3ntin edited 
https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits