Hi Thomas, hi all,
I'm back from holidays and have more time to deal with this now ...
2018-06-17 0:38 GMT+02:00 Janus Weil <[email protected]>:
>
> Am 16. Juni 2018 23:14:08 MESZ schrieb Thomas Koenig <[email protected]>:
>>In my patch, I have tried to do all three things at the same time, and
>>after this discussion, I still think that this is the right path
>>to follow.
>>
>>So, here is an update on the patch, which also covers ALLOCATED.
>>
>>Regression-tested. OK?
>
> I absolutely welcome the warnings for impure functions as second operand to
> .and./.or. operators, but (as noted earlier) I disagree with changing the
> ordering of operands. As our lengthy discussions show, things are already
> complicated enough, and this additional optimization just adds to the
> confusion IMHO.
the previously proposed patch by Thomas did various things at once,
not all of which we could agree upon unfortunately.
However, there also were some things that everyone seemed to agree
upon, namely that there should be warnings for the cases where impure
functions are currently short-circuited.
The patch in the attachment contains those parts of Thomas' patch that
implement that warning, but does no further optimizations or
special-casing.
Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
Cheers,
Janus
2018-06-25 Thomas Koenig <[email protected]>
Janus Weil <[email protected]>
PR fortran/85599
* dump-parse-tree (show_attr): Add handling of implicit_pure.
* resolve.c (impure_function_callback): New function.
(resolve_operator): Call it vial gfc_expr_walker.
2018-06-25 Janus Weil <[email protected]>
PR fortran/85599
* gfortran.dg/short_circuiting.f90: New test.
Index: gcc/fortran/dump-parse-tree.c
===================================================================
--- gcc/fortran/dump-parse-tree.c (revision 262024)
+++ gcc/fortran/dump-parse-tree.c (working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
fputs (" ELEMENTAL", dumpfile);
if (attr->pure)
fputs (" PURE", dumpfile);
+ if (attr->implicit_pure)
+ fputs (" IMPLICIT_PURE", dumpfile);
if (attr->recursive)
fputs (" RECURSIVE", dumpfile);
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c (revision 262024)
+++ gcc/fortran/resolve.c (working copy)
@@ -3821,6 +3821,44 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop
}
+/* Callback finding an impure function as an operand to an .and. or
+ .or. expression. Remember the last function warned about to
+ avoid double warnings when recursing. */
+
+static int
+impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
+ void *data)
+{
+ gfc_expr *f = *e;
+ const char *name;
+ static gfc_expr *last = NULL;
+ bool *found = (bool *) data;
+
+ if (f->expr_type == EXPR_FUNCTION)
+ {
+ *found = 1;
+ if (f != last && !pure_function (f, &name))
+ {
+ /* This could still be a function without side effects, i.e.
+ implicit pure. Do not warn for that case. */
+ if (f->symtree == NULL || f->symtree->n.sym == NULL
+ || !gfc_implicit_pure (f->symtree->n.sym))
+ {
+ if (name)
+ gfc_warning (OPT_Wsurprising, "Function %qs at %L "
+ "might not be evaluated", name, &f->where);
+ else
+ gfc_warning (OPT_Wsurprising, "Function at %L "
+ "might not be evaluated", &f->where);
+ }
+ }
+ last = f;
+ }
+
+ return 0;
+}
+
+
/* Resolve an operator expression node. This can involve replacing the
operation with a user defined function call. */
@@ -3929,6 +3967,14 @@ resolve_operator (gfc_expr *e)
gfc_convert_type (op1, &e->ts, 2);
else if (op2->ts.kind < e->ts.kind)
gfc_convert_type (op2, &e->ts, 2);
+
+ if (e->value.op.op == INTRINSIC_AND || e->value.op.op == INTRINSIC_OR)
+ {
+ /* Warn about short-circuiting
+ with impure function as second operand. */
+ bool op2_f = false;
+ gfc_expr_walker (&op2, impure_function_callback, &op2_f);
+ }
break;
}
! { dg-do compile }
! { dg-additional-options "-Wsurprising" }
!
! PR 85599: Prevent short-circuiting of logical expressions for non-pure functions
!
! Contributed by Janus Weil <[email protected]>
program short_circuit
logical :: flag
flag = .false.
flag = check() .and. flag
flag = flag .and. check() ! { dg-warning "might not be evaluated" }
flag = flag .and. pure_check()
contains
logical function check()
integer, save :: i = 1
print *, "check", i
i = i + 1
check = .true.
end function
logical pure function pure_check()
pure_check = .true.
end function
end
