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
@@
