From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16646 invoked by alias); 2 Sep 2014 07:52:06 -0000 Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-bugs-owner@gcc.gnu.org Received: (qmail 16179 invoked by uid 48); 2 Sep 2014 07:52:00 -0000 From: "burnus at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug fortran/62270] -Wlogical-not-parentheses warnings Date: Tue, 02 Sep 2014 07:52:00 -0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: fortran X-Bugzilla-Version: 5.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: burnus at gcc dot gnu.org X-Bugzilla-Status: NEW X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-SW-Source: 2014-09/txt/msg00399.txt.bz2 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62270 --- Comment #3 from Tobias Burnus --- (In reply to Marek Polacek from comment #0) > -Wlogical-not-parentheses detected these two issues: I really missed such a warning before; Coverity also detected such issues, but it has way too much output to dig through all. > interface.c:compare_parameter > 2013 /* F2008, 12.5.2.5; IR F08/0073. */ > 2014 if (formal->ts.type == BT_CLASS && formal->attr.class_ok > 2015 && actual->expr_type != EXPR_NULL > 2016 && ((CLASS_DATA (formal)->attr.class_pointer > 2017 && !formal->attr.intent == INTENT_IN) Here, it really should be: Print an error when passing a nonpolymophic actual argument to a polymorphic dummy/formal argument - to avoid modification of the run-time type. But as that's not possible with INTENT_IN, one can permit it in that case. Thus, the obvious patch should be correct: --- a/gcc/fortran/interface.c +++ b/gcc/fortran/interface.c @@ -2016,3 +2016,3 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, && ((CLASS_DATA (formal)->attr.class_pointer - && !formal->attr.intent == INTENT_IN) + && formal->attr.intent != INTENT_IN) || CLASS_DATA (formal)->attr.allocatable)) > trans-expr.c:gfc_conv_procedure_call > 4445 if (fsym->attr.optional > 4446 && e->expr_type == EXPR_VARIABLE > 4447 && (!e->ref > 4448 || (e->ref->type == REF_ARRAY > 4449 && !e->ref->u.ar.type != AR_FULL)) In the surrounding code block, one inserts a deallocation call to the data component of a array descriptor ("free (var->data);") - but if the argument can be absent ("optional" attribute), one has var == NULL. Thus, one needs to add an extra check before dereferencing. But that is only needed if the users don't add array/subcomponent references themselves (e.g."var(:)%subcomp"). Thus, the check is only needed if e->ref == NULL - or one has an (automaically added) reference to the full array. Again, the obvious patch should be correct: --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -4592 +4592 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, - && !e->ref->u.ar.type != AR_FULL)) + && e->ref->u.ar.type != AR_FULL)) Side remark: I wonder whether BT_CLASS needs some extra care. > The first one should likely be "&& formal->attr.intent != INTENT_IN)", but > that regressed gfortran.dg/pointer_intent_7.f90 test. The problem is that one has two different issues - and now one earlier check triggers. For call bar3p (b) there are two related issues: One passes a pointer-intent(in) variable to a pointer-intent(inout) dummy-argument variable. And passes a nonpolymorphic pointer to a polymorphic dummy-argument variable w/o intent(in). Seemingly, the second issue is checked before in the code - and didn't trigger. (Which was a bug.) Thus, simply change the dg-error message. [Depending what the user intended, one or the other message is more helpful - hence, it doesn't matter which one is shown.] I won't be able to package and test this code before this evening; feel free to package, regtest and submit it yourself. (In reply to Thomas Koenig from comment #2) > A longer version would be [...] Is this what is intended here? Well, the code is bogus - for obvious reasons. The problem was more what is correct and makes code wise sense - re-writing the existing condition doesn't help.