Hi all,
here is another update of the patch. It cures the previously-mentioned
problems with generic interfaces, adds some documentation (as
suggested by Dominique) and slightly enhances the error message
(mentioning the impurity of the function we're warning about).
I tested it on a fairly large code base and found no further false
positives. Also it still regtests cleanly. Ok for trunk?
Cheers,
Janus
2018-07-15 Thomas Koenig <tkoe...@gcc.gnu.org>
Janus Weil <ja...@gcc.gnu.org>
PR fortran/85599
* dump-parse-tree.c (show_attr): Add handling of implicit_pure.
* gfortran.texi: Add chapter on evaluation of logical expressions.
* resolve.c (implicit_pure_function): New function.
(check_pure_function): Use it here.
(impure_function_callback): New function.
(resolve_operator): Call it via gfc_expr_walker.
2018-07-15 Janus Weil <ja...@gcc.gnu.org>
PR fortran/85599
* gfortran.dg/short_circuiting.f90: New test.
2018-07-13 10:02 GMT+02:00 Janus Weil <ja...@gcc.gnu.org>:
> Just noticed another problematic case: Calls to generic interfaces are
> not detected as implicitly pure, see enhanced test case in attachment.
> Will look into this problem on the weekend ...
>
> Cheers,
> Janus
>
> 2018-07-12 21:43 GMT+02:00 Janus Weil <ja...@gcc.gnu.org>:
>> Hi all,
>>
>> here is a minor update of the patch, which cures some problems with
>> implicitly pure functions in the previous version.
>>
>> Most implicitly pure functions were actually detected correctly, but
>> implicitly pure functions that called other implicitly pure functions
>> were not detected properly, and therefore could trigger a warning.
>> This is fixed in the current version, which still regtests cleanly
>> (note that alloc-comp-3.f90 currently fails due to PR 86417). The test
>> case is also enhanced to include the problematic case.
>>
>> Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>>
>> 2018-07-11 23:06 GMT+02:00 Janus Weil <ja...@gcc.gnu.org>:
>>> Hi all,
>>>
>>> after the dust of the heated discussion around this PR has settled a
>>> bit, here is another attempt to implement at least some basic warnings
>>> about compiler-dependent behavior concerning the short-circuiting of
>>> logical expressions.
>>>
>>> As a reminder (and recap of the previous discussion), the Fortran
>>> standard unfortunately is a bit sloppy in this area: It allows
>>> compilers to short-circuit the second operand of .AND. / .OR.
>>> operators, but does not require this. As a result, compilers can do
>>> what they want without conflicting with the standard, and they do:
>>> gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR),
>>> ifort does not.
>>>
>>> I'm continuing here the least-invasive approach of keeping gfortran's
>>> current behavior, but warning about cases where compilers may produce
>>> different results.
>>>
>>> The attached patch is very close to the version I posted previously
>>> (which was already approved by Janne), with the difference that the
>>> warnings are now triggered by -Wextra and not -Wsurprising (which is
>>> included in -Wall), as suggested by Nick Maclaren. I think this is
>>> more reasonable, since not everyone may want to see these warnings.
>>>
>>> Note that I don't want to warn about all possible optimizations that
>>> might be allowed by the standard, but only about those that are
>>> actually problematic in practice and result in compiler-dependent
>>> behavior.
>>>
>>> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>>
>>> Cheers,
>>> Janus
>>>
>>>
>>> 2018-07-11 Thomas Koenig <tkoe...@gcc.gnu.org>
>>> Janus Weil <ja...@gcc.gnu.org>
>>>
>>> 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-07-11 Janus Weil <ja...@gcc.gnu.org>
>>>
>>> PR fortran/85599
>>> * gfortran.dg/short_circuiting.f90: New test.
Index: gcc/fortran/dump-parse-tree.c
===================================================================
--- gcc/fortran/dump-parse-tree.c (revision 262563)
+++ 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/gfortran.texi
===================================================================
--- gcc/fortran/gfortran.texi (revision 262563)
+++ gcc/fortran/gfortran.texi (working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
@menu
* KIND Type Parameters::
* Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
* Thread-safety of the runtime library::
* Data consistency and durability::
* Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not require the compiler to evaluate all parts of an
+expression, if they do not contribute to the final result. For logical
+expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU
+Fortran will optimize out function calls (even to impure functions) if the
+result of the expression can be established without them. However, since not
+all compilers do that, and such an optimization can potentially modify the
+program flow and subsequent results, GNU Fortran throws warnings for such
+situations with the @option{-Wextra} flag.
+
+
@node Thread-safety of the runtime library
@section Thread-safety of the runtime library
@cindex thread-safety, threads
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c (revision 262563)
+++ gcc/fortran/resolve.c (working copy)
@@ -2982,6 +2982,21 @@ pure_function (gfc_expr *e, const char **name)
}
+/* Check if the expression is a reference to an implicitly pure function. */
+
+static int
+implicit_pure_function (gfc_expr *e)
+{
+ gfc_component *comp = gfc_get_proc_ptr_comp (e);
+ if (comp)
+ return gfc_implicit_pure (comp->ts.interface);
+ else if (e->value.function.esym)
+ return gfc_implicit_pure (e->value.function.esym);
+ else
+ return 0;
+}
+
+
static bool
impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym,
int *f ATTRIBUTE_UNUSED)
@@ -3034,7 +3049,8 @@ static bool check_pure_function (gfc_expr *e)
"within a PURE procedure", name, &e->where);
return false;
}
- gfc_unset_implicit_pure (NULL);
+ if (!implicit_pure_function (e))
+ gfc_unset_implicit_pure (NULL);
}
return true;
}
@@ -3822,6 +3838,40 @@ 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) && !implicit_pure_function (f))
+ {
+ if (name)
+ gfc_warning (OPT_Wextra,
+ "Impure function %qs at %L might not be evaluated",
+ name, &f->where);
+ else
+ gfc_warning (OPT_Wextra,
+ "Impure 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. */
@@ -3930,6 +3980,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 "-Wextra" }
!
! PR 85599: warn about short-circuiting of logical expressions for non-pure functions
!
! Contributed by Janus Weil <ja...@gcc.gnu.org>
module a
interface impl_pure_a
module procedure impl_pure_a1
end interface
contains
logical function impl_pure_a1()
impl_pure_a1 = .true.
end function
end module
program short_circuit
use a
logical :: flag
flag = .false.
flag = check() .and. flag
flag = flag .and. check() ! { dg-warning "might not be evaluated" }
flag = flag .and. pure_check()
flag = flag .and. impl_pure_1()
flag = flag .and. impl_pure_2()
flag = flag .and. impl_pure_a1()
flag = flag .and. impl_pure_a()
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
logical function impl_pure_1()
impl_pure_1 = .true.
end function
logical function impl_pure_2()
impl_pure_2 = impl_pure_1()
end function
end
