On Tue, Aug 17, 2021 at 07:10:28AM -0700, Jason Merrill wrote: > Looks good, thanks. I think you didn't see that I also asked for some added > comments; OK with those added.
Oops, I've indeed missed them, sorry. On Mon, Aug 16, 2021 at 03:57:21PM -0400, Jason Merrill wrote: > Add a comment that discussion in core suggests that we might move toward > treating multiple union fields of the same type as the same field, so this > constraint might get dropped in the future. Just same type fields, or even any fields with layout compatible types? Anyway, either of that would require further changes in the code. So that I don't repost the whole large patch, here is just incremental diff with the added comments: --- gcc/cp/semantics.c 2021-08-17 11:36:44.024227609 +0200 +++ gcc/cp/semantics.c 2021-08-17 16:41:57.070923754 +0200 @@ -10923,6 +10923,16 @@ basetype2, membertype2, arg2); if (TREE_TYPE (ret) == boolean_type_node) return ret; + /* If both arg1 and arg2 are INTEGER_CSTs, is_corresponding_member_aggr + already returns boolean_{true,false}_node whether those particular + members are corresponding members or not. Otherwise, if only + one of them is INTEGER_CST (canonicalized to first being INTEGER_CST + above), it returns boolean_false_node if it is certainly not a + corresponding member and otherwise we need to do a runtime check that + those two OFFSET_TYPE offsets are equal. + If neither of the operands is INTEGER_CST, is_corresponding_member_aggr + returns the largest offset at which the members would be corresponding + members, so perform arg1 <= ret && arg1 == arg2 runtime check. */ gcc_assert (TREE_CODE (arg2) != INTEGER_CST); if (TREE_CODE (arg1) == INTEGER_CST) return fold_build2 (EQ_EXPR, boolean_type_node, arg1, --- gcc/cp/typeck.c 2021-08-17 11:18:53.271850970 +0200 +++ gcc/cp/typeck.c 2021-08-17 16:48:56.165115017 +0200 @@ -1727,6 +1727,15 @@ field2 = DECL_CHAIN (field2); } } + /* Otherwise both types must be union types. + The standard says: + "Two standard-layout unions are layout-compatible if they have + the same number of non-static data members and corresponding + non-static data members (in any order) have layout-compatible + types." + but the code anticipates that bitfield vs. non-bitfield, + different bitfield widths or presence/absence of + [[no_unique_address]] should be checked as well. */ auto_vec<tree, 16> vec; unsigned int count = 0; for (; field1; field1 = DECL_CHAIN (field1)) @@ -1735,6 +1744,9 @@ for (; field2; field2 = DECL_CHAIN (field2)) if (TREE_CODE (field2) == FIELD_DECL) vec.safe_push (field2); + /* Discussions on core lean towards treating multiple union fields + of the same type as the same field, so this might need changing + in the future. */ if (count != vec.length ()) return false; for (field1 = TYPE_FIELDS (type1); field1; field1 = DECL_CHAIN (field1)) Jakub