Clang fails to detect illegal copy constructor with template class as its parameter

[Rush10233]

The following code is apparently illegal, as the declaration of A(A<V,T>) should be treated as a copy constructor, which forces passing the argument by reference. While clang ignores that error and accepts it:

template < class T, class V > struct A
{
    A ();
    A (A &);
    A (A < V,T >);
};

void f () 
{
    A <int, int> (A < int, int >());
}

Note that if replace A<V,T> with A<T,T> or A<V,V>, the code is still accepted. But when replacing it with A<T,V>, the code is correctly rejected.

Reducing the number of template class arguments to 1 (template <class T> A and A(A<T>)) also makes sense. We suspect the bug will occur only if the template argument list in copy ctor parameter is not identical to that in class declaration.

https://godbolt.org/z/GKv3E6c4f

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (Rush10233)

The following code is apparently illegal, as the declaration of `A(A<V,T>)` should be treated as a copy constructor, which forces passing the argument by reference. While clang ignores that error and accepts it:

template < class T, class V > struct A
{
    A ();
    A (A &);
    A (A < V,T >);
};

void f () 
{
    A <int, int> (A < int, int >());
}

Note that if replace A<V,T> with A<T,T> or A<V,V>, the code is still accepted. But when replacing it with A<T,V>, the code is correctly rejected.

Reducing the number of template class arguments to 1 (template <class T> A and A(A<T>)) also makes sense. We suspect the bug will occur only if the template argument list in copy ctor parameter is not identical to that in class declaration.

https://godbolt.org/z/GKv3E6c4f

[shafik]

I believe it should only be treated as an ill-formed copy constructor if T and V are the same: https://godbolt.org/z/sjKzzMxdn

template < class T, class V > struct A
{
    A ();
    A (A &);
    A (A < V,T >);
};

void f () 
{
   A<double,int> (A <int,double>{} );

   A<int,double> a{};
   A<double,int> a2(a);

   A<int,int> a3{};
   A<int,int> a4{a3};
}

[cor3ntin]

The bug is here

llvm-project/clang/lib/Sema/SemaDeclCXX.cpp

Lines 10921 to 10922 in 369b822

	Constructor->getTemplateSpecializationKind() !=
	TSK_ImplicitInstantiation) {

This was introduced by ffe14e3 , but it looks wrong to me.
If the type of the parameter is the same as the type of the specialization, we should diagnose it.

https://eel.is/c++draft/class.copy.ctor#1

There are quite a few tests to fix.

Note that we should not diagnose the case where the constructor is a template so we should check !Constructor->isFunctionTemplateSpecialization()

struct A
{
    A ();
    template < class T > 
    A (T);
};

void f () 
{
  A a;
  A b(a); // no warning expected
}

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. In the comments of the issue, request for it to be assigned to you.
2. Fix the issue locally.
3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
4. Create a Git commit.
5. Run git clang-format HEAD~1 to format your changes.
6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: None (Rush10233)

The following code is apparently illegal, as the declaration of `A(A<V,T>)` should be treated as a copy constructor, which forces passing the argument by reference. While clang ignores that error and accepts it:

template < class T, class V > struct A
{
    A ();
    A (A &);
    A (A < V,T >);
};

void f () 
{
    A <int, int> (A < int, int >());
}

Note that if replace A<V,T> with A<T,T> or A<V,V>, the code is still accepted. But when replacing it with A<T,V>, the code is correctly rejected.

Reducing the number of template class arguments to 1 (template <class T> A and A(A<T>)) also makes sense. We suspect the bug will occur only if the template argument list in copy ctor parameter is not identical to that in class declaration.

https://godbolt.org/z/GKv3E6c4f

[akshaykumars614]

I am starting to analyze the issue.