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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to