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. Richard > >> gcc/ >> * langhooks.h (lang_hooks_for_types::simulate_record_decl): New hook. >> * langhooks-def.h (lhd_simulate_record_decl): Declare. >> (LANG_HOOKS_SIMULATE_RECORD_DECL): Define. >> (LANG_HOOKS_FOR_TYPES_INITIALIZER): Include it. >> * langhooks.c (lhd_simulate_record_decl): New function. >> >> gcc/c/ >> * c-tree.h (c_simulate_record_decl): Declare. >> * c-objc-common.h (LANG_HOOKS_SIMULATE_RECORD_DECL): Override. >> * c-decl.c (c_simulate_record_decl): New function. >> >> gcc/cp/ >> * decl.c: Include langhooks-def.h. >> (cxx_simulate_record_decl): New function. >> * cp-objcp-common.h (cxx_simulate_record_decl): Declare. >> (LANG_HOOKS_SIMULATE_RECORD_DECL): Override. >> --- >> gcc/c/c-decl.c | 30 ++++++++++++++++++++++++++++++ >> gcc/c/c-objc-common.h | 2 ++ >> gcc/c/c-tree.h | 2 ++ >> gcc/cp/cp-objcp-common.h | 4 ++++ >> gcc/cp/decl.c | 37 +++++++++++++++++++++++++++++++++++++ >> gcc/langhooks-def.h | 4 ++++ >> gcc/langhooks.c | 19 +++++++++++++++++++ >> gcc/langhooks.h | 10 ++++++++++ >> 8 files changed, 108 insertions(+) >> >> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c >> index 771efa3eadf..186fa1692c1 100644 >> --- a/gcc/c/c-decl.c >> +++ b/gcc/c/c-decl.c >> @@ -9436,6 +9436,36 @@ c_simulate_enum_decl (location_t loc, const char >> *name, >> input_location = saved_loc; >> return enumtype; >> } >> + >> +/* Implement LANG_HOOKS_SIMULATE_RECORD_DECL. */ >> + >> +tree >> +c_simulate_record_decl (location_t loc, const char *name, >> + array_slice<const tree> fields) >> +{ >> + location_t saved_loc = input_location; >> + input_location = loc; >> + >> + class c_struct_parse_info *struct_info; >> + tree ident = get_identifier (name); >> + tree type = start_struct (loc, RECORD_TYPE, ident, &struct_info); >> + >> + for (unsigned int i = 0; i < fields.size (); ++i) >> + { >> + DECL_FIELD_CONTEXT (fields[i]) = type; >> + if (i > 0) >> + DECL_CHAIN (fields[i - 1]) = fields[i]; >> + } >> + >> + finish_struct (loc, type, fields[0], NULL_TREE, struct_info); >> + >> + tree decl = build_decl (loc, TYPE_DECL, ident, type); >> + set_underlying_type (decl); >> + lang_hooks.decls.pushdecl (decl); >> + >> + input_location = saved_loc; >> + return type; >> +} >> >> /* Create the FUNCTION_DECL for a function definition. >> DECLSPECS, DECLARATOR and ATTRIBUTES are the parts of >> diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h >> index 7d35a0621e4..f4e8271f06c 100644 >> --- a/gcc/c/c-objc-common.h >> +++ b/gcc/c/c-objc-common.h >> @@ -81,6 +81,8 @@ along with GCC; see the file COPYING3. If not see >> >> #undef LANG_HOOKS_SIMULATE_ENUM_DECL >> #define LANG_HOOKS_SIMULATE_ENUM_DECL c_simulate_enum_decl >> +#undef LANG_HOOKS_SIMULATE_RECORD_DECL >> +#define LANG_HOOKS_SIMULATE_RECORD_DECL c_simulate_record_decl >> #undef LANG_HOOKS_TYPE_FOR_MODE >> #define LANG_HOOKS_TYPE_FOR_MODE c_common_type_for_mode >> #undef LANG_HOOKS_TYPE_FOR_SIZE >> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h >> index a046c6b0926..f1dbbd5d573 100644 >> --- a/gcc/c/c-tree.h >> +++ b/gcc/c/c-tree.h >> @@ -598,6 +598,8 @@ extern tree finish_struct (location_t, tree, tree, tree, >> class c_struct_parse_info *); >> extern tree c_simulate_enum_decl (location_t, const char *, >> vec<string_int_pair> *); >> +extern tree c_simulate_record_decl (location_t, const char *, >> + array_slice<const tree>); >> extern struct c_arg_info *build_arg_info (void); >> extern struct c_arg_info *get_parm_info (bool, tree); >> extern tree grokfield (location_t, struct c_declarator *, >> diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h >> index f1704aad557..d5859406e8f 100644 >> --- a/gcc/cp/cp-objcp-common.h >> +++ b/gcc/cp/cp-objcp-common.h >> @@ -39,6 +39,8 @@ extern bool cp_handle_option (size_t, const char *, >> HOST_WIDE_INT, int, >> extern tree cxx_make_type_hook (tree_code); >> extern tree cxx_simulate_enum_decl (location_t, const char *, >> vec<string_int_pair> *); >> +extern tree cxx_simulate_record_decl (location_t, const char *, >> + array_slice<const tree>); >> >> /* Lang hooks that are shared between C++ and ObjC++ are defined here. >> Hooks >> specific to C++ or ObjC++ go in cp/cp-lang.c and objcp/objcp-lang.c, >> @@ -139,6 +141,8 @@ extern tree cxx_simulate_enum_decl (location_t, const >> char *, >> #define LANG_HOOKS_MAKE_TYPE cxx_make_type_hook >> #undef LANG_HOOKS_SIMULATE_ENUM_DECL >> #define LANG_HOOKS_SIMULATE_ENUM_DECL cxx_simulate_enum_decl >> +#undef LANG_HOOKS_SIMULATE_RECORD_DECL >> +#define LANG_HOOKS_SIMULATE_RECORD_DECL cxx_simulate_record_decl >> #undef LANG_HOOKS_TYPE_FOR_MODE >> #define LANG_HOOKS_TYPE_FOR_MODE c_common_type_for_mode >> #undef LANG_HOOKS_TYPE_FOR_SIZE >> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c >> index 561debe6a0e..db8ec90fef1 100644 >> --- a/gcc/cp/decl.c >> +++ b/gcc/cp/decl.c >> @@ -57,6 +57,7 @@ along with GCC; see the file COPYING3. If not see >> #include "omp-general.h" >> #include "omp-offload.h" /* For offload_vars. */ >> #include "opts.h" >> +#include "langhooks-def.h" /* For lhd_simulate_record_decl */ >> >> /* Possible cases of bad specifiers type used by bad_specifiers. */ >> enum bad_spec_place { >> @@ -16601,6 +16602,42 @@ cxx_simulate_enum_decl (location_t loc, const char >> *name, >> input_location = saved_loc; >> return enumtype; >> } >> + >> +/* Implement LANG_HOOKS_SIMULATE_RECORD_DECL. */ >> + >> +tree >> +cxx_simulate_record_decl (location_t loc, const char *name, >> + array_slice<const tree> fields) >> +{ >> + iloc_sentinel ils (loc); >> + >> + tree ident = get_identifier (name); >> + tree type = xref_tag (/*tag_code=*/record_type, ident); >> + if (type != error_mark_node >> + && (TREE_CODE (type) != RECORD_TYPE || COMPLETE_TYPE_P (type))) >> + { >> + error ("redefinition of %q#T", type); >> + type = error_mark_node; >> + } >> + if (type == error_mark_node) >> + return lhd_simulate_record_decl (loc, name, fields); >> + >> + 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); >> + set_underlying_type (decl); >> + lang_hooks.decls.pushdecl (decl); >> + >> + return type; >> +} >> >> /* We're defining DECL. Make sure that its type is OK. */ >> >> diff --git a/gcc/langhooks-def.h b/gcc/langhooks-def.h >> index 02b4681dd96..5f17620aaeb 100644 >> --- a/gcc/langhooks-def.h >> +++ b/gcc/langhooks-def.h >> @@ -56,6 +56,8 @@ extern void lhd_overwrite_decl_assembler_name (tree decl, >> tree name); >> extern bool lhd_warn_unused_global_decl (const_tree); >> extern tree lhd_simulate_enum_decl (location_t, const char *, >> vec<string_int_pair> *); >> +extern tree lhd_simulate_record_decl (location_t, const char *, >> + array_slice<const tree>); >> extern tree lhd_type_for_size (unsigned precision, int unsignedp); >> extern void lhd_incomplete_type_error (location_t, const_tree, const_tree); >> extern tree lhd_type_promotes_to (tree); >> @@ -183,6 +185,7 @@ extern tree lhd_unit_size_without_reusable_padding >> (tree); >> >> #define LANG_HOOKS_MAKE_TYPE lhd_make_node >> #define LANG_HOOKS_SIMULATE_ENUM_DECL lhd_simulate_enum_decl >> +#define LANG_HOOKS_SIMULATE_RECORD_DECL lhd_simulate_record_decl >> #define LANG_HOOKS_CLASSIFY_RECORD NULL >> #define LANG_HOOKS_TYPE_FOR_SIZE lhd_type_for_size >> #define LANG_HOOKS_INCOMPLETE_TYPE_ERROR lhd_incomplete_type_error >> @@ -217,6 +220,7 @@ extern tree lhd_unit_size_without_reusable_padding >> (tree); >> #define LANG_HOOKS_FOR_TYPES_INITIALIZER { \ >> LANG_HOOKS_MAKE_TYPE, \ >> LANG_HOOKS_SIMULATE_ENUM_DECL, \ >> + LANG_HOOKS_SIMULATE_RECORD_DECL, \ >> LANG_HOOKS_CLASSIFY_RECORD, \ >> LANG_HOOKS_TYPE_FOR_MODE, \ >> LANG_HOOKS_TYPE_FOR_SIZE, \ >> diff --git a/gcc/langhooks.c b/gcc/langhooks.c >> index 48c72377778..49613b42077 100644 >> --- a/gcc/langhooks.c >> +++ b/gcc/langhooks.c >> @@ -516,6 +516,25 @@ lhd_simulate_enum_decl (location_t loc, const char >> *name, >> return enumtype; >> } >> >> +/* Default implementation of LANG_HOOKS_SIMULATE_RECORD_DECL. >> + Just create a normal RECORD_TYPE and a TYPE_DECL for it. */ >> +tree >> +lhd_simulate_record_decl (location_t loc, const char *name, >> + array_slice<const tree> fields) >> +{ >> + for (unsigned int i = 1; i < fields.size (); ++i) >> + /* Reversed by finish_builtin_struct. */ >> + DECL_CHAIN (fields[i]) = fields[i - 1]; >> + >> + tree type = lang_hooks.types.make_type (RECORD_TYPE); >> + finish_builtin_struct (type, name, fields.back (), NULL_TREE); >> + >> + tree decl = build_decl (loc, TYPE_DECL, get_identifier (name), type); >> + lang_hooks.decls.pushdecl (decl); >> + >> + return type; >> +} >> + >> /* Default implementation of LANG_HOOKS_TYPE_FOR_SIZE. >> Return an integer type with PRECISION bits of precision, >> that is unsigned if UNSIGNEDP is nonzero, otherwise signed. */ >> diff --git a/gcc/langhooks.h b/gcc/langhooks.h >> index ffd3e0bf2db..3e89134e8b4 100644 >> --- a/gcc/langhooks.h >> +++ b/gcc/langhooks.h >> @@ -68,6 +68,16 @@ struct lang_hooks_for_types >> them all with the given source location. */ >> tree (*simulate_enum_decl) (location_t, const char *, >> vec<string_int_pair> *); >> >> + /* Do the equivalent of: >> + >> + typedef struct NAME { FIELDS; } NAME; >> + >> + associating it with location LOC. Return the associated RECORD_TYPE. >> + >> + FIELDS is a list of FIELD_DECLs, in layout order. */ >> + tree (*simulate_record_decl) (location_t loc, const char *name, >> + array_slice<const tree> fields); >> + >> /* Return what kind of RECORD_TYPE this is, mainly for purposes of >> debug information. If not defined, record types are assumed to >> be structures. */ >>