[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d4d21e2e095: [Clang][AST] Fix
__has_unique_object_representations computation for unnamed… (authored by
royjacobson).
Changed prior to commit:
https://reviews.llvm.org/D145852?vs=509298=510273#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
Files:
clang/docs/ReleaseNotes.rst
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/type-traits.cpp
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2828,6 +2828,17 @@
static_assert(!has_unique_object_representations::value,
"Alignment causes padding");
static_assert(!has_unique_object_representations::value,
"Also no arrays that have padding");
+struct __attribute__((packed)) PackedNoPadding1 {
+ short i;
+ int j;
+};
+struct __attribute__((packed)) PackedNoPadding2 {
+ int j;
+ short i;
+};
+static_assert(has_unique_object_representations::value,
"Packed structs have no padding");
+static_assert(has_unique_object_representations::value,
"Packed structs have no padding");
+
static_assert(!has_unique_object_representations::value, "Functions
are not unique");
static_assert(!has_unique_object_representations::value,
"Functions are not unique");
static_assert(!has_unique_object_representations::value,
"Functions are not unique");
@@ -2878,9 +2889,35 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct __attribute__((packed)) UnnamedBitfieldPacked {
+ int named : 8;
+ int : 24;
+};
+
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
+
+struct UnnamedEmptyBitfieldSplit {
+ short named;
+ int : 0;
+ short also_named;
+};
+
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
static_assert(has_unique_object_representations::value,
"Bitfield padding");
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(has_unique_object_representations::value,
"Bitfield padding");
+static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2), "Wrong
size");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2716,6 +2716,11 @@
int64_t FieldSizeInBits =
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
+// If we have explicit padding bits, they don't contribute bits
+// to the actual object representation, so return 0.
+if (Field->isUnnamedBitfield())
+ return 0;
+
int64_t BitfieldSize = Field->getBitWidthValue(Context);
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -291,6 +291,9 @@
template parameters with different nested constraints.
- Fix type equivalence comparison between auto types to take constraints into
account.
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2828,6 +2828,17 @@
static_assert(!has_unique_object_representations::value, "Alignment causes padding");
static_assert(!has_unique_object_representations::value, "Also no arrays that have padding");
+struct __attribute__((packed)) PackedNoPadding1 {
+ short i;
+ int j;
+};
+struct __attribute__((packed)) PackedNoPadding2 {
+ int j;
+ short i;
+};
+static_assert(has_unique_object_representations::value, "Packed structs have no padding");
+static_assert(has_unique_object_representations::value, "Packed structs have no padding");
+
static_assert(!has_unique_object_representations::value, "Functions are not unique");
static_assert(!has_unique_object_representations::value, "Functions are not unique");
static_assert(!has_unique_object_representations::value, "Functions are not unique");
@@ -2878,9 +2889,35 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct __attribute__((packed))
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
shafik accepted this revision. shafik added a comment. LGTM, thank for adding the tests! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145852/new/ https://reviews.llvm.org/D145852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson updated this revision to Diff 509298.
royjacobson added a comment.
Add some ((packed)) tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
Files:
clang/docs/ReleaseNotes.rst
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/type-traits.cpp
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2828,6 +2828,17 @@
static_assert(!has_unique_object_representations::value,
"Alignment causes padding");
static_assert(!has_unique_object_representations::value,
"Also no arrays that have padding");
+struct __attribute__((packed)) PackedNoPadding1 {
+ short i;
+ int j;
+};
+struct __attribute__((packed)) PackedNoPadding2 {
+ int j;
+ short i;
+};
+static_assert(has_unique_object_representations::value,
"Packed structs have no padding");
+static_assert(has_unique_object_representations::value,
"Packed structs have no padding");
+
static_assert(!has_unique_object_representations::value, "Functions
are not unique");
static_assert(!has_unique_object_representations::value,
"Functions are not unique");
static_assert(!has_unique_object_representations::value,
"Functions are not unique");
@@ -2878,9 +2889,35 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct __attribute__((packed)) UnnamedBitfieldPacked {
+ int named : 8;
+ int : 24;
+};
+
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
+
+struct UnnamedEmptyBitfieldSplit {
+ short named;
+ int : 0;
+ short also_named;
+};
+
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
static_assert(has_unique_object_representations::value,
"Bitfield padding");
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(has_unique_object_representations::value,
"Bitfield padding");
+static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2), "Wrong
size");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2810,6 +2810,11 @@
int64_t FieldSizeInBits =
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
+// If we have explicit padding bits, they don't contribute bits
+// to the actual object representation, so return 0.
+if (Field->isUnnamedBitfield())
+ return 0;
+
int64_t BitfieldSize = Field->getBitWidthValue(Context);
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2828,6 +2828,17 @@
static_assert(!has_unique_object_representations::value, "Alignment causes padding");
static_assert(!has_unique_object_representations::value, "Also no arrays that have padding");
+struct __attribute__((packed)) PackedNoPadding1 {
+ short i;
+ int j;
+};
+struct __attribute__((packed)) PackedNoPadding2 {
+ int j;
+ short i;
+};
+static_assert(has_unique_object_representations::value, "Packed structs have no padding");
+static_assert(has_unique_object_representations::value, "Packed structs have no padding");
+
static_assert(!has_unique_object_representations::value, "Functions are not unique");
static_assert(!has_unique_object_representations::value, "Functions are not unique");
static_assert(!has_unique_object_representations::value, "Functions are not unique");
@@ -2878,9 +2889,35 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct __attribute__((packed)) UnnamedBitfieldPacked {
+ int named : 8;
+ int : 24;
+};
+
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
+
+struct UnnamedEmptyBitfieldSplit {
+ short named;
+ int : 0;
+ short also_named;
+};
+
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
shafik added inline comments.
Comment at: clang/test/SemaCXX/type-traits.cpp:2886-2889
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
royjacobson wrote:
> royjacobson wrote:
> > shafik wrote:
> > > aaron.ballman wrote:
> > > > I think there's one more test to add:
> > > > ```
> > > > struct UnnamedEmptyBitfieldSplit {
> > > > short named;
> > > > int : 0;
> > > > short also_named;
> > > > };
> > > > static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2));
> > > > static_assert(!has_unique_object_representations::value,
> > > > "Bitfield padding");
> > > > ```
> > > Do we also want to check packed structs as well?
> > Do you have a test case where it would matter? Apparently `packed` is
> > specified to ignore zero width bit fields.
> >
> small ping :) @shafik
Apologies, I just realized you thought I meant another zero sized bit-field
case whereas I meant just test packed in general w/ non-zero sized bit-fields.
The result should match for unpacked but it would be good to verify considering
we seem to always get bitten by cases we don't cover.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson added inline comments.
Comment at: clang/test/SemaCXX/type-traits.cpp:2886-2889
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
royjacobson wrote:
> shafik wrote:
> > aaron.ballman wrote:
> > > I think there's one more test to add:
> > > ```
> > > struct UnnamedEmptyBitfieldSplit {
> > > short named;
> > > int : 0;
> > > short also_named;
> > > };
> > > static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2));
> > > static_assert(!has_unique_object_representations::value,
> > > "Bitfield padding");
> > > ```
> > Do we also want to check packed structs as well?
> Do you have a test case where it would matter? Apparently `packed` is
> specified to ignore zero width bit fields.
>
small ping :) @shafik
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson marked an inline comment as done.
royjacobson added inline comments.
Comment at: clang/test/SemaCXX/type-traits.cpp:2886-2889
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
shafik wrote:
> aaron.ballman wrote:
> > I think there's one more test to add:
> > ```
> > struct UnnamedEmptyBitfieldSplit {
> > short named;
> > int : 0;
> > short also_named;
> > };
> > static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2));
> > static_assert(!has_unique_object_representations::value,
> > "Bitfield padding");
> > ```
> Do we also want to check packed structs as well?
Do you have a test case where it would matter? Apparently `packed` is specified
to ignore zero width bit fields.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
shafik added inline comments.
Comment at: clang/test/SemaCXX/type-traits.cpp:2886-2889
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
aaron.ballman wrote:
> I think there's one more test to add:
> ```
> struct UnnamedEmptyBitfieldSplit {
> short named;
> int : 0;
> short also_named;
> };
> static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2));
> static_assert(!has_unique_object_representations::value,
> "Bitfield padding");
> ```
Do we also want to check packed structs as well?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson marked 2 inline comments as done. royjacobson added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2817 +// unique representation. +if (Field->isUnnamedBitfield() && BitfieldSize > 0) + return std::nullopt; aaron.ballman wrote: > I think the check for a non-zero bit-field width might not be completely > correct in cases where it's used to split an allocation unit. I suggested a > test case below. That was actually fine: the padding bits would not be a part of the unnamed bit field so they're detected. Then I realized that the surrounding code just sums the object representation bits and checks if that equals `sizeof(T)`. But unnamed fields don't contribute any bits to the object representation. So a simpler way to fix this is to return 0 unconditionally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145852/new/ https://reviews.llvm.org/D145852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson updated this revision to Diff 504736.
royjacobson added a comment.
Add a test case, slightly simpler modeling.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
Files:
clang/docs/ReleaseNotes.rst
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/type-traits.cpp
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,29 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
+
+struct UnnamedEmptyBitfieldSplit {
+ short named;
+ int : 0;
+ short also_named;
+};
+
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
static_assert(has_unique_object_representations::value,
"Bitfield padding");
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(has_unique_object_representations::value,
"Bitfield padding");
+static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2), "Wrong
size");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2810,6 +2810,11 @@
int64_t FieldSizeInBits =
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
+// If we have explicit padding bits, they don't contribute bits
+// to the actual object representation, so return 0.
+if (Field->isUnnamedBitfield())
+ return 0;
+
int64_t BitfieldSize = Field->getBitWidthValue(Context);
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,29 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
+
+struct UnnamedEmptyBitfieldSplit {
+ short named;
+ int : 0;
+ short also_named;
+};
+
static_assert(!has_unique_object_representations::value, "Bitfield padding");
static_assert(has_unique_object_representations::value, "Bitfield padding");
static_assert(!has_unique_object_representations::value, "Bitfield padding");
+static_assert(!has_unique_object_representations::value, "Bitfield padding");
+static_assert(has_unique_object_representations::value, "Bitfield padding");
+static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2), "Wrong size");
+static_assert(!has_unique_object_representations::value, "Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2810,6 +2810,11 @@
int64_t FieldSizeInBits =
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
+// If we have explicit padding bits, they don't contribute bits
+// to the actual object representation, so return 0.
+if (Field->isUnnamedBitfield())
+ return 0;
+
int64_t BitfieldSize = Field->getBitWidthValue(Context);
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
___
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
aaron.ballman added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:2817
+// unique representation.
+if (Field->isUnnamedBitfield() && BitfieldSize > 0)
+ return std::nullopt;
I think the check for a non-zero bit-field width might not be completely
correct in cases where it's used to split an allocation unit. I suggested a
test case below.
Comment at: clang/test/SemaCXX/type-traits.cpp:2886-2889
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
I think there's one more test to add:
```
struct UnnamedEmptyBitfieldSplit {
short named;
int : 0;
short also_named;
};
static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2));
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson updated this revision to Diff 504390.
royjacobson added a comment.
Handle 0-length unnamed bit fields as well.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
Files:
clang/docs/ReleaseNotes.rst
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/type-traits.cpp
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,21 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
+
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
static_assert(has_unique_object_representations::value,
"Bitfield padding");
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(has_unique_object_representations::value,
"Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2811,6 +2811,12 @@
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
int64_t BitfieldSize = Field->getBitWidthValue(Context);
+
+// If we have explicit padding bits, return nullopt to indicate no
+// unique representation.
+if (Field->isUnnamedBitfield() && BitfieldSize > 0)
+ return std::nullopt;
+
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
cast(Field->getType())->getNumBits())
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,21 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
+struct UnnamedEmptyBitfield {
+ int named;
+ int : 0;
+};
+
static_assert(!has_unique_object_representations::value, "Bitfield padding");
static_assert(has_unique_object_representations::value, "Bitfield padding");
static_assert(!has_unique_object_representations::value, "Bitfield padding");
+static_assert(!has_unique_object_representations::value, "Bitfield padding");
+static_assert(has_unique_object_representations::value, "Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2811,6 +2811,12 @@
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
int64_t BitfieldSize = Field->getBitWidthValue(Context);
+
+// If we have explicit padding bits, return nullopt to indicate no
+// unique representation.
+if (Field->isUnnamedBitfield() && BitfieldSize > 0)
+ return std::nullopt;
+
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
cast(Field->getType())->getNumBits())
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
Endill accepted this revision. Endill added a comment. This revision is now accepted and ready to land. LGTM I got confused at first why this fix is done in a function named `getSubobjectSizeInBits`, and why do we calculate size to produce a boolean value in the end. Then I realized that this function returns number of bits of value representation, or `std::nullopt` if there are any padding bits, and we take advantage of the latter to implement `hasUniqueObjectRepresentations`. I guess I'm either missing some context or that function could be named better (not in this patch, of course). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145852/new/ https://reviews.llvm.org/D145852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson updated this revision to Diff 504376.
royjacobson added a comment.
Change to a simpler fix.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145852/new/
https://reviews.llvm.org/D145852
Files:
clang/docs/ReleaseNotes.rst
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/type-traits.cpp
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,15 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
static_assert(has_unique_object_representations::value,
"Bitfield padding");
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2810,6 +2810,9 @@
int64_t FieldSizeInBits =
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
+if (Field->isUnnamedBitfield())
+ return std::nullopt;
+
int64_t BitfieldSize = Field->getBitWidthValue(Context);
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,15 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
static_assert(!has_unique_object_representations::value, "Bitfield padding");
static_assert(has_unique_object_representations::value, "Bitfield padding");
static_assert(!has_unique_object_representations::value, "Bitfield padding");
+static_assert(!has_unique_object_representations::value, "Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2810,6 +2810,9 @@
int64_t FieldSizeInBits =
Context.toBits(Context.getTypeSizeInChars(Field->getType()));
if (Field->isBitField()) {
+if (Field->isUnnamedBitfield())
+ return std::nullopt;
+
int64_t BitfieldSize = Field->getBitWidthValue(Context);
if (IsBitIntType) {
if ((unsigned)BitfieldSize >
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.
royjacobson created this revision.
Herald added a project: All.
royjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
As pointed out in https://github.com/llvm/llvm-project/issues/61336, objects
with
unnamed bitfields aren't be uniquely representable.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145852
Files:
clang/docs/ReleaseNotes.rst
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/type-traits.cpp
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,15 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
static_assert(has_unique_object_representations::value,
"Bitfield padding");
static_assert(!has_unique_object_representations::value,
"Bitfield padding");
+static_assert(!has_unique_object_representations::value,
"Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2831,13 +2831,17 @@
return structHasUniqueObjectRepresentations(Context, RD);
}
-template
+template
static std::optional structSubobjectsHaveUniqueObjectRepresentations(
const RangeT , int64_t CurOffsetInBits,
const ASTContext , const clang::ASTRecordLayout ) {
for (const auto *Subobject : Subobjects) {
std::optional SizeInBits =
getSubobjectSizeInBits(Subobject, Context);
+if constexpr (CheckUnnamedBitFields) {
+ if (Subobject->isUnnamedBitfield())
+return std::nullopt;
+}
if (!SizeInBits)
return std::nullopt;
if (*SizeInBits != 0) {
@@ -2873,15 +2877,15 @@
});
std::optional OffsetAfterBases =
-structSubobjectsHaveUniqueObjectRepresentations(Bases, CurOffsetInBits,
-Context, Layout);
+structSubobjectsHaveUniqueObjectRepresentations(
+Bases, CurOffsetInBits, Context, Layout);
if (!OffsetAfterBases)
return std::nullopt;
CurOffsetInBits = *OffsetAfterBases;
}
std::optional OffsetAfterFields =
- structSubobjectsHaveUniqueObjectRepresentations(
+ structSubobjectsHaveUniqueObjectRepresentations(
RD->fields(), CurOffsetInBits, Context, Layout);
if (!OffsetAfterFields)
return std::nullopt;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 `_)
+- Fix bug in the computation of the ``__has_unique_object_representations``
+ builtin for types with unnamed bitfields.
+ (`#61336 `_)
Bug Fixes to AST Handling
^
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2878,9 +2878,15 @@
char d : 2;
};
+struct UnnamedBitfield {
+ int named : 8;
+ int : 24;
+};
+
static_assert(!has_unique_object_representations::value, "Bitfield padding");
static_assert(has_unique_object_representations::value, "Bitfield padding");
static_assert(!has_unique_object_representations::value, "Bitfield padding");
+static_assert(!has_unique_object_representations::value, "Bitfield padding");
struct BoolBitfield {
bool b : 8;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2831,13 +2831,17 @@
return structHasUniqueObjectRepresentations(Context, RD);
}
-template
+template
static std::optional structSubobjectsHaveUniqueObjectRepresentations(
const RangeT , int64_t CurOffsetInBits,
const ASTContext , const clang::ASTRecordLayout ) {
for (const auto *Subobject : Subobjects) {
std::optional SizeInBits =
getSubobjectSizeInBits(Subobject, Context);
+if constexpr (CheckUnnamedBitFields) {
+ if (Subobject->isUnnamedBitfield())
+return std::nullopt;
+}
if (!SizeInBits)
return std::nullopt;
if (*SizeInBits != 0) {
@@ -2873,15 +2877,15 @@
});
std::optional OffsetAfterBases =
-structSubobjectsHaveUniqueObjectRepresentations(Bases, CurOffsetInBits,
-Context, Layout);
+structSubobjectsHaveUniqueObjectRepresentations(
+
