Hi all,
in attachment the patch which includes the review comments provided by Tobias.
The patch is bootstrapped and tested on x86_64-unknown-linux-gnu.
Regards.
Alessandro
2012/5/20 Tobias Burnus <bur...@net-b.de>:
> Hi Alessandro,
>
>
> Alessandro Fanfarillo wrote:
>>
>> in attachment there's a patch for PR 48831, it also includes a new
>> test case suggested by Tobias Burnus.
>> The patch is bootstrapped and tested on x86_64-unknown-linux-gnu.
>
>
> Please try to ensure that your patch has a text mime type - it shows up as
> Content-Type: application/octet-stream;
> which makes reading, reviewing and quoting your patch more difficult.
>
> PR fortran/48831
> * gfortran.h: Add non-static prototype declaration
> of check_init_expr function.
> * check.c (kind_check): Change if condition related
> to check_init_expr.
> * expr.c: Remove prototype declaration of
> check_init_expr function and static keyword.
>
>
> You should add the name of the function you change in parentheses, e.g.
>
> * gfortran.h (check_init_expr): Add prototype declaration of function.
>
> (The "non-static" is superfluous as static functions shouldn't be in header
> files.) For "check_init_expr" I'd use "Remove forward declaration" instead
> of "Remove prototype declaration" but that's personal style. But again, you
> should include the function name in parentheses. The reason is that one can
> more quickly find it, if it is always at the same spot.
>
> As mentioned before, the gfortran convention is to prefix functions (gfc_) -
> at least those which are nonstatic. Please change the function name.
>
> - if (k->expr_type != EXPR_CONSTANT)
> + if (check_init_expr(k) != SUCCESS)
>
>
> GNU style: Add a space before the "(" of the function argument:
> "check_init_expr (k)".
>
>
> +/* Check an intrinsic arithmetic operation to see if it is consistent
> + with some type of expression. */
> +gfc_try check_init_expr (gfc_expr *);
>
>
>
> I have to admit that after reading only the comment, I had no idea what the
> function does - especially the "some type" is not really helpful. How about
> a simple "Check whether an expression is an initialization/constant
> expression." Initialization and constant expressions are well defined in
> the Fortran standard. (Actually, I find the function name speaks already for
> itself, thus, I do not see the need for a comment, but I also do not mind a
> comment.)
>
> (One problem with the name "constant expression" vs. "initialization
> expression" is that Fortran 90/95 distinguish between them while Fortran
> 2003/2008 have merged them to a single type of expression; Fortran 2003
> calls the merged expression type "initialization expression" while Fortran
> 2008 calls them "constant expressions". In principle, gfortran should make
> the distinction with -std=f95 and reject expressions which are nonconstant
> and only initexpressions when the standard demands it, but I am not sure
> whether gfortran does. That part of gfortran is a bit unclean and the
> distinction between init/const expr is nowadays largely ignored by the
> gfortran developers.)
>
> Otherwise, the patch looks OK.
>
> Tobias
2012-06-03 Alessandro Fanfarillo <fanfarillo....@gmail.com>
Tobias Burnus <bur...@net-b.de>
PR fortran/48831
* gfortran.h (check_init_expr): Add prototype declaration
of function.
* check.c (kind_check): Change if condition related
to check_init_expr.
* expr.c (check_init_expr): Remove forward declaration
and static keyword. Change name in gfc_check_init_expr.
2012-06-03 Alessandro Fanfarillo <fanfarillo....@gmail.com>
PR fortran/48831
* gfortran.dg/parameter_array_element_2.f90: New.
Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c (revisione 188147)
+++ gcc/fortran/expr.c (copia locale)
@@ -1943,12 +1943,6 @@
}
-/* Check an intrinsic arithmetic operation to see if it is consistent
- with some type of expression. */
-
-static gfc_try check_init_expr (gfc_expr *);
-
-
/* Scalarize an expression for an elemental intrinsic call. */
static gfc_try
@@ -1994,7 +1988,7 @@
for (; a; a = a->next)
{
/* Check that this is OK for an initialization expression. */
- if (a->expr && check_init_expr (a->expr) == FAILURE)
+ if (a->expr && gfc_check_init_expr (a->expr) == FAILURE)
goto cleanup;
rank[n] = 0;
@@ -2231,7 +2225,7 @@
gfc_actual_arglist *ap;
for (ap = e->value.function.actual; ap; ap = ap->next)
- if (check_init_expr (ap->expr) == FAILURE)
+ if (gfc_check_init_expr (ap->expr) == FAILURE)
return MATCH_ERROR;
return MATCH_YES;
@@ -2319,7 +2313,7 @@
&ap->expr->where);
return MATCH_ERROR;
}
- else if (not_restricted && check_init_expr (ap->expr) == FAILURE)
+ else if (not_restricted && gfc_check_init_expr (ap->expr) == FAILURE)
return MATCH_ERROR;
if (not_restricted == 0
@@ -2437,8 +2431,8 @@
intrinsics in the context of initialization expressions. If
FAILURE is returned an error message has been generated. */
-static gfc_try
-check_init_expr (gfc_expr *e)
+gfc_try
+gfc_check_init_expr (gfc_expr *e)
{
match m;
gfc_try t;
@@ -2449,7 +2443,7 @@
switch (e->expr_type)
{
case EXPR_OP:
- t = check_intrinsic_op (e, check_init_expr);
+ t = check_intrinsic_op (e, gfc_check_init_expr);
if (t == SUCCESS)
t = gfc_simplify_expr (e, 0);
@@ -2573,11 +2567,11 @@
break;
case EXPR_SUBSTRING:
- t = check_init_expr (e->ref->u.ss.start);
+ t = gfc_check_init_expr (e->ref->u.ss.start);
if (t == FAILURE)
break;
- t = check_init_expr (e->ref->u.ss.end);
+ t = gfc_check_init_expr (e->ref->u.ss.end);
if (t == SUCCESS)
t = gfc_simplify_expr (e, 0);
@@ -2592,14 +2586,14 @@
if (t == FAILURE)
break;
- t = gfc_check_constructor (e, check_init_expr);
+ t = gfc_check_constructor (e, gfc_check_init_expr);
if (t == FAILURE)
break;
break;
case EXPR_ARRAY:
- t = gfc_check_constructor (e, check_init_expr);
+ t = gfc_check_constructor (e, gfc_check_init_expr);
if (t == FAILURE)
break;
@@ -2629,7 +2623,7 @@
gfc_init_expr_flag = true;
t = gfc_resolve_expr (expr);
if (t == SUCCESS)
- t = check_init_expr (expr);
+ t = gfc_check_init_expr (expr);
gfc_init_expr_flag = false;
if (t == FAILURE)
Index: gcc/fortran/check.c
===================================================================
--- gcc/fortran/check.c (revisione 188147)
+++ gcc/fortran/check.c (copia locale)
@@ -163,7 +163,7 @@
if (scalar_check (k, n) == FAILURE)
return FAILURE;
- if (k->expr_type != EXPR_CONSTANT)
+ if (gfc_check_init_expr (k) != SUCCESS)
{
gfc_error ("'%s' argument of '%s' intrinsic at %L must be a constant",
gfc_current_intrinsic_arg[n]->name, gfc_current_intrinsic,
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h (revisione 188147)
+++ gcc/fortran/gfortran.h (copia locale)
@@ -2708,6 +2708,7 @@
const char *gfc_extract_int (gfc_expr *, int *);
bool is_subref_array (gfc_expr *);
bool gfc_is_simply_contiguous (gfc_expr *, bool);
+gfc_try gfc_check_init_expr (gfc_expr *);
gfc_expr *gfc_build_conversion (gfc_expr *);
void gfc_free_ref_list (gfc_ref *);
Index: gcc/testsuite/gfortran.dg/parameter_array_element_2.f90
===================================================================
--- gcc/testsuite/gfortran.dg/parameter_array_element_2.f90 (revisione 0)
+++ gcc/testsuite/gfortran.dg/parameter_array_element_2.f90 (revisione 0)
@@ -0,0 +1,16 @@
+! { dg-do compile }
+! PR fortran/48831
+! Contributed by Tobias Burnus
+
+program p1
+ implicit none
+ integer, parameter :: i1 = kind(0)
+ integer, parameter :: i2(1) = [i1]
+ integer(kind=i2(1)) :: i3
+
+ i3 = int(0, i1)
+ print *, i3
+
+ i3 = int(0, i2(1)) ! This line gives an error when compiling.
+ print *, i3
+end program p1
