On Sat, Oct 30, 2021 at 2:29 PM Richard Sandiford <richard.sandif...@arm.com>
wrote:
> Jason Merrill <ja...@redhat.com> writes:
> > On 10/18/21 16:35, Richard Sandiford wrote:
> >> Jason Merrill <ja...@redhat.com> writes:
> >>> On 9/24/21 13:53, Richard Sandiford wrote:
> >>>> + if (type == error_mark_node)
> >>>> + return lhd_simulate_record_decl (loc, name, fields);
> >>>
> >>> Why fall back to the language-independent function on error? Is there
> a
> >>> case where that gives better error recovery than just returning
> >>> error_mark_node?
> >>
> >> I don't think falling back necessarily improves future error messages
> >> (or makes them worse). The reason was more that the code to handle
> >> target builtins generally expects to be able to create whatever types
> >> and functions it wants. If we return something unexpected, even it's
> >> error_mark_node, then there's a higher risk of ICEs later on.
> >>
> >> I guess that's a bit defeatist. But in practice, the first code
> >> that uses the hook will be code that previously ran at start-up
> >> and so didn't have to worry about these errors.
> >>
> >> In practice I think errors will be extremely rare.
> >>
> >>>> + xref_basetypes (type, NULL_TREE);
> >>>> + type = begin_class_definition (type);
> >>>> + if (type == error_mark_node)
> >>>> + return lhd_simulate_record_decl (loc, name, fields);
> >>>> +
> >>>> + for (tree field : fields)
> >>>> + finish_member_declaration (field);
> >>>> +
> >>>> + type = finish_struct (type, NULL_TREE);
> >>>> +
> >>>> + tree decl = build_decl (loc, TYPE_DECL, ident, type);
> >>>> + TYPE_NAME (type) = decl;
> >>>> + TYPE_STUB_DECL (type) = decl;
> >>>
> >>> Setting TYPE_NAME and TYPE_STUB_DECL to the typedef is wrong; it should
> >>> work to just remove these two lines. I expect they're also wrong for
> C.
> >>>
> >>> For C++ only, I wonder if you need this typedef at all.
> >>>
> >>> If you do want it, you need to use set_underlying_type to create a real
> >>> typedef. I expect that's also true for C.
> >>
> >> Ah, yeah, thanks for the pointer. Fixed in the patch below.
> >>
> >> I wanted the hook to simulate the typedef even for C++ because its
> >> first user will be arm_neon.h. The spec for arm_neon.h says that the
> >> types must be declared as:
> >>
> >> typedef struct int32x2x4_t { … } int32x2x4_t;
> >>
> >> etc. So, although it's a silly edge case, code that tries to take
> >> advantage of the struct stat hack, such as:
> >>
> >> #include <arm_neon.h>
> >> struct int32x2x4_t int32x2x4_t = {};
> >>
> >> should continue to be rejected for C++ as well as C.
> >>
> >> Maybe in future we could add a flag to suppress the typedef if some
> >> callers prefer that behaviour.
> >>
> >> Tested as before.
> >
> > Can the C++ hook go in cp-lang.c (which already includes
> > langhooks-def.h) instead of decl.c? With that change, the patch is OK
> > in a week if nobody else has feedback.
>
> Thanks. I just tried with that change, but it breaks Objective C++ builds,
> since cp-lang.o isn't linked there.
>
Then it's OK without that change.
Jason
