3 ms·

It's true that this implementation is wrong, but I disagree that it's a bad idea. Certainly it's possible to do correctly in various ways, e.g. by having the m

by comex 9y ago

It's true that this implementation is wrong, but I disagree that it's a bad idea. Certainly it's possible to do correctly in various ways, e.g. by having the macro pass offsetof(Some, the_union) (but the existing code doesn't name the union and there would have to be consideration for multiple variant types).

klodolph 9y ago
"Certainly it's possible to do correctly in various ways" Not very convincing. At the very least, you'd need to pass in the type name as a macro parameter, use non-portable GCC extensions, or define some additional boilerplate for each union type. We can't just say "offsetof" and handwave the actual macro definition here. The traditional solution, to define an enum type, works reasonably. You can certainly get silent type errors from typos but perhaps we can tolerate the risk. It's not worse than, say, qsort in terms of type safety. The options I alluded to above are due to the fact that in order to allow the compiler to type-check for us, we need to keep the type of "x" passed in to "as()". You need to check the tag and either abort or return the correct union member. So "x" appears twice, but that's a bad idea for a macro. So you either use the GCC extension which allows statements as expressions, or you dispatch to a boilerplate function for each union type. I can't think of a different way to do things.
- comex 9y ago
  > Not very convincing. Sorry, I was in a hurry when I wrote that comment :) The easiest way is to take advantage of the fact that a pointer to a union can be casted to a pointer to one of its members (C11 6.7.2.1.16), or in other words that all the possible variants are at the same offset. So we can do: typedef struct Some variantof_SomeOne; #define as(x, T) ((struct T *)variant_cast(x, tag(T), \ offsetof(variantof_##T, u))) where the definition of Some would have to be changed to give the union field a name, and variant_cast would add the offset given as an argument. An alternative would be one of the options you mentioned, a boilerplate function. But it doesn't have to be 'real' boilerplate: it's possible to use macros to automate the definition of such functions. Given suitable macros, you can write something like: #define FOO_FIELDS(f) \ f(one, struct bar) \ f(two, float) \ f(three, int) DEFINE_VARIANT(foo, FOO_FIELDS) and get as much boilerplate as you want, like: enum foo_tag { foo_tag_one, foo_tag_two, foo_tag_three }; struct foo { enum foo_tag tag; union { struct bar one; float two; int three; }; }; static inline bool foo_is_one(struct foo *fo) { return foo->tag == foo_tag_one; } static inline struct bar * foo_get_one(struct foo *foo) { assert(foo->tag == foo_tag_one); return &foo->one; } [etc. for other variants]

by comex 9y ago

klodolph 9y ago

"Certainly it's possible to do correctly in various ways" Not very convincing. At the very least, you'd need to pass in the type name as a macro parameter, use non-portable GCC extensions, or define some additional boilerplate for each union type. We can't just say "offsetof" and handwave the actual macro definition here. The traditional solution, to define an enum type, works reasonably. You can certainly get silent type errors from typos but perhaps we can tolerate the risk. It's not worse than, say, qsort in terms of type safety. The options I alluded to above are due to the fact that in order to allow the compiler to type-check for us, we need to keep the type of "x" passed in to "as()". You need to check the tag and either abort or return the correct union member. So "x" appears twice, but that's a bad idea for a macro. So you either use the GCC extension which allows statements as expressions, or you dispatch to a boilerplate function for each union type. I can't think of a different way to do things.

comex 9y ago
> Not very convincing. Sorry, I was in a hurry when I wrote that comment :) The easiest way is to take advantage of the fact that a pointer to a union can be casted to a pointer to one of its members (C11 6.7.2.1.16), or in other words that all the possible variants are at the same offset. So we can do: typedef struct Some variantof_SomeOne; #define as(x, T) ((struct T *)variant_cast(x, tag(T), \ offsetof(variantof_##T, u))) where the definition of Some would have to be changed to give the union field a name, and variant_cast would add the offset given as an argument. An alternative would be one of the options you mentioned, a boilerplate function. But it doesn't have to be 'real' boilerplate: it's possible to use macros to automate the definition of such functions. Given suitable macros, you can write something like: #define FOO_FIELDS(f) \ f(one, struct bar) \ f(two, float) \ f(three, int) DEFINE_VARIANT(foo, FOO_FIELDS) and get as much boilerplate as you want, like: enum foo_tag { foo_tag_one, foo_tag_two, foo_tag_three }; struct foo { enum foo_tag tag; union { struct bar one; float two; int three; }; }; static inline bool foo_is_one(struct foo *fo) { return foo->tag == foo_tag_one; } static inline struct bar * foo_get_one(struct foo *foo) { assert(foo->tag == foo_tag_one); return &foo->one; } [etc. for other variants]