Re: [PATCH] Add a simulate_record_decl lang hook

Jason Merrill via Gcc-patches Tue, 19 Oct 2021 07:57:05 -0700

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 includeslanghooks-def.h) instead of decl.c? With that change, the patch is OKin a week if nobody else has feedback.

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.  */

Re: [PATCH] Add a simulate_record_decl lang hook

Reply via email to