[clang] Clang should detect illegal copy constructor with template class as its parameter

[Rajveer100]

Resolves #80963

As described in the snippet of the issue, A<T,V> is correctly detected while it fails to reject in other cases.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Rajveer Singh Bharadwaj (Rajveer100)

Changes

Resolves Issue #80963

As described in the snippet of the issue, A<T,V> is correctly detected while it fails to reject in other cases.

---

Full diff: https://github.com/llvm/llvm-project/pull/81251.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaDeclCXX.cpp (+1-1)
• (added) clang/test/SemaCXX/GH80963.cpp (+15)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ab8a967b06a456..9778679ee40d47 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -10918,7 +10918,7 @@ void Sema::CheckConstructor(CXXConstructorDecl *Constructor) {
   //   parameters have default arguments.
   if (!Constructor->isInvalidDecl() &&
       Constructor->hasOneParamOrDefaultArgs() &&
-      Constructor->getTemplateSpecializationKind() !=
+      Constructor->getTemplateSpecializationKind() ==
           TSK_ImplicitInstantiation) {
     QualType ParamType = Constructor->getParamDecl(0)->getType();
     QualType ClassTy = Context.getTagDeclType(ClassDecl);
diff --git a/clang/test/SemaCXX/GH80963.cpp b/clang/test/SemaCXX/GH80963.cpp
new file mode 100644
index 00000000000000..55779e6ff191b7
--- /dev/null
+++ b/clang/test/SemaCXX/GH80963.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template < class T, class V > struct A
+{
+    A ();
+    A (A &);
+    A (A < V,T >);
+    // expected-error@-1 {{copy constructor must pass its first argument by reference}}
+};
+
+void f ()
+{
+    A <int, int> (A < int, int >());
+    // expected-note@-1 {{in instantiation of template class 'A<int, int>' requested here}}
+}

[Rajveer100]

@cor3ntin @shafik

I am not quite sure if this is the intended fix, also I will check the few other failing tests once the CI completes its job.

[shafik]

@cor3ntin the fix looks like the one you recommended but that is quite a lot of tests that fail.

[shafik]

This also needs a release note.

[cor3ntin]

Could you merge main inside that branch? There are some test failures (which i hope are unrelated). Thanks

[Rajveer100]

I have removed this entirely, although it may still not be optimal, it did reduce few more test failures:

Constructor->hasOneParamOrDefaultArgs() &&
      Constructor->getTemplateSpecializationKind() !=
          TSK_ImplicitInstantiation

Any particular suggestions apart from updating the tests?

[cor3ntin]

I have removed this entirely, although it may still not be optimal, it did reduce few more test failures:

Constructor->hasOneParamOrDefaultArgs() &&
      Constructor->getTemplateSpecializationKind() !=
          TSK_ImplicitInstantiation

Any particular suggestions apart from updating the tests?

I think we should check !Constructor->isFunctionTemplateSpecialization()
See #80963 (comment)