Skip to content

fix(folding): correct unique object rep checks [] - #326

Open
jakub-labenski wants to merge 1 commit into
edgcpp:mainfrom
jakub-labenski:has_unique_object_representations
Open

jakub-labenski wants to merge 1 commit into
edgcpp:mainfrom
jakub-labenski:has_unique_object_representations

Conversation

@jakub-labenski

Copy link
Copy Markdown

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.

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.

@nv-cmeerw nv-cmeerw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats issue #327 now.

Comment thread src/Changes
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++:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is not actually gcc specific, but also applies to default/standard mode, so should just have an example with --c++11.

Comment thread src/Changes
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should the behavior for standard-mode be in this case?

//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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/folding.c
field->source_corresp.assoc_info ==
(char *)get_unnamed_field_symbol()) {
/* An unnamed bit-field of non-zero width is padding. */
result = FALSE;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that need to be restricted to non-union class types?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants