rsmith added a comment.
In D62988#1540359 <https://reviews.llvm.org/D62988#1540359>, @rjmccall wrote:
> For ARC, you could bzero the union member; this is what how we tell people to
> initialize malloc'ed memory as well, since there's no default-constructor
> concept that they can invoke for such cases.
> Our immediate need for this attribute is that we have some code that wants
> to adopt non-trivial annotations in unions, with no intention of ever copying
> or destroying them as aggregates.
OK, so we'd be looking at something like this when using the attribute:
typedef struct S {
int is_b;
union {
float f;
__attribute__((non_trivial_union_member)) id b;
};
} S;
S make() {
S s = {0, 0.0f};
return s;
}
void set_f(S *s, float f) {
if (s->is_b) s->b = 0;
s->is_b = 0;
s->f = f;
}
void set_b(S *s, id b) {
if (!s->is_b) bzero(s, sizeof(S));
s->is_b = 1;
s->b = b;
}
void destroy_s(S *s) {
if (s->is_b) s->b = 0;
}
versus the same code written without the attribute, which might be:
typedef struct S {
int is_b;
union {
float f;
void *b;
};
} S;
S make() {
S s = {0, 0.0f};
return s;
}
void set_f(S *s, float f) {
if (s->is_b) (__bridge_transfer id)s->b;
s->is_b = 0;
s->f = f;
}
void set_b(S *s, id b) {
if (s->is_b) (__bridge_transfer id)s->b;
s->is_b = 1;
s->b = (__bridge_retained void*)b;
}
void destroy_s(S *s) {
if (s->is_b) (__bridge_transfer id)s->b;
}
To me, using the attribute here doesn't seem worthwhile; the first form of this
code seems scary since important reference counting actions are hidden behind
what appear to be dead stores, and the second form (while verbose) is at least
explicit about what's going on. If you still think this is a useful feature, so
be it, but we should at least be explicit in Clang's ARC documentation about
how to correctly make use of it.
I'm not sure whether we should support this as a way of disabling the union
triviality features in general, or only the restriction on lifetime types, but
either way, I think the support of this attribute should not be dependent on
the language mode; I think adding a C-only extension for this does a disservice
to our users.
> Of course a better solution would be to make any C code that would copy or
> destroy the aggregate type illegal, but that seems like a big project. But
> maybe there'd be no need for this attribute in the long term if we eventually
> do have that support.
Yeah, maybe so; adopting the C++11 "unrestricted unions" behavior for
non-trivial types in unions in C does seem to make a certain amount of sense.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62988/new/
https://reviews.llvm.org/D62988
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
