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
