diff mbox

[Ada] RFA: Add some "missing" integer_one_node conversions

Message ID 87iopd1ln1.fsf@talisman.default
State New
Headers show

Commit Message

Richard Sandiford May 10, 2014, 7:25 p.m. UTC
While testing another patch, I hit cases where Ada was building or
folding additions involving integer_one_node without converting
it to the type of the other operand.  This looked unintentional,
since all other uses of integer_one_node seemed to have the conversion.

Tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/ada/
	* gcc-interface/decl.c (gnat_to_gnu_entity): Convert
	integer_one_node to the appropriate type.
	* gcc-interface/trans.c (gnat_to_gnu): Likewise.
	(pos_to_constructor): Likewise.

Comments

Richard Biener May 13, 2014, 8:54 a.m. UTC | #1
On Sat, May 10, 2014 at 9:25 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> While testing another patch, I hit cases where Ada was building or
> folding additions involving integer_one_node without converting
> it to the type of the other operand.  This looked unintentional,
> since all other uses of integer_one_node seemed to have the conversion.
>
> Tested on x86_64-linux-gnu.  OK to install?

Hum.  Unless convert () is required for some odd reason I'd prefer
to convert these to build_int_cst (gnu_string_index_type, 1) and the
like.

Or finally add an int_const_binop overload with a HWI 2nd operand.

Of course in the end it's Ada maintainers call.

Richard.

> Thanks,
> Richard
>
>
> gcc/ada/
>         * gcc-interface/decl.c (gnat_to_gnu_entity): Convert
>         integer_one_node to the appropriate type.
>         * gcc-interface/trans.c (gnat_to_gnu): Likewise.
>         (pos_to_constructor): Likewise.
>
> Index: gcc/ada/gcc-interface/decl.c
> ===================================================================
> --- gcc/ada/gcc-interface/decl.c        2014-05-10 19:45:34.404009508 +0100
> +++ gcc/ada/gcc-interface/decl.c        2014-05-10 20:21:24.169587544 +0100
> @@ -2840,7 +2840,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
>           = build_binary_op (PLUS_EXPR, gnu_string_index_type,
>                              gnu_lower_bound,
>                              int_const_binop (MINUS_EXPR, gnu_length,
> -                                             integer_one_node));
> +                                             convert (gnu_string_index_type,
> +                                                      integer_one_node)));
>         tree gnu_index_type
>           = create_index_type (convert (sizetype, gnu_lower_bound),
>                                convert (sizetype, gnu_upper_bound),
> Index: gcc/ada/gcc-interface/trans.c
> ===================================================================
> --- gcc/ada/gcc-interface/trans.c       2014-05-10 19:45:34.404009508 +0100
> +++ gcc/ada/gcc-interface/trans.c       2014-05-10 20:21:24.160587477 +0100
> @@ -5597,6 +5597,7 @@ gnat_to_gnu (Node_Id gnat_node)
>           int length = String_Length (gnat_string);
>           int i;
>           tree gnu_idx = TYPE_MIN_VALUE (TYPE_DOMAIN (gnu_result_type));
> +         tree gnu_one_node = convert (TREE_TYPE (gnu_idx), integer_one_node);
>           vec<constructor_elt, va_gc> *gnu_vec;
>           vec_alloc (gnu_vec, length);
>
> @@ -5606,7 +5607,7 @@ gnat_to_gnu (Node_Id gnat_node)
>                                       Get_String_Char (gnat_string, i + 1));
>
>               CONSTRUCTOR_APPEND_ELT (gnu_vec, gnu_idx, t);
> -             gnu_idx = int_const_binop (PLUS_EXPR, gnu_idx, integer_one_node);
> +             gnu_idx = int_const_binop (PLUS_EXPR, gnu_idx, gnu_one_node);
>             }
>
>           gnu_result = gnat_build_constructor (gnu_result_type, gnu_vec);
> @@ -9092,7 +9093,9 @@ pos_to_constructor (Node_Id gnat_expr, t
>        CONSTRUCTOR_APPEND_ELT (gnu_expr_vec, gnu_index,
>                               convert (TREE_TYPE (gnu_array_type), gnu_expr));
>
> -      gnu_index = int_const_binop (PLUS_EXPR, gnu_index, integer_one_node);
> +      gnu_index = int_const_binop (PLUS_EXPR, gnu_index,
> +                                  convert (TREE_TYPE (gnu_index),
> +                                           integer_one_node));
>      }
>
>    return gnat_build_constructor (gnu_array_type, gnu_expr_vec);
Eric Botcazou May 14, 2014, 10:09 a.m. UTC | #2
> 	* gcc-interface/decl.c (gnat_to_gnu_entity): Convert
> 	integer_one_node to the appropriate type.
> 	* gcc-interface/trans.c (gnat_to_gnu): Likewise.
> 	(pos_to_constructor): Likewise.

OK for the sake of consistency, thanks.
diff mbox

Patch

Index: gcc/ada/gcc-interface/decl.c
===================================================================
--- gcc/ada/gcc-interface/decl.c	2014-05-10 19:45:34.404009508 +0100
+++ gcc/ada/gcc-interface/decl.c	2014-05-10 20:21:24.169587544 +0100
@@ -2840,7 +2840,8 @@  gnat_to_gnu_entity (Entity_Id gnat_entit
 	  = build_binary_op (PLUS_EXPR, gnu_string_index_type,
 			     gnu_lower_bound,
 			     int_const_binop (MINUS_EXPR, gnu_length,
-					      integer_one_node));
+					      convert (gnu_string_index_type,
+						       integer_one_node)));
 	tree gnu_index_type
 	  = create_index_type (convert (sizetype, gnu_lower_bound),
 			       convert (sizetype, gnu_upper_bound),
Index: gcc/ada/gcc-interface/trans.c
===================================================================
--- gcc/ada/gcc-interface/trans.c	2014-05-10 19:45:34.404009508 +0100
+++ gcc/ada/gcc-interface/trans.c	2014-05-10 20:21:24.160587477 +0100
@@ -5597,6 +5597,7 @@  gnat_to_gnu (Node_Id gnat_node)
 	  int length = String_Length (gnat_string);
 	  int i;
 	  tree gnu_idx = TYPE_MIN_VALUE (TYPE_DOMAIN (gnu_result_type));
+	  tree gnu_one_node = convert (TREE_TYPE (gnu_idx), integer_one_node);
 	  vec<constructor_elt, va_gc> *gnu_vec;
 	  vec_alloc (gnu_vec, length);
 
@@ -5606,7 +5607,7 @@  gnat_to_gnu (Node_Id gnat_node)
 				      Get_String_Char (gnat_string, i + 1));
 
 	      CONSTRUCTOR_APPEND_ELT (gnu_vec, gnu_idx, t);
-	      gnu_idx = int_const_binop (PLUS_EXPR, gnu_idx, integer_one_node);
+	      gnu_idx = int_const_binop (PLUS_EXPR, gnu_idx, gnu_one_node);
 	    }
 
 	  gnu_result = gnat_build_constructor (gnu_result_type, gnu_vec);
@@ -9092,7 +9093,9 @@  pos_to_constructor (Node_Id gnat_expr, t
       CONSTRUCTOR_APPEND_ELT (gnu_expr_vec, gnu_index,
 			      convert (TREE_TYPE (gnu_array_type), gnu_expr));
 
-      gnu_index = int_const_binop (PLUS_EXPR, gnu_index, integer_one_node);
+      gnu_index = int_const_binop (PLUS_EXPR, gnu_index,
+				   convert (TREE_TYPE (gnu_index),
+					    integer_one_node));
     }
 
   return gnat_build_constructor (gnu_array_type, gnu_expr_vec);