Skip to content

[Sema] Diagnose by-value copy constructors in template instantiations #130866

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 13, 2025
Merged

[Sema] Diagnose by-value copy constructors in template instantiations #130866

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
80e764f
[Sema] Diagnose by-value copy constructors in template instantiations
Megan0704-1 Mar 12, 2025
4fe1d93
[Docs] Add release note for #80963 fix
Megan0704-1 Mar 12, 2025
12d9237
[Sema] Fix test diagnostics for copy-ctor-template
Megan0704-1 Mar 12, 2025
790d151
[SemaCXX] Update constructor-template.cpp test for #80963
Megan0704-1 Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10921,8 +10921,8 @@ void Sema::CheckConstructor(CXXConstructorDecl *Constructor) {
// parameters have default arguments.
if (!Constructor->isInvalidDecl() &&
Constructor->hasOneParamOrDefaultArgs() &&
Constructor->getTemplateSpecializationKind() !=
TSK_ImplicitInstantiation) {
!Constructor->isFunctionTemplateSpecialization()
) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for catching this, and I am so sorry for the oversight.
I forgot to run clang-format before merging.
Should I submitt a follow-up PR to fix the formatting issues?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed it too, sorry.

yes, feel free to submit a new pr. thanks

QualType ParamType = Constructor->getParamDecl(0)->getType();
QualType ClassTy = Context.getTagDeclType(ClassDecl);
if (Context.getCanonicalType(ParamType).getUnqualifiedType() == ClassTy) {
Expand Down
22 changes: 22 additions & 0 deletions clang/test/SemaCXX/copy-ctor-template.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for @hubert-reinterpretcast comment here
#80963 (comment)

Can you add tests for something like
A<int, double> a = A<double, int>(); // not a copy constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I’ll add tests for the A<int, double> case and update the PR shortly.