[PATCH] D145851: [Clang][Sema] Fix incorrect deletion of default constructors for some unions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:23
+
+// CHECK:      define dso_local void @_Z1fv() {{.*}} {
+// CHECK-NEXT: entry:
----------------



================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:24
+// CHECK:      define dso_local void @_Z1fv() {{.*}} {
+// CHECK-NEXT: entry:
+// CHECK:        call void @_ZN8UnionIntC1Ev{{.*}}
----------------
Don't need the `entry:` line,


================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:25
+// CHECK-NEXT: entry:
+// CHECK:        call void @_ZN8UnionIntC1Ev{{.*}}
+// CHECK-NEXT:   call void @_ZN15UnionNonTrivialC1Ev{{.*}}
----------------
Wildcards are implicit at the end of a line.


================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:26
+// CHECK:        call void @_ZN8UnionIntC1Ev{{.*}}
+// CHECK-NEXT:   call void @_ZN15UnionNonTrivialC1Ev{{.*}}
+// CHECK-NEXT:   ret void
----------------



================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:27
+// CHECK-NEXT:   call void @_ZN15UnionNonTrivialC1Ev{{.*}}
+// CHECK-NEXT:   ret void
+// CHECK-NEXT: }
----------------
This and the line after it aren't really necessary/valuable, particularly in case we get debug info/builtin/etc added later.


================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:30
+
+// CHECK:      define {{.*}} void @_ZN8UnionIntC1Ev{{.*}}
+// CHECK-NEXT: entry:
----------------
I typically suggest not requiring a space either before or after a `{{.*}}` block, since that wont match 'empty'.


================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:31
+// CHECK:      define {{.*}} void @_ZN8UnionIntC1Ev{{.*}}
+// CHECK-NEXT: entry:
+// CHECK:        call void @_ZN8UnionIntC2Ev{{.*}}
----------------
Same here, skip the entry , ret, and closing curley brace line.


================
Comment at: clang/test/CodeGen/union-non-trivial-member.cpp:32
+// CHECK-NEXT: entry:
+// CHECK:        call void @_ZN8UnionIntC2Ev{{.*}}
+// CHECK-NEXT:   ret void
----------------
same advise as above on the following ones too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145851/new/

https://reviews.llvm.org/D145851