Page MenuHomePhabricator

[Sema][ObjC] Disallow non-trivial C struct fields in unions

Authored by ahatanak on Dec 13 2018, 9:32 AM.



This patch fixes a bug where clang doesn’t reject union fields of non-trivial struct types:

struct S0 {
  id x;

struct S1 {
  id y;

union U0 {
  struct S0 s0;  // no diagnostics.
  struct S1 s1;  // no diagnostics.

union U1 {
  id x;  // clang rejects ObjC pointer fields in unions.

void test(union U0 a) {
  // Both ‘S0::x’ and ‘S1::y' are destructed in the IR.


Diff Detail


Event Timeline

ahatanak created this revision.Dec 13 2018, 9:32 AM

Just to clarify, this patch doesn't change clang's handling of C++ unions that have non-static data members with non-trivial special member functions.

ahatanak planned changes to this revision.Feb 5 2019, 4:08 PM

This patch is unnecessarily complicated and is not correct.

ahatanak updated this revision to Diff 185466.Feb 5 2019, 6:49 PM

Make sure that volatile trivial fields in a struct don't cause Sema to emit a diagnostic. Simplify the code in Sema::ActOnFields .

rjmccall accepted this revision.Feb 7 2019, 10:35 AM

Two minor tweaks, but otherwise LGTM.

2273 ↗(On Diff #185466)

Might be good to spell this out: we don't want to apply the C restriction in C++ because C++ (1) can apply the restriction at a finer grain by banning copying/destroying the union and (2) allows users to override these restrictions by declaring explicit constructors/etc., which we're not proposing to add to C.

24 ↗(On Diff #185466)


This revision is now accepted and ready to land.Feb 7 2019, 10:35 AM
This revision was automatically updated to reflect the committed changes.
ahatanak marked 2 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 12:21 PM