This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Ada PATCH] don't modify record fields in update_pointer_to
- From: Duncan Sands <baldrick at free dot fr>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Eric Botcazou <ebotcazou at adacore dot com>, Arnaud Charlet <charlet at adacore dot com>
- Date: Thu, 7 Jun 2007 19:11:20 +0200
- Subject: Re: [Ada PATCH] don't modify record fields in update_pointer_to
- References: <200702211356.45659.baldrick@free.fr> <200704302351.47014.baldrick@free.fr> <200706041547.21879.ebotcazou@adacore.com>
Hi Eric,
> Thanks for your work. The last missing bits from the AdaCore tree as of April
> 20 should be merged really soon now, and they happen to again conflict with
> your changes, albeit in a minor way (at the very end of update_pointer_to).
I've redone the patch (attached) based on the tree as of today. It bootstraps
and does not introduce any new failures in the testsuite.
> This looks good on principle and we are considering merging it. A few nits:
>
> @@ -3006,8 +3006,8 @@
>
> /* If we are pointing to an incomplete type whose completion is an
> unconstrained array, make a fat pointer type instead of a pointer
> - to VOID. The two types in our fields will be pointers to VOID and
> - will be replaced in update_pointer_to. Similarly, if the type
> + to VOID. The two types in our fields will be pointers to dummy nodes
> + and will be replaced in update_pointer_to. Similarly, if the type
> itself is a dummy type or an unconstrained array. Also make
> a dummy TYPE_OBJECT_RECORD_TYPE in case we have any thin
> pointers to it. */
>
> We only make a pointer to VOID in a couple of specific cases so I think that
> you can remove "to VOID" altogether.
Done.
> @@ -3053,6 +3053,21 @@
> gnu_type = TYPE_POINTER_TO (gnu_old);
> if (!gnu_type)
> {
> + tree gnu_template_type = make_node (ENUMERAL_TYPE);
> + tree gnu_ptr_template = build_pointer_type (gnu_template_type);
> + tree gnu_array_type = make_node (ENUMERAL_TYPE);
> + tree gnu_ptr_array = build_pointer_type (gnu_array_type);
> +
> + TYPE_NAME (gnu_template_type)
> + = concat_id_with_name (get_entity_name (gnat_desig_type),
> + "XUB");
> + TYPE_DUMMY_P (gnu_template_type) = 1;
> +
> + TYPE_NAME (gnu_array_type)
> + = concat_id_with_name (get_entity_name (gnat_desig_type),
> + "XUA");
> + TYPE_DUMMY_P (gnu_array_type) = 1;
> +
> gnu_type = make_node (RECORD_TYPE);
> SET_TYPE_UNCONSTRAINED_ARRAY (gnu_type, gnu_old);
> TYPE_POINTER_TO (gnu_old) = gnu_type;
>
> The entity whose name is used to build the XUT record is gnat_desig_equiv just
> below, so I think that it should be used for the XUA and XUB types too.
I didn't find gnat_desig_equiv anywhere, and XUT is also built using gnat_desig_type.
So I didn't make any changes here.
> @@ -3002,13 +2994,12 @@
>
> update_pointer_to (TYPE_OBJECT_RECORD_TYPE (old_type), new_obj_rec);
>
> - TREE_TYPE (TYPE_FIELDS (new_obj_rec)) = TREE_TYPE (ptr_temp_type);
> TREE_TYPE (TREE_CHAIN (TYPE_FIELDS (new_obj_rec)))
> - = TREE_TYPE (TREE_TYPE (new_fields));
> + = TREE_TYPE (TREE_TYPE (array_field));
> DECL_SIZE (TREE_CHAIN (TYPE_FIELDS (new_obj_rec)))
> - = TYPE_SIZE (TREE_TYPE (TREE_TYPE (new_fields)));
> + = TYPE_SIZE (TREE_TYPE (TREE_TYPE (array_field)));
> DECL_SIZE_UNIT (TREE_CHAIN (TYPE_FIELDS (new_obj_rec)))
> - = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (new_fields)));
> + = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (array_field)));
>
> TYPE_SIZE (new_obj_rec)
> = size_binop (PLUS_EXPR,
>
> Why are you removing the first line instead of updating it?
The first line is redundant: the field already has this type. Just to be
sure I added an assertion that this was true, and then bootstrapped and
ran the ACATS and gnat test suites - the assertion never fired. You might
also wonder why the second line is not redundant: that's because the array
type was modified a few lines above (gnat_substitute_in_type).
I also modified a comment in update_pointer_to which was still talking about
void pointers.
Ciao,
Duncan.
Index: gcc.fsf.master/gcc/ada/decl.c
===================================================================
--- gcc.fsf.master.orig/gcc/ada/decl.c 2007-06-07 17:43:48.000000000 +0200
+++ gcc.fsf.master/gcc/ada/decl.c 2007-06-07 17:50:33.000000000 +0200
@@ -3041,12 +3041,11 @@
&& ! Is_Constrained (gnat_desig_rep));
/* If we are pointing to an incomplete type whose completion is an
- unconstrained array, make a fat pointer type instead of a pointer
- to VOID. The two types in our fields will be pointers to VOID and
- will be replaced in update_pointer_to. Similarly, if the type
- itself is a dummy type or an unconstrained array. Also make
- a dummy TYPE_OBJECT_RECORD_TYPE in case we have any thin
- pointers to it. */
+ unconstrained array, make a fat pointer type. The two types in our
+ fields will be pointers to dummy nodes and will be replaced in
+ update_pointer_to. Similarly, if the type itself is a dummy type or
+ an unconstrained array. Also make a dummy TYPE_OBJECT_RECORD_TYPE
+ in case we have any thin pointers to it. */
if (is_unconstrained_array
&& (Present (gnat_desig_full)
@@ -3075,6 +3074,21 @@
gnu_type = TYPE_POINTER_TO (gnu_old);
if (!gnu_type)
{
+ tree gnu_template_type = make_node (ENUMERAL_TYPE);
+ tree gnu_ptr_template = build_pointer_type (gnu_template_type);
+ tree gnu_array_type = make_node (ENUMERAL_TYPE);
+ tree gnu_ptr_array = build_pointer_type (gnu_array_type);
+
+ TYPE_NAME (gnu_template_type)
+ = concat_id_with_name (get_entity_name (gnat_desig_type),
+ "XUB");
+ TYPE_DUMMY_P (gnu_template_type) = 1;
+
+ TYPE_NAME (gnu_array_type)
+ = concat_id_with_name (get_entity_name (gnat_desig_type),
+ "XUA");
+ TYPE_DUMMY_P (gnu_array_type) = 1;
+
gnu_type = make_node (RECORD_TYPE);
SET_TYPE_UNCONSTRAINED_ARRAY (gnu_type, gnu_old);
TYPE_POINTER_TO (gnu_old) = gnu_type;
@@ -3084,10 +3098,10 @@
= chainon (chainon (NULL_TREE,
create_field_decl
(get_identifier ("P_ARRAY"),
- ptr_void_type_node, gnu_type,
- 0, 0, 0, 0)),
+ gnu_ptr_array,
+ gnu_type, 0, 0, 0, 0)),
create_field_decl (get_identifier ("P_BOUNDS"),
- ptr_void_type_node,
+ gnu_ptr_template,
gnu_type, 0, 0, 0, 0));
/* Make sure we can place this into a register. */
Index: gcc.fsf.master/gcc/ada/trans.c
===================================================================
--- gcc.fsf.master.orig/gcc/ada/trans.c 2007-06-07 17:43:48.000000000 +0200
+++ gcc.fsf.master/gcc/ada/trans.c 2007-06-07 17:50:33.000000000 +0200
@@ -5201,19 +5201,6 @@
return GS_ALL_DONE;
}
- return GS_UNHANDLED;
-
- case COMPONENT_REF:
- /* We have a kludge here. If the FIELD_DECL is from a fat pointer and is
- from an early dummy type, replace it with the proper FIELD_DECL. */
- if (TYPE_FAT_POINTER_P (TREE_TYPE (TREE_OPERAND (*expr_p, 0)))
- && DECL_ORIGINAL_FIELD (TREE_OPERAND (*expr_p, 1)))
- {
- TREE_OPERAND (*expr_p, 1)
- = DECL_ORIGINAL_FIELD (TREE_OPERAND (*expr_p, 1));
- return GS_OK;
- }
-
/* ... fall through ... */
default:
Index: gcc.fsf.master/gcc/ada/utils.c
===================================================================
--- gcc.fsf.master.orig/gcc/ada/utils.c 2007-06-07 17:43:48.000000000 +0200
+++ gcc.fsf.master/gcc/ada/utils.c 2007-06-07 17:50:33.000000000 +0200
@@ -3160,29 +3160,23 @@
}
/* Now deal with the unconstrained array case. In this case the "pointer"
- is actually a RECORD_TYPE where the types of both fields are
- pointers to void. In that case, copy the field list from the
- old type to the new one and update the fields' context. */
+ is actually a RECORD_TYPE where both fields are pointers to dummy nodes.
+ Turn them into pointers to the correct types using update_pointer_to. */
else if (TREE_CODE (ptr) != RECORD_TYPE || !TYPE_IS_FAT_POINTER_P (ptr))
gcc_unreachable ();
else
{
tree new_obj_rec = TYPE_OBJECT_RECORD_TYPE (new_type);
- tree fields = TYPE_FIELDS (TYPE_POINTER_TO (new_type));
- tree new_fields, ptr_temp_type, new_ref, bounds, var;
-
- /* Replace contents of old pointer with those of new pointer. */
- new_fields = copy_node (fields);
- TREE_CHAIN (new_fields) = copy_node (TREE_CHAIN (fields));
-
- SET_DECL_ORIGINAL_FIELD (TYPE_FIELDS (ptr), new_fields);
- SET_DECL_ORIGINAL_FIELD (TREE_CHAIN (TYPE_FIELDS (ptr)),
- TREE_CHAIN (new_fields));
-
- TYPE_FIELDS (ptr) = new_fields;
- DECL_CONTEXT (new_fields) = ptr;
- DECL_CONTEXT (TREE_CHAIN (new_fields)) = ptr;
+ tree array_field = TYPE_FIELDS (ptr);
+ tree bounds_field = TREE_CHAIN (TYPE_FIELDS (ptr));
+ tree new_ptr = TYPE_POINTER_TO (new_type);
+ tree new_ref;
+ tree var;
+
+ update_pointer_to
+ (TREE_TYPE (TREE_TYPE (bounds_field)),
+ TREE_TYPE (TREE_TYPE (TREE_CHAIN (TYPE_FIELDS (new_ptr)))));
/* Rework the PLACEHOLDER_EXPR inside the reference to the template
bounds and update the pointers to them.
@@ -3190,24 +3184,21 @@
??? This is now the only use of gnat_substitute_in_type, which
is now a very "heavy" routine to do this, so it should be replaced
at some point. */
- bounds = TREE_TYPE (TREE_TYPE (new_fields));
- ptr_temp_type = TREE_TYPE (TREE_CHAIN (new_fields));
- new_ref = build3 (COMPONENT_REF, ptr_temp_type,
+ new_ref = build3 (COMPONENT_REF, TREE_TYPE (bounds_field),
build0 (PLACEHOLDER_EXPR, ptr),
- TREE_CHAIN (new_fields), NULL_TREE);
- update_pointer_to (bounds,
- gnat_substitute_in_type (bounds,
- TREE_CHAIN (fields),
- new_ref));
+ bounds_field, NULL_TREE);
- for (var = TYPE_MAIN_VARIANT (ptr); var; var = TYPE_NEXT_VARIANT (var))
- {
- SET_TYPE_UNCONSTRAINED_ARRAY (var, new_type);
+ update_pointer_to
+ (TREE_TYPE (TREE_TYPE (TYPE_FIELDS (new_ptr))),
+ gnat_substitute_in_type (TREE_TYPE (TREE_TYPE (TYPE_FIELDS (new_ptr))),
+ TREE_CHAIN (TYPE_FIELDS (new_ptr)), new_ref));
+
+ update_pointer_to
+ (TREE_TYPE (TREE_TYPE (array_field)),
+ TREE_TYPE (TREE_TYPE (TYPE_FIELDS (new_ptr))));
- /* This may seem a bit gross, in particular wrt DECL_CONTEXT, but
- actually is in keeping with what build_qualified_type does. */
- TYPE_FIELDS (var) = new_fields;
- }
+ for (var = TYPE_MAIN_VARIANT (ptr); var; var = TYPE_NEXT_VARIANT (var))
+ SET_TYPE_UNCONSTRAINED_ARRAY (var, new_type);
TYPE_POINTER_TO (new_type) = TYPE_REFERENCE_TO (new_type)
= TREE_TYPE (new_type) = ptr;
@@ -3218,11 +3209,8 @@
update_pointer_to (TYPE_OBJECT_RECORD_TYPE (old_type), new_obj_rec);
- TREE_TYPE (TYPE_FIELDS (new_obj_rec))
- = TREE_TYPE (TREE_TYPE (TREE_CHAIN (new_fields)));
-
TREE_TYPE (TREE_CHAIN (TYPE_FIELDS (new_obj_rec)))
- = TREE_TYPE (TREE_TYPE (new_fields));
+ = TREE_TYPE (TREE_TYPE (array_field));
/* The size recomputation needs to account for alignment constraints, so
we let layout_type work it out. This will reset the field offsets to
Index: gcc.fsf.master/gcc/ada/ChangeLog
===================================================================
--- gcc.fsf.master.orig/gcc/ada/ChangeLog 2007-06-07 17:43:48.000000000 +0200
+++ gcc.fsf.master/gcc/ada/ChangeLog 2007-06-07 17:51:02.000000000 +0200
@@ -1,3 +1,11 @@
+2007-06-07 Duncan Sands <baldrick@free.fr>
+
+ * decl.c (gnat_to_gnu_entity): Use pointers to dummy nodes, rather than
+ void*, for the fields when making a new fat pointer type.
+ * trans.c (gnat_gimplify_expr): Remove COMPONENT_REF kludge.
+ * utils.c (update_pointer_to): Update fat pointers by updating the dummy
+ node pointers used for the fields.
+
2007-06-06 Thomas Quinot <quinot@adacore.com>
Bob Duff <duff@adacore.com>