[PATCH] D100057: Remove warning "suggest braces" for aggregate initialization of an empty class with an aggregate base class.

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

rsmith accepted this revision.
rsmith added a comment.

Thanks, I think this makes sense as a generalization of the existing "idiomatic aggregate initialization" rule. Some suggested cleanups but otherwise LGTM.



================
Comment at: clang/lib/Sema/SemaInit.cpp:1001
 /// the braces in aggregate initialization.
 static bool isIdiomaticBraceElisionEntity(const InitializedEntity &Entity) {
   // Recursive initialization of the one and only field within an aggregate
----------------
I wonder if it'd be clearer to replace the one base and no fields check and the one field and no bases check with something like:

```
  // Brace elision is idiomatic if we're initializing the only direct subobject of
  // an aggregate of record type.
  if (Entity.getKind() == InitializedEntity::EK_Base ||
      Entity.getKind() == InitializedEntity::EK_Member) {
    auto *ParentRD = Entity.getParent()->getType()->getAsRecordDecl();
    unsigned Subobjects = std::distance(ParentRD->field_begin(), ParentRD->field_end());
    if (auto *CXXRD = dyn_cast<CXXRecordDecl>(ParentRD))
      Subobjects += CXXRD->getNumBases();
    return Subobjects == 1;
  }
```

This'd be linear in the number of fields instead of constant-time, but I suspect that doesn't matter in practice. Aggregate initialization is linear in the number of fields regardless.

(No strong preference here; take this or leave it.)


================
Comment at: clang/lib/Sema/SemaInit.cpp:1017-1021
+    if (CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(ParentRD)) {
+      if (CXXRD->getNumBases() == 1) {
+        return ParentRD->field_begin() == ParentRD->field_end();
+      }
+    }
----------------
Some minor simplifications. (We can `cast` rather than `dyn_cast`ing here because we're initializing a base class, so the parent *must* be a C++ class type.)


================
Comment at: clang/lib/Sema/SemaInit.cpp:1025
 
-  auto FieldIt = ParentRD->field_begin();
-  assert(FieldIt != ParentRD->field_end() &&
-         "no fields but have initializer for member?");
-  return ++FieldIt == ParentRD->field_end();
+  // Allows elide brace initialization for aggregates with one member.
+  if (Entity.getKind() == InitializedEntity::EK_Member) {
----------------



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

https://reviews.llvm.org/D100057