From patchwork Wed Jul 11 21:06:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janus Weil X-Patchwork-Id: 942710 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-481385-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="lqJvWQZJ"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41Qs7M4yDjz9s01 for ; Thu, 12 Jul 2018 07:06:22 +1000 (AEST) Received: (qmail 117790 invoked by alias); 11 Jul 2018 21:06:15 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 117738 invoked by uid 89); 11 Jul 2018 21:06:11 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-9.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=dust, concerning, Hx-languages-length:4698, warned X-HELO: mail-yb0-f172.google.com Received: from mail-yb0-f172.google.com (HELO mail-yb0-f172.google.com) (209.85.213.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 11 Jul 2018 21:06:09 +0000 Received: by mail-yb0-f172.google.com with SMTP id k127-v6so10589456ybk.6; Wed, 11 Jul 2018 14:06:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:from:date:message-id:subject:to; bh=LT/HEKC0Vny6MddlfYBk82OAR4+byb1/Q2tEj96wLTM=; b=lqJvWQZJQCmPXPQH7/8N0mW1i3q0ZJl6aN97yiN+Go7+lp7daTO/kkVb9Tr4cIo4zo N7/zfGDxpS7VQQ9srKrzaLHZt0IKosJ5GU9LyFyrBBE3BgTM1w88IicDR9gz78XFXpRT XNtfKc19ifCL20BZf2GNsagPw6lup2D0lDBJG87h9NbKA3Uit0opA03EmyuTSvLNxSzB /sKS4GAqL18rXd3yo0t5+eqa/Qkb8VSVDu8dPuCYGmZ7txRzEY8vk/XOevM5STZo93Rp DaBrX8j2JypbnX/857TMHHCcdzOA49KyI5CFZ6H87rq4M+kSzsbARSJPjeUmRQGJpFIP yNEg== MIME-Version: 1.0 Sender: jaydub66@gmail.com Received: by 2002:a81:9a13:0:0:0:0:0 with HTTP; Wed, 11 Jul 2018 14:06:07 -0700 (PDT) From: Janus Weil Date: Wed, 11 Jul 2018 23:06:07 +0200 Message-ID: Subject: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions To: gfortran , gcc-patches 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 Janus Weil 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 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/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 262563) +++ gcc/fortran/resolve.c (working copy) @@ -3822,6 +3822,46 @@ 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_Wextra, + "Function %qs at %L might not be evaluated", + name, &f->where); + else + gfc_warning (OPT_Wextra, + "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 +3970,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; }