[clang] 64c24f4 - Remove warning "suggest braces" for aggregate initialization of an empty class with an aggregate base class.

[Aaron Ballman via cfe-commits]

Author: Hana Dusíková
Date: 2021-04-13T15:45:09-04:00
New Revision: 64c24f493e5f4637ee193f10f469cdd2695b4ba6

URL: https://github.com/llvm/llvm-project/commit/64c24f493e5f4637ee193f10f469cdd2695b4ba6
DIFF: https://github.com/llvm/llvm-project/commit/64c24f493e5f4637ee193f10f469cdd2695b4ba6.diff

LOG: Remove warning "suggest braces" for aggregate initialization of an empty class with an aggregate base class.

I recently ran into issues with aggregates and inheritance, I'm using
it for creating a type-safe library where most of the types are build
over "tagged" std::array. After bit of cleaning and enabling -Wall
-Wextra -pedantic I noticed clang only in my pipeline gives me warning.
After a bit of focusing on it I found it's not helpful, and contemplate
disabling the warning all together. After a discussion with other
library authors I found it's bothering more people and decided to fix
it.

Removes this warning:

template<typename T, int N> struct StdArray {
    T contents[N];
  };

template<typename T, int N> struct AggregateAndEmpty : StdArray<T,N> { };

AggregateAndEmpty<int, 3> p = {1, 2, 3}; // <-- warning here about omitted braces

Added: 
    

Modified: 
    clang/lib/Sema/SemaInit.cpp
    clang/test/SemaCXX/aggregate-initialization.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index ec7e4722ea4e..c8e8c8696b6a 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -1007,21 +1007,33 @@ static bool isIdiomaticBraceElisionEntity(const InitializedEntity &Entity) {
   //
   // (where std::array is an aggregate struct containing a single array field.
 
-  // FIXME: Should aggregate initialization of a struct with a single
-  // base class and no members also suppress the warning?
-  if (Entity.getKind() != InitializedEntity::EK_Member || !Entity.getParent())
+  if (!Entity.getParent())
     return false;
 
-  auto *ParentRD =
-      Entity.getParent()->getType()->castAs<RecordType>()->getDecl();
-  if (CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(ParentRD))
-    if (CXXRD->getNumBases())
-      return false;
+  // Allows elide brace initialization for aggregates with empty base.
+  if (Entity.getKind() == InitializedEntity::EK_Base) {
+    auto *ParentRD =
+        Entity.getParent()->getType()->castAs<RecordType>()->getDecl();
+    CXXRecordDecl *CXXRD = cast<CXXRecordDecl>(ParentRD);
+    return CXXRD->getNumBases() == 1 && CXXRD->field_empty();
+  }
+
+  // Allow brace elision if the only subobject is a field.
+  if (Entity.getKind() == InitializedEntity::EK_Member) {
+    auto *ParentRD =
+        Entity.getParent()->getType()->castAs<RecordType>()->getDecl();
+    if (CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(ParentRD)) {
+      if (CXXRD->getNumBases()) {
+        return false;
+      }
+    }
+    auto FieldIt = ParentRD->field_begin();
+    assert(FieldIt != ParentRD->field_end() &&
+           "no fields but have initializer for member?");
+    return ++FieldIt == ParentRD->field_end();
+  }
 
-  auto FieldIt = ParentRD->field_begin();
-  assert(FieldIt != ParentRD->field_end() &&
-         "no fields but have initializer for member?");
-  return ++FieldIt == ParentRD->field_end();
+  return false;
 }
 
 /// Check whether the range of the initializer \p ParentIList from element

diff  --git a/clang/test/SemaCXX/aggregate-initialization.cpp b/clang/test/SemaCXX/aggregate-initialization.cpp
index c71a7449d31d..3c12aca12d21 100644
--- a/clang/test/SemaCXX/aggregate-initialization.cpp
+++ b/clang/test/SemaCXX/aggregate-initialization.cpp
@@ -172,11 +172,20 @@ namespace IdiomaticStdArrayInitDoesNotWarn {
   };
   ArrayAndBaseClass<int, 3> z = {1, 2, 3}; // expected-warning {{suggest braces}}
 
-  // It's not clear whether we should be warning in this case. If this
-  // pattern becomes idiomatic, it would be reasonable to suppress the
-  // warning here too.
+  // This pattern is used for tagged aggregates and must not warn
   template<typename T, int N> struct JustABaseClass : StdArray<T, N> {};
-  JustABaseClass<int, 3> w = {1, 2, 3}; // expected-warning {{suggest braces}}
+  JustABaseClass<int, 3> w = {1, 2, 3};
+  // but this should be also ok
+  JustABaseClass<int, 3> v = {{1, 2, 3}};
+
+  template <typename T, int N> struct OnionBaseClass : JustABaseClass<T, N> {};
+  OnionBaseClass<int, 3> u = {1, 2, 3};
+  OnionBaseClass<int, 3> t = {{{1, 2, 3}}};
+
+  struct EmptyBase {};
+
+  template <typename T, int N> struct AggregateAndEmpty : StdArray<T, N>, EmptyBase {};
+  AggregateAndEmpty<int, 3> p = {1, 2, 3}; // expected-warning {{suggest braces}}
 #endif
 
 #pragma clang diagnostic pop