Patchwork [FORTRAN] Quash two -Wlogical-not-parentheses warnings

login
register
mail settings
Submitter Marek Polacek
Date Sept. 1, 2014, 1:23 p.m.
Message ID <20140901132318.GJ2264@redhat.com>
Download mbox | patch
Permalink /patch/384828/
State New
Headers show

Comments

Marek Polacek - Sept. 1, 2014, 1:23 p.m.
These two issues are the last ones blocking moving -Wlogical-not-parentheses
to -Wall.  I tried to fix them, but my attempts failed, so I opened
PR62270.  I hope we could for now go with the following; it only quiets
the warning, but otherwise does not change the code.  Hopefully someone
familiar with the Fortran codebase will take a look at that PR...

2014-09-01  Marek Polacek  <polacek@redhat.com>

	* interface.c (compare_parameter): Wrap LHS of a comparison in parens.
	* trans-expr.c (gfc_conv_procedure_call): Likewise.


	Marek
Steven Bosscher - Sept. 1, 2014, 1:28 p.m.
On Mon, Sep 1, 2014 at 3:23 PM, Marek Polacek wrote:

> diff --git gcc/fortran/interface.c gcc/fortran/interface.c
> index b210d18..68d8545 100644
> --- gcc/fortran/interface.c
> +++ gcc/fortran/interface.c
> @@ -2014,7 +2014,7 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
>    if (formal->ts.type == BT_CLASS && formal->attr.class_ok
>        && actual->expr_type != EXPR_NULL
>        && ((CLASS_DATA (formal)->attr.class_pointer
> -          && !formal->attr.intent == INTENT_IN)
> +          && (!formal->attr.intent) == INTENT_IN)
>            || CLASS_DATA (formal)->attr.allocatable))
>      {
>        if (actual->ts.type != BT_CLASS)

This is certainly not OK, intent is a tri-state.


> diff --git gcc/fortran/trans-expr.c gcc/fortran/trans-expr.c
> index f2ed474..6592c7e 100644
> --- gcc/fortran/trans-expr.c
> +++ gcc/fortran/trans-expr.c
> @@ -4589,7 +4589,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
>                       && e->expr_type == EXPR_VARIABLE
>                       && (!e->ref
>                           || (e->ref->type == REF_ARRAY
> -                             && !e->ref->u.ar.type != AR_FULL))
> +                             && (!e->ref->u.ar.type) != AR_FULL))
>                       && e->symtree->n.sym->attr.optional)
>                     {
>                       tmp = fold_build3_loc (input_location, COND_EXPR,

Also not OK.

You probably want to wrap the (in)equality tests in parenthesis.

Ciao!
Steven
Marek Polacek - Sept. 1, 2014, 1:45 p.m.
On Mon, Sep 01, 2014 at 03:28:42PM +0200, Steven Bosscher wrote:
> On Mon, Sep 1, 2014 at 3:23 PM, Marek Polacek wrote:
> 
> > diff --git gcc/fortran/interface.c gcc/fortran/interface.c
> > index b210d18..68d8545 100644
> > --- gcc/fortran/interface.c
> > +++ gcc/fortran/interface.c
> > @@ -2014,7 +2014,7 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
> >    if (formal->ts.type == BT_CLASS && formal->attr.class_ok
> >        && actual->expr_type != EXPR_NULL
> >        && ((CLASS_DATA (formal)->attr.class_pointer
> > -          && !formal->attr.intent == INTENT_IN)
> > +          && (!formal->attr.intent) == INTENT_IN)
> >            || CLASS_DATA (formal)->attr.allocatable))
> >      {
> >        if (actual->ts.type != BT_CLASS)
> 
> This is certainly not OK, intent is a tri-state.
> 
> 
> > diff --git gcc/fortran/trans-expr.c gcc/fortran/trans-expr.c
> > index f2ed474..6592c7e 100644
> > --- gcc/fortran/trans-expr.c
> > +++ gcc/fortran/trans-expr.c
> > @@ -4589,7 +4589,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
> >                       && e->expr_type == EXPR_VARIABLE
> >                       && (!e->ref
> >                           || (e->ref->type == REF_ARRAY
> > -                             && !e->ref->u.ar.type != AR_FULL))
> > +                             && (!e->ref->u.ar.type) != AR_FULL))
> >                       && e->symtree->n.sym->attr.optional)
> >                     {
> >                       tmp = fold_build3_loc (input_location, COND_EXPR,
> 
> Also not OK.

Have you noticed that I'm not adding the !, only the parens?  The
code, as is, is highly suspicious, that's why we warn.  I'd strongly
prefer if we could apply a proper fix instead of this makeshift patch,
but that needs someone with Fortran knowledge; all the "obvious" fixes
regressed some tests.  That's why I filed PR62270.
 
> You probably want to wrap the (in)equality tests in parenthesis.

No, that doesn't suppress the warning.

	Marek

Patch

diff --git gcc/fortran/interface.c gcc/fortran/interface.c
index b210d18..68d8545 100644
--- gcc/fortran/interface.c
+++ gcc/fortran/interface.c
@@ -2014,7 +2014,7 @@  compare_parameter (gfc_symbol *formal, gfc_expr *actual,
   if (formal->ts.type == BT_CLASS && formal->attr.class_ok
       && actual->expr_type != EXPR_NULL
       && ((CLASS_DATA (formal)->attr.class_pointer
-	   && !formal->attr.intent == INTENT_IN)
+	   && (!formal->attr.intent) == INTENT_IN)
           || CLASS_DATA (formal)->attr.allocatable))
     {
       if (actual->ts.type != BT_CLASS)
diff --git gcc/fortran/trans-expr.c gcc/fortran/trans-expr.c
index f2ed474..6592c7e 100644
--- gcc/fortran/trans-expr.c
+++ gcc/fortran/trans-expr.c
@@ -4589,7 +4589,7 @@  gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 		      && e->expr_type == EXPR_VARIABLE
 		      && (!e->ref
 			  || (e->ref->type == REF_ARRAY
-			      && !e->ref->u.ar.type != AR_FULL))
+			      && (!e->ref->u.ar.type) != AR_FULL))
 		      && e->symtree->n.sym->attr.optional)
 		    {
 		      tmp = fold_build3_loc (input_location, COND_EXPR,