Repository navigation
fix(folding): correct unique object rep checks [GH #327] - #326
jakub-labenski wants to merge 1 commit into
Conversation
| A base class that declares no nonstatic data members of its own but inherits | ||
| some (and is therefore not empty) was treated as an empty base class. Its | ||
| size was then counted as tail padding, so the trait was incorrectly FALSE for | ||
| classes derived from it. For example, with --g++: |
There was a problem hiding this comment.
The issue is not actually gcc specific, but also applies to default/standard mode, so should just have an example with --c++11.
There was a problem hiding this comment.
Entry has been updated: --g++ was change to --c++11 and an empty message was added to static_assert.
| This is now fixed. In addition, the following version-dependent behaviors | ||
| of g++ and Clang are now emulated: | ||
|
|
||
| - An unnamed bit-field of nonzero width is treated as padding (making the |
There was a problem hiding this comment.
What should the behavior for standard-mode be in this case?
There was a problem hiding this comment.
In standard mode, an unnamed bit-field of nonzero width is treated as padding, so __has_unique_object_representations(S) evaluates to false.
This was fixed in the latest update, along with new tests covering standard mode as well as GCC, Clang, and Microsoft emulation modes.
| //remark:has_unique_object_representations with a base that has no direct fields | ||
| //type:fp | ||
| //options_all:--c++17 | ||
| //options:--g++ --gnu_version=150100:--clang --clang_version=190000 |
There was a problem hiding this comment.
The test case should also test standard mode (and ideally, also MSVC modes).
Also need to have test coverage for the other code changes (unnamed bit-field, volatile qualifiers). Should also test each version condition from the source code.
There was a problem hiding this comment.
I've updated the tests to include standard mode and Microsoft emulation, and added the missing tests for the remaining changes.
| field->source_corresp.assoc_info == | ||
| (char *)get_unnamed_field_symbol()) { | ||
| /* An unnamed bit-field of non-zero width is padding. */ | ||
| result = FALSE; |
There was a problem hiding this comment.
Does that need to be restricted to non-union class types?
There was a problem hiding this comment.
Yes, it should. In a union, an unnamed bit-field overlaps the other members, so it shouldn't be treated as padding on its own. Instead, any bit-field narrower than the union is treated as padding, except in Clang and Microsoft modes (which ignore that case). This is now implemented and tests have been added.
Fix __has_unique_object_representations for several cases: - A base class with no direct nonstatic data members that inherits some (and is thus not empty) was treated as an empty base. Its size was counted as tail padding, making the trait incorrectly FALSE for derived classes. Use is_empty_class_type instead of checking for an empty field list. - Treat unnamed bit-fields of nonzero width in classes as padding, as the standard requires. Keep the old behavior only in Clang modes before 17 and g++ modes before 13.1 (except the 10.5, 11.4, and 12.3 backports). - Do not apply the unnamed bit-field rule to unions, where the bit-field overlaps the other members; the existing union size check decides. - In Microsoft mode, as in Clang mode, do not treat a union bit-field narrower than the union as padding, matching MSVC. - Restrict treating volatile-qualified non-class types as lacking unique object representations to g++ modes before version 10, and no longer apply it in Clang modes. Add regression tests has-unique-object-rep-inherited-base, has-unique-object-rep-bit-field, has-unique-object-rep-union-bit-field, and has-unique-object-rep-volatile, covering standard, g++, Clang, and Microsoft modes, and update expectations for imported Clang type-traits and g++ has-unique-obj-representations3 tests.
c05bcb2 to
625b0c5
Compare
|
I'm not sure what the recommended practice is for syncing with main. I rebased the branch (and the pull request) onto main, then amended my commit to keep the git history clean. Below is an AI-generated summary of the changes in my commit only:
|
jakub-labenski
changed the title
This branch has not been deployed
2 participants
Discussion: #290
Fix __has_unique_object_representations for several cases:
A base class with no direct nonstatic data members that inherits some (and is thus not empty) was treated as an empty base. Its size was counted as tail padding, making the trait incorrectly FALSE for derived classes. Use is_empty_class_type instead of checking for an empty field list.
Treat unnamed bit-fields of nonzero width as padding in Clang 17+ and g++ 13.1+ (also g++ 10.5, 11.4, 12.3 backports).
Restrict treating volatile-qualified non-class types as lacking unique object representations to g++ modes before version 10, and no longer apply it in Clang modes.
Add regression test has-unique-object-rep-inherited-base and update expectations for imported Clang type-traits and g++ has-unique-obj-representations3 tests.