[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
shafik wrote: @avikivity https://lists.isocpp.org/core/2025/03/17665.php https://github.com/llvm/llvm-project/pull/130866 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
avikivity wrote: Ah, it's behind a password https://github.com/llvm/llvm-project/pull/130866 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
avikivity wrote:
> This error is [produced for the following
> code](https://github.com/carbon-language/carbon-lang/pull/5170):
>
> ```c++
> template
> class SetView {
> SetView(SetView> 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.
>
> [Edit: I've mailed the core reflector.]
Can you link to the defect report please?
https://github.com/llvm/llvm-project/pull/130866
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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 [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
@@ -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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
@@ -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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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 [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
https://github.com/cor3ntin closed https://github.com/llvm/llvm-project/pull/130866 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
@@ -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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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)
cor3ntin wrote: @Megan0704-1 Can you look into the test failures? thanks https://github.com/llvm/llvm-project/pull/130866 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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 [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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 [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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 [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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)
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)
@@ -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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
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 [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Diagnose by-value copy constructors in template instantiations (PR #130866)
https://github.com/cor3ntin edited https://github.com/llvm/llvm-project/pull/130866 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
