r331620 - Non-zero-length bit-fields make a class non-empty.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Sun May 6 23:43:30 PDT 2018


Author: rsmith
Date: Sun May  6 23:43:30 2018
New Revision: 331620

URL: http://llvm.org/viewvc/llvm-project?rev=331620&view=rev
Log:
Non-zero-length bit-fields make a class non-empty.

This implements the rule intended by the standard (see LWG 2358)
and the rule intended by the Itanium C++ ABI (see
https://github.com/itanium-cxx-abi/cxx-abi/pull/51), and makes
Clang match the behavior of GCC, ICC, and MSVC.

A pedantic reading of both the standard and the ABI indicate that Clang
is currently technically correct, but that's not worth much when it's
clear that the wording is wrong in both those places.

This is an ABI break for classes that derive from a class that is empty
other than one or more unnamed non-zero-length bit-fields. Such cases
are expected to be rare, but -fclang-abi-compat=6 restores the old
behavior just in case.

Differential Revision: https://reviews.llvm.org/D45174

Added:
    cfe/trunk/test/Layout/v6-empty.cpp
Modified:
    cfe/trunk/docs/ReleaseNotes.rst
    cfe/trunk/lib/AST/DeclCXX.cpp
    cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
    cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=331620&r1=331619&r2=331620&view=diff
==============================================================================
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Sun May  6 23:43:30 2018
@@ -84,6 +84,15 @@ Non-comprehensive list of changes in thi
   Clang 7 and earlier versions. The old behavior can be restored by setting
   ``-fclang-abi-compat`` to ``6`` or earlier.
 
+- Clang implements the proposed resolution of LWG issue 2358, along with the
+  `corresponding change to the Itanium C++ ABI
+  <https://github.com/itanium-cxx-abi/cxx-abi/pull/51>`_, which make classes
+  containing only unnamed non-zero-length bit-fields be considered non-empty.
+  This is an ABI break compared to prior Clang releases, but makes Clang
+  generate code that is ABI-compatible with other compilers. The old
+  behavior can be restored by setting ``-fclang-abi-compat`` to ``6`` or
+  lower.
+
 - ...
 
 New Compiler Flags

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=331620&r1=331619&r2=331620&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Sun May  6 23:43:30 2018
@@ -856,10 +856,10 @@ void CXXRecordDecl::addedMember(Decl *D)
     return;
   }
 
-  ASTContext &Context = getASTContext();
-
   // Handle non-static data members.
   if (const auto *Field = dyn_cast<FieldDecl>(D)) {
+    ASTContext &Context = getASTContext();
+
     // C++2a [class]p7:
     //   A standard-layout class is a class that:
     //    [...]
@@ -872,8 +872,16 @@ void CXXRecordDecl::addedMember(Decl *D)
     //   A declaration for a bit-field that omits the identifier declares an 
     //   unnamed bit-field. Unnamed bit-fields are not members and cannot be 
     //   initialized.
-    if (Field->isUnnamedBitfield())
+    if (Field->isUnnamedBitfield()) {
+      // C++ [meta.unary.prop]p4: [LWG2358]
+      //   T is a class type [...] with [...] no unnamed bit-fields of non-zero
+      //   length
+      if (data().Empty && !Field->isZeroLengthBitField(Context) &&
+          Context.getLangOpts().getClangABICompat() >
+              LangOptions::ClangABI::Ver6)
+        data().Empty = false;
       return;
+    }
     
     // C++11 [class]p7:
     //   A standard-layout class is a class that:
@@ -1220,12 +1228,8 @@ void CXXRecordDecl::addedMember(Decl *D)
     }
 
     // C++14 [meta.unary.prop]p4:
-    //   T is a class type [...] with [...] no non-static data members other
-    //   than bit-fields of length 0...
-    if (data().Empty) {
-      if (!Field->isZeroLengthBitField(Context))
-        data().Empty = false;
-    }
+    //   T is a class type [...] with [...] no non-static data members
+    data().Empty = false;
   }
   
   // Handle using declarations of conversion functions.

Modified: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=331620&r1=331619&r2=331620&view=diff
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Sun May  6 23:43:30 2018
@@ -188,12 +188,12 @@ struct YA {
 	__declspec(align(32)) char : 1;
 };
 // CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:    0 | struct YA (empty)
+// CHECK-NEXT:    0 | struct YA
 // CHECK-NEXT:0:0-0 |   char
 // CHECK-NEXT:      | [sizeof=32, align=32
 // CHECK-NEXT:      |  nvsize=32, nvalign=32]
 // CHECK-X64: *** Dumping AST Record Layout
-// CHECK-X64-NEXT:    0 | struct YA (empty)
+// CHECK-X64-NEXT:    0 | struct YA
 // CHECK-X64-NEXT:0:0-0 |   char
 // CHECK-X64-NEXT:      | [sizeof=32, align=32
 // CHECK-X64-NEXT:      |  nvsize=32, nvalign=32]
@@ -206,14 +206,14 @@ struct YB {
 // CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:    0 | struct YB
 // CHECK-NEXT:    0 |   char a
-// CHECK-NEXT:    1 |   struct YA b (empty)
+// CHECK-NEXT:    1 |   struct YA b
 // CHECK-NEXT:1:0-0 |     char
 // CHECK-NEXT:      | [sizeof=33, align=1
 // CHECK-NEXT:      |  nvsize=33, nvalign=1]
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64-NEXT:    0 | struct YB
 // CHECK-X64-NEXT:    0 |   char a
-// CHECK-X64-NEXT:    1 |   struct YA b (empty)
+// CHECK-X64-NEXT:    1 |   struct YA b
 // CHECK-X64-NEXT:1:0-0 |     char
 // CHECK-X64-NEXT:      | [sizeof=33, align=1
 // CHECK-X64-NEXT:      |  nvsize=33, nvalign=1]
@@ -223,12 +223,12 @@ struct YC {
 	__declspec(align(32)) char : 1;
 };
 // CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:    0 | struct YC (empty)
+// CHECK-NEXT:    0 | struct YC
 // CHECK-NEXT:0:0-0 |   char
 // CHECK-NEXT:      | [sizeof=32, align=32
 // CHECK-NEXT:      |  nvsize=32, nvalign=32]
 // CHECK-X64: *** Dumping AST Record Layout
-// CHECK-X64-NEXT:    0 | struct YC (empty)
+// CHECK-X64-NEXT:    0 | struct YC
 // CHECK-X64-NEXT:    0:0-0 |   char
 // CHECK-X64-NEXT:      | [sizeof=8, align=32
 // CHECK-X64-NEXT:      |  nvsize=8, nvalign=32]
@@ -241,14 +241,14 @@ struct YD {
 // CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:    0 | struct YD
 // CHECK-NEXT:    0 |   char a
-// CHECK-NEXT:    1 |   struct YC b (empty)
+// CHECK-NEXT:    1 |   struct YC b
 // CHECK-NEXT:1:0-0 |     char
 // CHECK-NEXT:      | [sizeof=33, align=1
 // CHECK-NEXT:      |  nvsize=33, nvalign=1]
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64-NEXT:    0 | struct YD
 // CHECK-X64-NEXT:    0 |   char a
-// CHECK-X64-NEXT:    1 |   struct YC b (empty)
+// CHECK-X64-NEXT:    1 |   struct YC b
 // CHECK-X64-NEXT:1:0-0 |     char
 // CHECK-X64-NEXT:      | [sizeof=9, align=1
 // CHECK-X64-NEXT:      |  nvsize=9, nvalign=1]
@@ -258,12 +258,12 @@ struct YE {
 	__declspec(align(32)) char : 1;
 };
 // CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:    0 | struct YE (empty)
+// CHECK-NEXT:    0 | struct YE
 // CHECK-NEXT:    0:0-0 |   char
 // CHECK-NEXT:      | [sizeof=4, align=32
 // CHECK-NEXT:      |  nvsize=4, nvalign=32]
 // CHECK-X64: *** Dumping AST Record Layout
-// CHECK-X64-NEXT:    0 | struct YE (empty)
+// CHECK-X64-NEXT:    0 | struct YE
 // CHECK-X64-NEXT:    0:0-0 |   char
 // CHECK-X64-NEXT:      | [sizeof=4, align=32
 // CHECK-X64-NEXT:      |  nvsize=4, nvalign=32]
@@ -276,14 +276,14 @@ struct YF {
 // CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:    0 | struct YF
 // CHECK-NEXT:    0 |   char a
-// CHECK-NEXT:    1 |   struct YE b (empty)
+// CHECK-NEXT:    1 |   struct YE b
 // CHECK-NEXT:1:0-0 |     char
 // CHECK-NEXT:      | [sizeof=5, align=1
 // CHECK-NEXT:      |  nvsize=5, nvalign=1]
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64-NEXT:    0 | struct YF
 // CHECK-X64-NEXT:    0 |   char a
-// CHECK-X64-NEXT:    1 |   struct YE b (empty)
+// CHECK-X64-NEXT:    1 |   struct YE b
 // CHECK-X64-NEXT:1:0-0 |     char
 // CHECK-X64-NEXT:      | [sizeof=5, align=1
 // CHECK-X64-NEXT:      |  nvsize=5, nvalign=1]

Added: cfe/trunk/test/Layout/v6-empty.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/v6-empty.cpp?rev=331620&view=auto
==============================================================================
--- cfe/trunk/test/Layout/v6-empty.cpp (added)
+++ cfe/trunk/test/Layout/v6-empty.cpp Sun May  6 23:43:30 2018
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -fclang-abi-compat=6 -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-V6
+// RUN: %clang_cc1 -fsyntax-only -fclang-abi-compat=7 -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-V7
+
+// In Clang 6 and before, we determined that Nonempty was empty, so we
+// applied EBO to it.
+struct Nonempty { int : 4; };
+struct A : Nonempty { int n; };
+int k = sizeof(A);
+
+// CHECK:*** Dumping AST Record Layout
+// CHECK:             0 | struct A
+// CHECK-V6-NEXT:     0 |   struct Nonempty (base) (empty)
+// CHECK-V7-NEXT:     0 |   struct Nonempty (base){{$}}
+// CHECK-NEXT:    0:0-3 |     int
+// CHECK-V6-NEXT:     0 |   int n
+// CHECK-V7-NEXT:     4 |   int n
+// CHECK-V6-NEXT:       | [sizeof=4, dsize=4, align=4,
+// CHECK-V6-NEXT:       |  nvsize=4, nvalign=4]
+// CHECK-V7-NEXT:       | [sizeof=8, dsize=8, align=4,
+// CHECK-V7-NEXT:       |  nvsize=8, nvalign=4]

Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=331620&r1=331619&r2=331620&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
+++ cfe/trunk/test/SemaCXX/type-traits.cpp Sun May  6 23:43:30 2018
@@ -262,6 +262,7 @@ void is_pod()
 typedef Empty EmptyAr[10];
 struct Bit0 { int : 0; };
 struct Bit0Cons { int : 0; Bit0Cons(); };
+struct AnonBitOnly { int : 3; };
 struct BitOnly { int x : 3; };
 struct DerivesVirt : virtual POD {};
 
@@ -287,6 +288,7 @@ void is_empty()
   { int arr[F(__is_empty(EmptyAr))]; }
   { int arr[F(__is_empty(HasRef))]; }
   { int arr[F(__is_empty(HasVirt))]; }
+  { int arr[F(__is_empty(AnonBitOnly))]; }
   { int arr[F(__is_empty(BitOnly))]; }
   { int arr[F(__is_empty(void))]; }
   { int arr[F(__is_empty(IntArNB))]; }




More information about the cfe-commits mailing list