Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaDeclCXX.cpp
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
29 changes: 29 additions & 0 deletions clang/test/SemaCXX/copy-ctor-template.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s

template<class T, class V>
struct 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, 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>
struct B{
B();
template<class U> B(U); // No error (templated constructor)
};

void h() {
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.