On Tue, 7 Nov 2017, Jason Merrill wrote: > On Tue, Nov 7, 2017 at 3:14 AM, Richard Biener <rguent...@suse.de> wrote: > > On Mon, 6 Nov 2017, Jason Merrill wrote: > > > >> On Mon, Nov 6, 2017 at 10:37 AM, Marek Polacek <pola...@redhat.com> wrote: > >> > On Fri, Nov 03, 2017 at 12:19:05PM -0400, Jason Merrill wrote: > >> >> On Fri, Nov 3, 2017 at 9:55 AM, Marek Polacek <pola...@redhat.com> > >> >> wrote: > >> >> > + TYPE_EMPTY_P (t) = targetm.calls.empty_record_p (t); > >> >> > >> >> I think we want to set this in finalize_type_size; since the point of > >> >> all this is becoming compliant with the psABI (and compatible with the > >> >> C front end), I wouldn't think it should be specific to the C++ front > >> >> end. > >> > > >> > Sure, that works. I'd wanted to do the TYPE_EMPTY_P setting in > >> > layout_type, > >> > but that wasn't called for the classes in my testcases. I've moved the > >> > setting of TYPE_EMPTY_P to finalize_type_size now. > >> > > >> >> > + TYPE_WARN_EMPTY_P (t) = warn_abi && abi_version_crosses (12); > >> >> > >> >> Can this flag go on the TRANSLATION_UNIT_DECL rather than the type? > >> > > >> > Yeah, that works, too. To avoid needing a lang hook, I'm setting the > >> > flag in cxx_init_decl_processing, hope that's ok. > >> > >> Sure. > >> > >> >> > + if (TREE_CODE (field) == FIELD_DECL > >> >> > + && (DECL_NAME (field) > >> >> > + || RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))) > >> >> > + && !default_is_empty_type (TREE_TYPE (field))) > >> >> > + return false; > >> >> > + return true; > >> >> > >> >> Hmm, this assumes that any unnamed field can be ignored; I'm concerned > >> >> that some front end might clear DECL_NAME for a reason that doesn't > >> >> imply that the field is just padding. > >> > > >> > In that case I guess we need a new lang hook, right? Because in > >> > default_is_empty_type we can't check FE-specific flags such as > >> > DECL_C_BIT_FIELD. For C++, should that lang hook be just > >> > is_really_empty_class? Or is there anything else I can do? > >> > >> Hmm, maybe leave it as it is and just document this assumption about > >> FIELD_DECL with null DECL_NAME, both here and in tree.def. > > > > But are you sure you are not changing the ABI for a language other > > than C++ then? > > No, that is the concern, we'd need to check that. > > > I don't think DECL_NAME has any special meaning - why > > not check DECL_ARTIFICIAL or another flag that has appropriate > > semantic - _what_ semantic are you looking for after all? > > That a struct consisting only of padding is considered empty. > > Perhaps we could use decl_flag_3 on FIELD_DECL for DECL_PADDING_P to > indicate that a field isn't really data. Or perhaps we should remove > such things from TYPE_FIELDS after layout is done.
Both works for me. As said I'd rather not use NULL DECL_NAME - backends might build record types without names for va_list. There's no technical reason to have a DECL_NAME for sth the user cannot access - which means every DECL_ARTIFICIAL entity. This patch would retroactively introduce one and thus we'd have to audit each and every piece of code building record types... Richard.
