[c++2a] P0683R1: Permit default member initializers for bit-fields.

This would be trivial, except that our in-memory and serialized representations
for FieldDecls assumed that this can't happen.

llvm-svn: 311867

Author: zygoloid

clang/include/clang/AST/Decl.h

@@ -2364,9 +2364,9 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext,
 /// FieldDecl - An instance of this class is created by Sema::ActOnField to
 /// represent a member of a struct/union/class.
 class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
-  // FIXME: This can be packed into the bitfields in Decl.
+  unsigned BitField : 1;
   unsigned Mutable : 1;
-  mutable unsigned CachedFieldIndex : 31;
+  mutable unsigned CachedFieldIndex : 30;
 
   /// The kinds of value we can store in InitializerOrBitWidth.
   ///
@@ -2376,7 +2376,7 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
     /// If the pointer is null, there's nothing special.  Otherwise,
     /// this is a bitfield and the pointer is the Expr* storing the
     /// bit-width.
-    ISK_BitWidthOrNothing = (unsigned) ICIS_NoInit,
+    ISK_NoInit = (unsigned) ICIS_NoInit,
 
     /// The pointer is an (optional due to delayed parsing) Expr*
     /// holding the copy-initializer.
@@ -2391,27 +2391,34 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
     ISK_CapturedVLAType,
   };
 
-  /// \brief Storage for either the bit-width, the in-class
-  /// initializer, or the captured variable length array bound.
-  ///
-  /// We can safely combine these because in-class initializers are
-  /// not permitted for bit-fields, and both are exclusive with VLA
-  /// captures.
+  /// If this is a bitfield with a default member initializer, this
+  /// structure is used to represent the two expressions.
+  struct InitAndBitWidth {
+    Expr *Init;
+    Expr *BitWidth;
+  };
+
+  /// \brief Storage for either the bit-width, the in-class initializer, or
+  /// both (via InitAndBitWidth), or the captured variable length array bound.
   ///
   /// If the storage kind is ISK_InClassCopyInit or
   /// ISK_InClassListInit, but the initializer is null, then this
-  /// field has an in-class initializer which has not yet been parsed
+  /// field has an in-class initializer that has not yet been parsed
   /// and attached.
+  // FIXME: Tail-allocate this to reduce the size of FieldDecl in the
+  // overwhelmingly common case that we have none of these things.
   llvm::PointerIntPair<void *, 2, InitStorageKind> InitStorage;
+
 protected:
   FieldDecl(Kind DK, DeclContext *DC, SourceLocation StartLoc,
             SourceLocation IdLoc, IdentifierInfo *Id,
             QualType T, TypeSourceInfo *TInfo, Expr *BW, bool Mutable,
             InClassInitStyle InitStyle)
     : DeclaratorDecl(DK, DC, IdLoc, Id, T, TInfo, StartLoc),
-      Mutable(Mutable), CachedFieldIndex(0),
-      InitStorage(BW, (InitStorageKind) InitStyle) {
-    assert((!BW || InitStyle == ICIS_NoInit) && "got initializer for bitfield");
+      BitField(false), Mutable(Mutable), CachedFieldIndex(0),
+      InitStorage(nullptr, (InitStorageKind) InitStyle) {
+    if (BW)
+      setBitWidth(BW);
   }
 
 public:
@@ -2431,10 +2438,7 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
   bool isMutable() const { return Mutable; }
 
   /// \brief Determines whether this field is a bitfield.
-  bool isBitField() const {
-    return InitStorage.getInt() == ISK_BitWidthOrNothing &&
-           InitStorage.getPointer() != nullptr;
-  }
+  bool isBitField() const { return BitField; }
 
   /// @brief Determines whether this is an unnamed bitfield.
   bool isUnnamedBitfield() const { return isBitField() && !getDeclName(); }
@@ -2446,66 +2450,76 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
   bool isAnonymousStructOrUnion() const;
 
   Expr *getBitWidth() const {
-    return isBitField()
-               ? static_cast<Expr *>(InitStorage.getPointer())
-               : nullptr;
+    if (!BitField)
+      return nullptr;
+    void *Ptr = InitStorage.getPointer();
+    if (getInClassInitStyle())
+      return static_cast<InitAndBitWidth*>(Ptr)->BitWidth;
+    return static_cast<Expr*>(Ptr);
   }
   unsigned getBitWidthValue(const ASTContext &Ctx) const;
 
   /// setBitWidth - Set the bit-field width for this member.
   // Note: used by some clients (i.e., do not remove it).
   void setBitWidth(Expr *Width) {
-    assert(InitStorage.getInt() == ISK_BitWidthOrNothing &&
-           InitStorage.getPointer() == nullptr &&
-           "bit width, initializer or captured type already set");
-    InitStorage.setPointerAndInt(Width, ISK_BitWidthOrNothing);
+    assert(!hasCapturedVLAType() && !BitField &&
+           "bit width or captured type already set");
+    assert(Width && "no bit width specified");
+    InitStorage.setPointer(
+        InitStorage.getInt()
+            ? new (getASTContext())
+                  InitAndBitWidth{getInClassInitializer(), Width}
+            : static_cast<void*>(Width));
+    BitField = true;
   }
 
   /// removeBitWidth - Remove the bit-field width from this member.
   // Note: used by some clients (i.e., do not remove it).
   void removeBitWidth() {
     assert(isBitField() && "no bitfield width to remove");
-    InitStorage.setPointerAndInt(nullptr, ISK_BitWidthOrNothing);
+    InitStorage.setPointer(getInClassInitializer());
+    BitField = false;
   }
 
-  /// getInClassInitStyle - Get the kind of (C++11) in-class initializer which
-  /// this field has.
+  /// Get the kind of (C++11) default member initializer that this field has.
   InClassInitStyle getInClassInitStyle() const {
     InitStorageKind storageKind = InitStorage.getInt();
     return (storageKind == ISK_CapturedVLAType
               ? ICIS_NoInit : (InClassInitStyle) storageKind);
   }
 
-  /// hasInClassInitializer - Determine whether this member has a C++11 in-class
-  /// initializer.
+  /// Determine whether this member has a C++11 default member initializer.
   bool hasInClassInitializer() const {
     return getInClassInitStyle() != ICIS_NoInit;
   }
 
-  /// getInClassInitializer - Get the C++11 in-class initializer for this
-  /// member, or null if one has not been set. If a valid declaration has an
-  /// in-class initializer, but this returns null, then we have not parsed and
-  /// attached it yet.
+  /// Get the C++11 default member initializer for this member, or null if one
+  /// has not been set. If a valid declaration has a default member initializer,
+  /// but this returns null, then we have not parsed and attached it yet.
   Expr *getInClassInitializer() const {
-    return hasInClassInitializer()
-               ? static_cast<Expr *>(InitStorage.getPointer())
-               : nullptr;
+    if (!hasInClassInitializer())
+      return nullptr;
+    void *Ptr = InitStorage.getPointer();
+    if (BitField)
+      return static_cast<InitAndBitWidth*>(Ptr)->Init;
+    return static_cast<Expr*>(Ptr);
   }
 
   /// setInClassInitializer - Set the C++11 in-class initializer for this
   /// member.
   void setInClassInitializer(Expr *Init) {
-    assert(hasInClassInitializer() &&
-           InitStorage.getPointer() == nullptr &&
-           "bit width, initializer or captured type already set");
-    InitStorage.setPointer(Init);
+    assert(hasInClassInitializer() && !getInClassInitializer());
+    if (BitField)
+      static_cast<InitAndBitWidth*>(InitStorage.getPointer())->Init = Init;
+    else
+      InitStorage.setPointer(Init);
   }
 
   /// removeInClassInitializer - Remove the C++11 in-class initializer from this
   /// member.
   void removeInClassInitializer() {
     assert(hasInClassInitializer() && "no initializer to remove");
-    InitStorage.setPointerAndInt(nullptr, ISK_BitWidthOrNothing);
+    InitStorage.setPointerAndInt(getBitWidth(), ISK_NoInit);
   }
 
   /// \brief Determine whether this member captures the variable length array

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -730,8 +730,12 @@ def ext_nonstatic_member_init : ExtWarn<
 def warn_cxx98_compat_nonstatic_member_init : Warning<
   "in-class initialization of non-static data members is incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
-def err_bitfield_member_init: Error<
-  "bit-field member cannot have an in-class initializer">;
+def ext_bitfield_member_init: ExtWarn<
+  "default member initializer for bit-field is a C++2a extension">,
+  InGroup<CXX2a>;
+def warn_cxx17_compat_bitfield_member_init: Warning<
+  "default member initializer for bit-field is incompatible with "
+  "C++ standards before C++2a">, InGroup<CXXPre2aCompat>, DefaultIgnore;
 def err_incomplete_array_member_init: Error<
   "array bound cannot be deduced from an in-class initializer">;
 

clang/lib/AST/Decl.cpp

@@ -3553,8 +3553,7 @@ bool FieldDecl::isAnonymousStructOrUnion() const {
 
 unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
   assert(isBitField() && "not a bitfield");
-  auto *BitWidth = static_cast<Expr *>(InitStorage.getPointer());
-  return BitWidth->EvaluateKnownConstInt(Ctx).getZExtValue();
+  return getBitWidth()->EvaluateKnownConstInt(Ctx).getZExtValue();
 }
 
 unsigned FieldDecl::getFieldIndex() const {
@@ -3577,25 +3576,18 @@ unsigned FieldDecl::getFieldIndex() const {
 }
 
 SourceRange FieldDecl::getSourceRange() const {
-  switch (InitStorage.getInt()) {
-  // All three of these cases store an optional Expr*.
-  case ISK_BitWidthOrNothing:
-  case ISK_InClassCopyInit:
-  case ISK_InClassListInit:
-    if (const auto *E = static_cast<const Expr *>(InitStorage.getPointer()))
-      return SourceRange(getInnerLocStart(), E->getLocEnd());
-    // FALLTHROUGH
-
-  case ISK_CapturedVLAType:
-    return DeclaratorDecl::getSourceRange();
-  }
-  llvm_unreachable("bad init storage kind");
+  const Expr *FinalExpr = getInClassInitializer();
+  if (!FinalExpr)
+    FinalExpr = getBitWidth();
+  if (FinalExpr)
+    return SourceRange(getInnerLocStart(), FinalExpr->getLocEnd());
+  return DeclaratorDecl::getSourceRange();
 }
 
 void FieldDecl::setCapturedVLAType(const VariableArrayType *VLAType) {
   assert((getParent()->isLambda() || getParent()->isCapturedRecord()) &&
          "capturing type in non-lambda or captured record.");
-  assert(InitStorage.getInt() == ISK_BitWidthOrNothing &&
+  assert(InitStorage.getInt() == ISK_NoInit &&
          InitStorage.getPointer() == nullptr &&
          "bit width, initializer or captured type already set");
   InitStorage.setPointerAndInt(const_cast<VariableArrayType *>(VLAType),

clang/lib/Parse/ParseDeclCXX.cpp

@@ -2720,10 +2720,7 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
     InClassInitStyle HasInClassInit = ICIS_NoInit;
     bool HasStaticInitializer = false;
     if (Tok.isOneOf(tok::equal, tok::l_brace) && PureSpecLoc.isInvalid()) {
-      if (BitfieldSize.get()) {
-        Diag(Tok, diag::err_bitfield_member_init);
-        SkipUntil(tok::comma, StopAtSemi | StopBeforeMatch);
-      } else if (DeclaratorInfo.isDeclarationOfFunction()) {
+      if (DeclaratorInfo.isDeclarationOfFunction()) {
         // It's a pure-specifier.
         if (!TryConsumePureSpecifier(/*AllowFunctionDefinition*/ false))
           // Parse it as an expression so that Sema can diagnose it.
@@ -2734,6 +2731,10 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
                      DeclSpec::SCS_typedef &&
                  !DS.isFriendSpecified()) {
         // It's a default member initializer.
+        if (BitfieldSize.get())
+          Diag(Tok, getLangOpts().CPlusPlus2a
+                        ? diag::warn_cxx17_compat_bitfield_member_init
+                        : diag::ext_bitfield_member_init);
         HasInClassInit = Tok.is(tok::equal) ? ICIS_CopyInit : ICIS_ListInit;
       } else {
         HasStaticInitializer = true;

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -1219,16 +1219,17 @@ void ASTDeclReader::VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D) {
 void ASTDeclReader::VisitFieldDecl(FieldDecl *FD) {
   VisitDeclaratorDecl(FD);
   FD->Mutable = Record.readInt();
-  if (int BitWidthOrInitializer = Record.readInt()) {
-    FD->InitStorage.setInt(
-          static_cast<FieldDecl::InitStorageKind>(BitWidthOrInitializer - 1));
-    if (FD->InitStorage.getInt() == FieldDecl::ISK_CapturedVLAType) {
-      // Read captured variable length array.
-      FD->InitStorage.setPointer(Record.readType().getAsOpaquePtr());
-    } else {
-      FD->InitStorage.setPointer(Record.readExpr());
-    }
+
+  if (auto ISK = static_cast<FieldDecl::InitStorageKind>(Record.readInt())) {
+    FD->InitStorage.setInt(ISK);
+    FD->InitStorage.setPointer(ISK == FieldDecl::ISK_CapturedVLAType
+                                   ? Record.readType().getAsOpaquePtr()
+                                   : Record.readExpr());
   }
+
+  if (auto *BW = Record.readExpr())
+    FD->setBitWidth(BW);
+
   if (!FD->getDeclName()) {
     if (FieldDecl *Tmpl = ReadDeclAs<FieldDecl>())
       Reader.getContext().setInstantiatedFromUnnamedFieldDecl(FD, Tmpl);

clang/lib/Serialization/ASTWriterDecl.cpp

@@ -849,17 +849,16 @@ void ASTDeclWriter::VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D) {
 void ASTDeclWriter::VisitFieldDecl(FieldDecl *D) {
   VisitDeclaratorDecl(D);
   Record.push_back(D->isMutable());
-  if (D->InitStorage.getInt() == FieldDecl::ISK_BitWidthOrNothing &&
-      D->InitStorage.getPointer() == nullptr) {
-    Record.push_back(0);
-  } else if (D->InitStorage.getInt() == FieldDecl::ISK_CapturedVLAType) {
-    Record.push_back(D->InitStorage.getInt() + 1);
-    Record.AddTypeRef(
-        QualType(static_cast<Type *>(D->InitStorage.getPointer()), 0));
-  } else {
-    Record.push_back(D->InitStorage.getInt() + 1);
-    Record.AddStmt(static_cast<Expr *>(D->InitStorage.getPointer()));
-  }
+
+  FieldDecl::InitStorageKind ISK = D->InitStorage.getInt();
+  Record.push_back(ISK);
+  if (ISK == FieldDecl::ISK_CapturedVLAType)
+    Record.AddTypeRef(QualType(D->getCapturedVLAType(), 0));
+  else if (ISK)
+    Record.AddStmt(D->getInClassInitializer());
+
+  Record.AddStmt(D->getBitWidth());
+
   if (!D->getDeclName())
     Record.AddDeclRef(Context.getInstantiatedFromUnnamedFieldDecl(D));
 
@@ -873,6 +872,7 @@ void ASTDeclWriter::VisitFieldDecl(FieldDecl *D) {
       !D->isModulePrivate() &&
       !D->getBitWidth() &&
       !D->hasInClassInitializer() &&
+      !D->hasCapturedVLAType() &&
       !D->hasExtInfo() &&
       !ObjCIvarDecl::classofKind(D->getKind()) &&
       !ObjCAtDefsFieldDecl::classofKind(D->getKind()) &&
@@ -1741,7 +1741,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(0));                       // hasExtInfo
   // FieldDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isMutable
-  Abv->Add(BitCodeAbbrevOp(0));                       //getBitWidth
+  Abv->Add(BitCodeAbbrevOp(0));                       // InitStyle
   // Type Source Info
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
@@ -1774,7 +1774,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(0));                       // hasExtInfo
   // FieldDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isMutable
-  Abv->Add(BitCodeAbbrevOp(0));                       //getBitWidth
+  Abv->Add(BitCodeAbbrevOp(0));                       // InitStyle
   // ObjC Ivar
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // getAccessControl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // getSynthesize

clang/test/PCH/cxx2a-bitfield-init.cpp

@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c++2a -include %s -verify %s
+// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s -DPCH
+
+#ifndef HEADER
+#define HEADER
+
+struct S {
+  unsigned int n : 5 = 49; // expected-warning {{changes value}}
+};
+
+int a;
+template<bool B> struct T {
+  int m : B ? 8 : a = 42;
+};
+
+#else
+
+// expected-error@-5 {{constant expression}} expected-note@-5 {{cannot modify}}
+
+static_assert(S().n == 17);
+static_assert(T<true>().m == 0);
+int q = T<false>().m; // expected-note {{instantiation of}}
+
+#endif

clang/test/Parser/cxx2a-bitfield-init.cpp

@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++2a -verify %s
+
+namespace std_example {
+  int a;
+  const int b = 0; // expected-note {{here}}
+  struct S {
+    int x1 : 8 = 42;
+    int x2 : 8 { 42 };
+    int y1 : true ? 8 : a = 42;
+    int y3 : (true ? 8 : b) = 42;
+    int z : 1 || new int { 1 };
+  };
+  static_assert(S{}.x1 == 42);
+  static_assert(S{}.x2 == 42);
+  static_assert(S{}.y1 == 0);
+  static_assert(S{}.y3 == 42);
+  static_assert(S{}.z == 0);
+
+  struct T {
+    int y2 : true ? 8 : b = 42; // expected-error {{cannot assign to variable 'b'}}
+  };
+}

clang/test/SemaCXX/member-init.cpp

@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -verify -std=c++11 -Wall %s
 
 struct Bitfield {
-  int n : 3 = 7; // expected-error {{bit-field member cannot have an in-class initializer}}
+  int n : 3 = 7; // expected-warning {{C++2a extension}} expected-warning {{changes value from 7 to -1}}
 };
 
 int a;