[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D53860#1280959, @twoh wrote:

> For me it seems that the bug is checking destructor accessibility of Base itself, not fields of Base.


The code is correct (or at least, following the current standard rule) as-is. The base class subobject itself is an element of the aggregate. Note that aggregate-initialization of `Derived` does not require `Base` to be an aggregate at all, so recursing to the fields of `Base` would be inappropriate.

If this is causing significant problems for real code, we can certainly try to take this back to the C++ committee and request a do-over, but I think it's unlikely the rule would be changed: for aggregate initialization, direct base class subobjects are (by design) treated exactly the same as fields, and the destructors for all such subobjects are potentially invoked by aggregate initialization (because if an exception is thrown after an element is initialized and before initialization of the complete object finishes, the element's destructor will be invoked). You could argue that the context for the access check should be the derived class and not the context in which aggregate initialization is performed. I've checked the minutes, and we did indeed discuss that when resolving C++ DR 2227, and decided that the context for the access check should be the context of the aggregate initialization, not the context of the class definition.

For what it's worth, this is not the only way that the C++17 extensions to aggregate initialization break existing code. There are easy workarounds: make sure that your class is not an aggregate (add a constructor), or don't use list-initialization.


Repository:
  rC Clang

https://reviews.llvm.org/D53860