[PATCH] D45898: [SemaCXX] Mark destructor as referenced

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 30 19:00:06 PDT 2018


rsmith added inline comments.


================
Comment at: lib/Sema/SemaInit.cpp:830
+
+  for (FieldDecl *FD : CXXRD->fields())
+    if (auto *CXXRDMember = FD->getType()->getAsCXXRecordDecl())
----------------
rsmith wrote:
> I think this now checks too much: only those subobjects for which we formed an `InitListExpr` should be checked. Testcase:
> 
> ```
> struct A { friend struct B; private: ~A(); };
> struct B { B(); A a; };
> struct C { B b; };
> C c = { B(); };
> ```
> 
> Here, we must not check that `A`'s destructor is accessible from the context of `C`'s initialization.
> 
> I think the best thing to do is probably to move the call to this function into `InitListChecker::CheckStructUnionTypes`, and make it just check one level.
You also need to check base classes (since C++17, aggregates can have base classes).


================
Comment at: lib/Sema/SemaInit.cpp:830-834
+  for (FieldDecl *FD : CXXRD->fields())
+    if (auto *CXXRDMember = FD->getType()->getAsCXXRecordDecl())
+      if (!checkMemberDestructor(CXXRDMember, FD->getType(), Loc, true,
+                                 SemaRef))
+        return false;
----------------
I think this now checks too much: only those subobjects for which we formed an `InitListExpr` should be checked. Testcase:

```
struct A { friend struct B; private: ~A(); };
struct B { B(); A a; };
struct C { B b; };
C c = { B(); };
```

Here, we must not check that `A`'s destructor is accessible from the context of `C`'s initialization.

I think the best thing to do is probably to move the call to this function into `InitListChecker::CheckStructUnionTypes`, and make it just check one level.


================
Comment at: lib/Sema/SemaInit.cpp:831
+  for (FieldDecl *FD : CXXRD->fields())
+    if (auto *CXXRDMember = FD->getType()->getAsCXXRecordDecl())
+      if (!checkMemberDestructor(CXXRDMember, FD->getType(), Loc, true,
----------------
Do we need a `getBaseElementType()` here? (What if the member is of array-of-class type?)


Repository:
  rC Clang

https://reviews.llvm.org/D45898





More information about the cfe-commits mailing list