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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D1000... Hana Dusíková via Phabricator via cfe-commits
    • [PATCH] ... Michael Schellenberger Costa via Phabricator via cfe-commits
    • [PATCH] ... Hana Dusíková via Phabricator via cfe-commits
    • [PATCH] ... Roman Lebedev via Phabricator via cfe-commits
    • [PATCH] ... Hana Dusíková via Phabricator via cfe-commits
    • [PATCH] ... JF Bastien via Phabricator via cfe-commits
    • [PATCH] ... Hana Dusíková via Phabricator via cfe-commits
    • [PATCH] ... Hana Dusíková via Phabricator via cfe-commits
    • [PATCH] ... Louis Dionne via Phabricator via cfe-commits
    • [PATCH] ... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] ... Hana Dusíková via Phabricator via cfe-commits
    • [PATCH] ... Aaron Ballman via Phabricator via cfe-commits
    • [PATCH] ... Aaron Ballman via Phabricator via cfe-commits

Reply via email to