Skip to content

[NFC] Fix formatting for #80963 #131100

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.

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 1 commit into from
Mar 13, 2025
Merged

Conversation

Megan0704-1
Copy link
Contributor

This PR fixes formatting issues in constructor-template.cpp introduced in #130866.

Changes:

  • Ran clang-format to adhere to LLVM style guidelines.
  • No functional changes.

CC: @cor3ntin @shafik
Thanks

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 13, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2025

@llvm/pr-subscribers-clang

Author: Kuo, Mei-Chun (Megan0704-1)

Changes

This PR fixes formatting issues in constructor-template.cpp introduced in #130866.

Changes:

  • Ran clang-format to adhere to LLVM style guidelines.
  • No functional changes.

CC: @cor3ntin @shafik
Thanks


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

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+1-2)
  • (added) clang/test/SemaCXX/copy-ctor-template.cpp (+29)
  • (modified) clang/test/SemaTemplate/constructor-template.cpp (+8-7)
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.
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..00b4006b5eb43 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->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 0000000000000..c58bd7c0c5e10
--- /dev/null
+++ b/clang/test/SemaCXX/copy-ctor-template.cpp
@@ -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
+}
diff --git a/clang/test/SemaTemplate/constructor-template.cpp b/clang/test/SemaTemplate/constructor-template.cpp
index a89dc60cfa347..13f00beb1ffc5 100644
--- a/clang/test/SemaTemplate/constructor-template.cpp
+++ b/clang/test/SemaTemplate/constructor-template.cpp
@@ -73,7 +73,7 @@ struct X3 {
   template<typename T> X3(T);
 };
 
-template<> X3::X3(X3); // expected-error{{must pass its first argument by reference}}
+template<> X3::X3(X3); // No error (template constructor)
 
 struct X4 {
   X4();
@@ -139,12 +139,12 @@ namespace self_by_value {
   template <class T, class U> struct A {
     A() {}
     A(const A<T,U> &o) {}
-    A(A<T,T> o) {}
+    A(A<T,T> o) {} // expected-error{{copy constructor must pass its first argument by reference}}
   };
 
   void helper(A<int,float>);
 
-  void test1(A<int,int> a) {
+  void test1(A<int,int> a) { // expected-note{{in instantiation of template class 'self_by_value::A<int, int>'}}
     helper(a);
   }
   void test2() {
@@ -156,12 +156,13 @@ namespace self_by_value_2 {
   template <class T, class U> struct A {
     A() {} // precxx17-note {{not viable: requires 0 arguments}}
     A(A<T,U> &o) {} // precxx17-note {{not viable: expects an lvalue}}
-    A(A<T,T> o) {} // precxx17-note {{ignored: instantiation takes its own class type by value}}
+    A(A<T,T> o) {} // expected-error{{copy constructor must pass its first argument by reference}}
   };
 
   void helper_A(A<int,int>); // precxx17-note {{passing argument to parameter here}}
   void test_A() {
-    helper_A(A<int,int>()); // precxx17-error {{no matching constructor}}
+    helper_A(A<int,int>()); // precxx17-error {{no matching constructor}} \
+                            // expected-note{{in instantiation of template class 'self_by_value_2::A<int, int>'}}
   }
 }
 
@@ -169,11 +170,11 @@ namespace self_by_value_3 {
   template <class T, class U> struct A {
     A() {}
     A(A<T,U> &o) {}
-    A(A<T,T> o) {}
+    A(A<T,T> o) {} // expected-error{{copy constructor must pass its first argument by reference}}
   };
 
   void helper_A(A<int,int>);
-  void test_A(A<int,int> b) {
+  void test_A(A<int,int> b) { // expected-note{{in instantiation of template class 'self_by_value_3::A<int, int>'}}
     helper_A(b);
   }
 }

@cor3ntin
Copy link
Contributor

I think you need to rebase!

@Megan0704-1 Megan0704-1 force-pushed the fix-80963-formatting branch from 08c7423 to a8d4f18 Compare March 13, 2025 17:14
@Megan0704-1 Megan0704-1 force-pushed the fix-80963-formatting branch from a8d4f18 to 64ee153 Compare March 13, 2025 17:33
@Megan0704-1
Copy link
Contributor Author

I rebased the PR, thank you again for all your feedbacks!

@cor3ntin cor3ntin merged commit e93e0dd into llvm:main Mar 13, 2025
7 of 10 checks passed
frederik-h pushed a commit to frederik-h/llvm-project that referenced this pull request Mar 18, 2025
This PR fixes formatting issues in `constructor-template.cpp` introduced
in llvm#130866.

Changes:  
- Ran `clang-format` to adhere to LLVM style guidelines.  
- No functional changes.  

CC: @cor3ntin @shafik 
Thanks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants