Implemented P0683R1 - Default member initializers for bit-fields

[hamzasood]

Summary

This patch implements P0683R1.

Member initialisers are allowed pre-C++11 as an extension. So I've also allowed bitfield member initialisers pre-C++2a as an extension (with appropriate warnings) for consistency.

include/clang/AST/Decl.h

 
 /// 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 Mutable : 1;
   mutable unsigned CachedFieldIndex : 31;
 
-  /// The kinds of value we can store in InitializerOrBitWidth.
+  /// \brief Expression representing the bit-width,
+  /// or nullptr if this field isn't a bitfield.
+  Expr *BitWidth;
+
+  /// The kinds of value we can store in InitStorage.
   ///
   /// Note that this is compatible with InClassInitStyle except for
   /// ISK_CapturedVLAType.
   enum InitStorageKind {
-    /// 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,
+    /// Nothing special, the pointer should be null.
+    ISK_Nothing = (unsigned) ICIS_NoInit,
 
     /// The pointer is an (optional due to delayed parsing) Expr*
     /// holding the copy-initializer.
     ISK_InClassCopyInit = (unsigned) ICIS_CopyInit,
 
     /// The pointer is an (optional due to delayed parsing) Expr*
     /// holding the list-initializer.
     ISK_InClassListInit = (unsigned) ICIS_ListInit,
 
     /// The pointer is a VariableArrayType* that's been captured;
     /// the enclosing context is a lambda or captured statement.
     ISK_CapturedVLAType,
   };
 
-  /// \brief Storage for either the bit-width, the in-class
-  /// initializer, or the captured variable length array bound.
+  /// \brief Storage for either 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.
+  /// exclusive with VLA captures.
   ///
   /// 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
   /// and attached.
   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");
-  }
+      Mutable(Mutable), CachedFieldIndex(0), BitWidth(BW),
+      InitStorage(nullptr, (InitStorageKind) InitStyle) { }
 
 public:
   static FieldDecl *Create(const ASTContext &C, DeclContext *DC,
                            SourceLocation StartLoc, SourceLocation IdLoc,
                            IdentifierInfo *Id, QualType T,
                            TypeSourceInfo *TInfo, Expr *BW, bool Mutable,
                            InClassInitStyle InitStyle);
 
   static FieldDecl *CreateDeserialized(ASTContext &C, unsigned ID);
   
   /// getFieldIndex - Returns the index of this field within its record,
   /// as appropriate for passing to ASTRecordLayout::getFieldOffset.
   unsigned getFieldIndex() const;
 
   /// isMutable - Determines whether this field is mutable (C++ only).
   bool isMutable() const { return Mutable; }
 
   /// \brief Determines whether this field is a bitfield.
   bool isBitField() const {
-    return InitStorage.getInt() == ISK_BitWidthOrNothing &&
-           InitStorage.getPointer() != nullptr;
+    return BitWidth != nullptr;
   }
 
   /// @brief Determines whether this is an unnamed bitfield.
   bool isUnnamedBitfield() const { return isBitField() && !getDeclName(); }
 
   /// isAnonymousStructOrUnion - Determines whether this field is a
   /// representative for an anonymous struct or union. Such fields are
   /// unnamed and are implicitly generated by the implementation to
   /// store the data for the anonymous union or struct.
   bool isAnonymousStructOrUnion() const;
 
   Expr *getBitWidth() const {
-    return isBitField()
-               ? static_cast<Expr *>(InitStorage.getPointer())
-               : nullptr;
+    return BitWidth;
   }
   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(InitStorage.getInt() != ISK_CapturedVLAType &&
+           BitWidth == nullptr &&
+           "bit width or captured type already set");
+    BitWidth = Width;
   }
 
   /// 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);
+    BitWidth = nullptr;
   }
 
   /// getInClassInitStyle - Get the kind of (C++11) in-class initializer which
   /// this field has.
   InClassInitStyle getInClassInitStyle() const {
     InitStorageKind storageKind = InitStorage.getInt();
     return (storageKind == ISK_CapturedVLAType
               ? ICIS_NoInit : (InClassInitStyle) storageKind);
return hasInClassInitializer()
                : nullptr;
   }
 
   /// 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");
+           "initializer or captured type already set");
     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(nullptr, ISK_Nothing);
   }
 
   /// \brief Determine whether this member captures the variable length array
   /// type.
   bool hasCapturedVLAType() const {
     return InitStorage.getInt() == ISK_CapturedVLAType;
   }
 

include/clang/Basic/DiagnosticGroups.td

 
 // Warnings for C++1y code which is not compatible with prior C++ standards.
 def CXXPre14Compat : DiagGroup<"c++98-c++11-compat">;
 def CXXPre14CompatPedantic : DiagGroup<"c++98-c++11-compat-pedantic",
                                        [CXXPre14Compat]>;
 def CXXPre1zCompat : DiagGroup<"c++98-c++11-c++14-compat">;
 def CXXPre1zCompatPedantic : DiagGroup<"c++98-c++11-c++14-compat-pedantic",
                                        [CXXPre1zCompat]>;
+def CXXPre2aCompat : DiagGroup<"c++98-c++11-c++14-c++1z-compat">;
+def CXXPre2aCompatPedantic : DiagGroup<"c++98-c++11-c++14-c++1z-compat-pedantic",
+                                       [CXXPre2aCompat]>;
 
 def CXX98CompatBindToTemporaryCopy :
   DiagGroup<"c++98-compat-bind-to-temporary-copy">;
 def CXX98CompatLocalTypeTemplateArgs :
   DiagGroup<"c++98-compat-local-type-template-args">;
 def CXX98CompatUnnamedTypeTemplateArgs :
   DiagGroup<"c++98-compat-unnamed-type-template-args">;
 

 // A warning group for warnings about using C++14 features as extensions in
 // earlier C++ versions.
 def CXX14 : DiagGroup<"c++14-extensions", [CXX14BinaryLiteral]>;
 
 // A warning group for warnings about using C++1z features as extensions in
 // earlier C++ versions.
 def CXX1z : DiagGroup<"c++1z-extensions">;
 
+// A warning group for warnings about using C++2a features as extensions in
+// earlier C++ versions.
+def CXX2a : DiagGroup<"c++2a-extensions">;
+
 def : DiagGroup<"c++0x-extensions", [CXX11]>;
 def : DiagGroup<"c++1y-extensions", [CXX14]>;
 
 def DelegatingCtorCycles :
   DiagGroup<"delegating-ctor-cycles">;
 
 // A warning group for warnings about using C11 features as extensions.
 def C11 : DiagGroup<"c11-extensions">;

include/clang/Basic/DiagnosticParseKinds.td

 
 // C++11 in-class member initialization
 def ext_nonstatic_member_init : ExtWarn<
   "in-class initialization of non-static data member is a C++11 extension">,
   InGroup<CXX11>;
 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 err_incomplete_array_member_init: Error<
   "array bound cannot be deduced from an in-class initializer">;
 
+// C++2a in-class bitfield member initialization
+def warn_cxx1z_compat_bitfield_member_init : Warning<
+  "in-class initialization of bit-field members is incompatible with "
+  "C++ standards before C++2a">, InGroup<CXXPre2aCompat>, DefaultIgnore;
+def ext_bitfield_member_init_cxx2a : ExtWarn<
+  "in-class initialization of bit-field members is a C++2a extension">,
+  InGroup<CXX2a>;
+
 // C++11 alias-declaration
 def ext_alias_declaration : ExtWarn<
   "alias declarations are a C++11 extension">, InGroup<CXX11>;
 def warn_cxx98_compat_alias_declaration : Warning<
   "alias declarations are incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
 def err_alias_declaration_not_identifier : Error<
   "name defined in alias declaration must be an identifier">;

lib/AST/Decl.cpp

bool FieldDecl::isAnonymousStructOrUnion() const {
   if (const auto *Record = getType()->getAs<RecordType>())
     return Record->getDecl()->isAnonymousStructOrUnion();
 
   return false;
 }
 
 unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
   assert(isBitField() && "not a bitfield");
-  auto *BitWidth = static_cast<Expr *>(InitStorage.getPointer());
   return BitWidth->EvaluateKnownConstInt(Ctx).getZExtValue();
 }
 
 unsigned FieldDecl::getFieldIndex() const {
   const FieldDecl *Canonical = getCanonicalDecl();
   if (Canonical != this)
     return Canonical->getFieldIndex();
 
   if (CachedFieldIndex) return CachedFieldIndex - 1;
 
   unsigned Index = 0;
   const RecordDecl *RD = getParent();
 
   for (auto *Field : RD->fields()) {
     Field->getCanonicalDecl()->CachedFieldIndex = Index + 1;
     ++Index;
   }
 
   assert(CachedFieldIndex && "failed to find field in parent");
   return CachedFieldIndex - 1;
 }
 
 SourceRange FieldDecl::getSourceRange() const {
   switch (InitStorage.getInt()) {
-  // All three of these cases store an optional Expr*.
-  case ISK_BitWidthOrNothing:
+  // Both of these cases store an optional Expr*.
   case ISK_InClassCopyInit:
   case ISK_InClassListInit:
     if (const auto *E = static_cast<const Expr *>(InitStorage.getPointer()))
       return SourceRange(getInnerLocStart(), E->getLocEnd());
     // FALLTHROUGH
 
+  case ISK_Nothing:
   case ISK_CapturedVLAType:
     return DeclaratorDecl::getSourceRange();
   }
   llvm_unreachable("bad init storage kind");
 }
 
 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_Nothing &&
          InitStorage.getPointer() == nullptr &&
-         "bit width, initializer or captured type already set");
+         "initializer or captured type already set");
   InitStorage.setPointerAndInt(const_cast<VariableArrayType *>(VLAType),
                                ISK_CapturedVLAType);
 }
 
 //===----------------------------------------------------------------------===//
 // TagDecl Implementation
 //===----------------------------------------------------------------------===//
 

lib/Parse/ParseDeclCXX.cpp

 ///         member-declarator
 ///         member-declarator-list ',' member-declarator
 ///
 ///       member-declarator:
 ///         declarator virt-specifier-seq[opt] pure-specifier[opt]
 ///         declarator constant-initializer[opt]
 /// [C++11] declarator brace-or-equal-initializer[opt]
 ///         identifier[opt] ':' constant-expression
+///             brace-or-equal-initializer[opt][C++2a]
 ///
 ///       virt-specifier-seq:
 ///         virt-specifier
 ///         virt-specifier-seq virt-specifier
 ///
 ///       virt-specifier:
 ///         override
 ///         final
Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
   // member-declarator-list:
   //   member-declarator
   //   member-declarator-list ',' member-declarator
 
   while (1) {
     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 (BitfieldSize.get())
+        Diag(Tok, !getLangOpts().CPlusPlus2a
+                      ? diag::ext_bitfield_member_init_cxx2a
+                      : diag::warn_cxx1z_compat_bitfield_member_init);
+
+      if (DeclaratorInfo.isDeclarationOfFunction()) {
         // It's a pure-specifier.
         if (!TryConsumePureSpecifier(/*AllowFunctionDefinition*/ false))
           // Parse it as an expression so that Sema can diagnose it.
           HasStaticInitializer = true;
       } else if (DeclaratorInfo.getDeclSpec().getStorageClassSpec() !=
                      DeclSpec::SCS_static &&
                  DeclaratorInfo.getDeclSpec().getStorageClassSpec() !=
                      DeclSpec::SCS_typedef &&

lib/Serialization/ASTReaderDecl.cpp

void ASTDeclReader::VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D) {
   D->IvarLoc = ReadSourceLocation();
   D->setGetterCXXConstructor(Record.readExpr());
   D->setSetterCXXAssignment(Record.readExpr());
 }
 
 void ASTDeclReader::VisitFieldDecl(FieldDecl *FD) {
   VisitDeclaratorDecl(FD);
   FD->Mutable = Record.readInt();
-  if (int BitWidthOrInitializer = Record.readInt()) {
+  FD->BitWidth = Record.readExpr();
+  if (int Initializer = Record.readInt()) {
     FD->InitStorage.setInt(
-          static_cast<FieldDecl::InitStorageKind>(BitWidthOrInitializer - 1));
+          static_cast<FieldDecl::InitStorageKind>(Initializer - 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 (!FD->getDeclName()) {

lib/Serialization/ASTWriterDecl.cpp

void ASTDeclWriter::VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D) {
   Record.AddStmt(D->getGetterCXXConstructor());
   Record.AddStmt(D->getSetterCXXAssignment());
   Code = serialization::DECL_OBJC_PROPERTY_IMPL;
 }
 
 void ASTDeclWriter::VisitFieldDecl(FieldDecl *D) {
   VisitDeclaratorDecl(D);
   Record.push_back(D->isMutable());
-  if (D->InitStorage.getInt() == FieldDecl::ISK_BitWidthOrNothing &&
-      D->InitStorage.getPointer() == nullptr) {
+  Record.AddStmt(D->getBitWidth());
+  if (D->InitStorage.getInt() == FieldDecl::ISK_Nothing) {
     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()));

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

+// RUN: %clang_cc1 -std=c++2a -verify %s
+// expected-no-diagnostics
+
+// This is a slightly modified version of the example from C++2a [class.mem]p7.
+// It tests the resolution of parsing ambiguities.
+
+int a;
+struct S {
+  unsigned x1 : 8 = 42;
+  unsigned x2 : 8 {42};
+
+  unsigned y1 : true ? 8 : a = 42;
+  unsigned y2 : (true ? 8 : a) = 42;
+
+  unsigned z1 : 1 || new int { 1 };
+  unsigned z2 : (1 || new int) { 1 };
+};
+
+constexpr S s{};
+static_assert(s.x1 == 42 && s.x2 == 42);
+static_assert(s.y1 == 0  && s.y2 == 42);
+static_assert(s.z1 == 0  && s.z2 == 1);

test/SemaCXX/member-init.cpp

 // 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 = 2; // expected-warning {{C++2a extension}}
 };
 
 int a;
 class NoWarning {
   int &n = a;
 public:
   int &GetN() { return n; }
 };

www/cxx_status.html

 ordering, resulting in ambiguity errors for reasonable and previously-valid
 code. This issue is expected to be rectified soon.
 </span>
 </p>
 </details>
 
 <h2 id="cxx20">C++2a implementation status</h2>
 
-<p>Clang does not yet support any of the proposed features of
-<!--<p>Clang has <b>experimental</b> support for some proposed features of-->
+<p>Clang has <b>experimental</b> support for some proposed features of
 the C++ standard following C++17, provisionally named C++2a.
 Note that support for these features may change or be removed without notice,
 as the draft C++2a standard evolves.
 
-<!--<p>You can use Clang in C++2a mode with the <code>-std=c++2a</code> option.</p>-->
+<p>You can use Clang in C++2a mode with the <code>-std=c++2a</code> option.</p>
 
 <details open>
 <summary>List of features and minimum Clang version with support</summary>
 
 <table width="689" border="1" cellspacing="0">
  <tr>
     <th>Language Feature</th>
     <th>C++2a Proposal</th>
     <th>Available in Clang?</th>
  </tr>
     <!-- Toronto 2017 papers -->
     <tr>
       <td>Default member initializers for bit-fields</td>
       <td><a href="http://wg21.link/p0683r1">P0683R1</a></td>
-      <td class="none" align="center">No</td>
+      <td class="svn" align="center">SVN</td>
     </tr>
     <tr>
       <td><tt>const&amp;</tt>-qualified pointers to members</td>
       <td><a href="http://wg21.link/p0704r1">P0704R1</a></td>
       <td class="none" align="center">No</td>
     </tr>
     <tr>
       <td>Allow <i>lambda-capture</i> <tt>[=, this]</tt></td>