llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

<details>
<summary>Changes</summary>

That check doesn't seem very useful. For non-dependent context records, 
ShouldDeleteSpecialMember is called when checking implicitly defined member 
functions, before the anonymous flag which the check relies on is set. (One 
could notice that in `ParseCXXClassMemberDeclaration`, 
`ParseDeclarationSpecifiers` ends up calling `ShouldDeleteSpecialMember`, while 
the flag is only set later in `ParsedFreeStandingDeclSpec`.)

For dependent contexts, this check actually breaks correctness: since we don't 
create those special members until the template is instantiated, their deletion 
checks are skipped because of the anonymity.

There's only one regression in ObjC test about notes; we are more explanative 
now.

Fixes https://github.com/llvm/llvm-project/issues/167217

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


4 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (-7) 
- (modified) clang/test/SemaCXX/anonymous-struct.cpp (+31) 
- (modified) clang/test/SemaObjCXX/arc-0x.mm (+1-1) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1cd465e25947a..374bfe3c5232f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -587,6 +587,7 @@ Bug Fixes to C++ Support
 - Diagnose unresolved overload sets in non-dependent compound requirements. 
(#GH51246) (#GH97753)
 - Fix a crash when extracting unavailable member type from alias in template 
deduction. (#GH165560)
 - Fix incorrect diagnostics for lambdas with init-captures inside braced 
initializers. (#GH163498)
+- Fixed an issue where templates prevented nested anonymous records from 
checking the deletion of special members. (#GH167217)
 - Fixed spurious diagnoses of certain nested lambda expressions. (#GH149121) 
(#GH156579)
 
 Bug Fixes to AST Handling
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 3bc748969065a..6c45a5e3525a8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9899,13 +9899,6 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD,
     return true;
   }
 
-  // For an anonymous struct or union, the copy and assignment special members
-  // will never be used, so skip the check. For an anonymous union declared at
-  // namespace scope, the constructor and destructor are used.
-  if (CSM != CXXSpecialMemberKind::DefaultConstructor &&
-      CSM != CXXSpecialMemberKind::Destructor && 
RD->isAnonymousStructOrUnion())
-    return false;
-
   // C++11 [class.copy]p7, p18:
   //   If the class definition declares a move constructor or move assignment
   //   operator, an implicitly declared copy constructor or copy assignment
diff --git a/clang/test/SemaCXX/anonymous-struct.cpp 
b/clang/test/SemaCXX/anonymous-struct.cpp
index 5927f4e639043..7800434165cd7 100644
--- a/clang/test/SemaCXX/anonymous-struct.cpp
+++ b/clang/test/SemaCXX/anonymous-struct.cpp
@@ -221,3 +221,34 @@ namespace ForwardDeclaredMember {
     };
   };
 }
+
+#if __cplusplus >= 201103L
+namespace GH167217 {
+
+struct NonMovable {
+  NonMovable(const NonMovable&) = delete;
+};
+
+struct Wrapper {
+  struct {
+    NonMovable v;
+  };
+};
+
+static_assert(!__is_constructible(Wrapper, const Wrapper&), "");
+static_assert(!__is_constructible(Wrapper, Wrapper), "");
+
+template<class T>
+struct WrapperTmpl {
+  struct {
+    NonMovable v;
+  };
+};
+
+using Wrapper2 = WrapperTmpl<NonMovable>;
+
+static_assert(!__is_constructible(Wrapper2, const Wrapper2&), "");
+static_assert(!__is_constructible(Wrapper2, Wrapper2), "");
+
+}
+#endif
diff --git a/clang/test/SemaObjCXX/arc-0x.mm b/clang/test/SemaObjCXX/arc-0x.mm
index a7418eceb244b..cd941502dabf7 100644
--- a/clang/test/SemaObjCXX/arc-0x.mm
+++ b/clang/test/SemaObjCXX/arc-0x.mm
@@ -163,7 +163,7 @@ void test() {
   struct S1 {
     union {
       union { // expected-note-re {{copy constructor of 'S1' is implicitly 
deleted because field 'test_union::S1::(anonymous union)::(anonymous union at 
{{.*}})' has a deleted copy constructor}} expected-note-re {{copy assignment 
operator of 'S1' is implicitly deleted because field 
'test_union::S1::(anonymous union)::(anonymous union at {{.*}})' has a deleted 
copy assignment operator}} expected-note-re 4 {{'S1' is implicitly deleted 
because field 'test_union::S1::(anonymous union)::(anonymous union at {{.*}})' 
has a deleted}}
-        id f0; // expected-note-re 2 {{{{.*}} of '(anonymous union at {{.*}}' 
is implicitly deleted because variant field 'f0' is an ObjC pointer}}
+        id f0; // expected-note-re 6 {{{{.*}} of '(anonymous union at {{.*}}' 
is implicitly deleted because variant field 'f0' is an ObjC pointer}}
         char f1;
       };
       int f2;

``````````

</details>


https://github.com/llvm/llvm-project/pull/171799
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to