[Bug c/53131] New: -Wlogical-op: ready for prime time in -Wall ?

[Bug c/53131] New: -Wlogical-op: ready for prime time in -Wall ?

[dcb314 at hotmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

             Bug #: 53131
           Summary: -Wlogical-op: ready for prime time in -Wall ?
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: dcb314@hotmail.com


I am somewhat surprised that -Wlogical-op isn't part of either -Wall
or possibly -Wextra.

I've just tried -Wlogical-op out on a build of a recent Linux kernel
and 90 warnings are produced, so there is some evidence that
it would be useful.

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
                 CC|                            |manu at gcc dot gnu.org
         Depends on|                            |43772

--- Comment #1 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-26 21:58:33 UTC ---
(In reply to comment #0)
> I am somewhat surprised that -Wlogical-op isn't part of either -Wall
> or possibly -Wextra.

It could be in -Wextra for GCC 4.8.

> I've just tried -Wlogical-op out on a build of a recent Linux kernel
> and 90 warnings are produced, so there is some evidence that
> it would be useful.

Do the warnings indicate bugs or not?

There is also PR43772, which would be nice to fix before enabling this in -Wall
or -Wextra. However, I don't even have any idea why we are warning there.

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[dcb314 at hotmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

--- Comment #2 from dcb <dcb314 at hotmail dot com> 2012-04-28 07:29:17 UTC ---
(In reply to comment #1)
> (In reply to comment #0)
> > I am somewhat surprised that -Wlogical-op isn't part of either -Wall
> > or possibly -Wextra.
> 
> It could be in -Wextra for GCC 4.8.

Righto. If it works well there, it can be promoted to -Wall later.

> > I've just tried -Wlogical-op out on a build of a recent Linux kernel
> > and 90 warnings are produced, so there is some evidence that
> > it would be useful.
> 
> Do the warnings indicate bugs or not?

Yes. I checked the first ten.

> There is also PR43772, which would be nice to fix before enabling this in -Wall
> or -Wextra. However, I don't even have any idea why we are warning there.

Agreed, although I wouldn't regard one broken corner case as a show stopper.

In general the warning does find bugs, even in heavily worked on code
like the Linux kernel. It finds nothing in the source code of gcc, however,
so no rework costs there.

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

--- Comment #3 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-28 10:37:04 UTC ---
(In reply to comment #2)
> > 
> > Do the warnings indicate bugs or not?
> 
> Yes. I checked the first ten.

And what do the kernel people say? Do they want Wlogical-op enabled?

> > There is also PR43772, which would be nice to fix before enabling this in -Wall
> > or -Wextra. However, I don't even have any idea why we are warning there.
> 
> Agreed, although I wouldn't regard one broken corner case as a show stopper.
> 

Unfortunately, this corner case triggers in Emacs, so I guess it is not that
uncommon. My intention is to try to fix it before touching the status of
-Wlogical-op. This will make easier to convince GCC maintainers. However, I
have a long list of other things I would like to fix firstm and very little
free time, so if you (or anyone else) managed to find out why that bug
triggers, it would help already a lot.

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

Marc Glisse <marc.glisse at normalesup dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marc.glisse at normalesup
                   |                            |dot org

--- Comment #4 from Marc Glisse <marc.glisse at normalesup dot org> 2012-04-28 12:05:20 UTC ---
(In reply to comment #2)
> > Do the warnings indicate bugs or not?
> Yes. I checked the first ten.

Could you give a sample? -Wlogical-op merges 2 unrelated warnings:
*) x && 2 (you would expect a boolean, not 2, so maybe x&2 was meant)
*) x<0 && x>0 (not so likely to happen) or x>=-5 || x<2 (always true)

and it is not clear which one you are most interested in.

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[dcb314 at hotmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

--- Comment #5 from dcb <dcb314 at hotmail dot com> 2012-04-28 12:17:57 UTC ---
(In reply to comment #4)
> Could you give a sample? -Wlogical-op merges 2 unrelated warnings:
> *) x && 2 (you would expect a boolean, not 2, so maybe x&2 was meant)
> *) x<0 && x>0 (not so likely to happen) or x>=-5 || x<2 (always true)
> 
> and it is not clear which one you are most interested in.

#2 is the one I am interested in.

I only discovered the other warning by accident, after 8 
of the 10 warnings I checked closely.

I used the Linux kernel as an example. I'd probably need to analyse
more cases, in other packages too, to be sure that most of what
the warning finds is #2.

It seems a pretty small warning, but I guess #1 and #2 could
be split up, if that helps get #2 in.

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

--- Comment #6 from Marc Glisse <marc.glisse at normalesup dot org> 2012-04-28 12:45:19 UTC ---
(In reply to comment #5)
> It seems a pretty small warning, but I guess #1 and #2 could
> be split up, if that helps get #2 in.

I think it is the opposite actually, #2 is more controversial than #1 (at least
until PR43772 is fixed).

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

--- Comment #7 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-05-07 13:42:10 UTC ---
In fact, the main show-stopper for adding -Wlogical-op to -Wextra (or -Wall) is
PR40172, which was the reason it was moved out of -Wextra in the first place. 

Someone would need to find a way to avoid warning in that case, for example
using the new tracking macro expansion infrastructure.

[Bug c/53131] -Wlogical-op: ready for prime time in -Wall ?

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53131

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |mpolacek at gcc dot gnu.org
         Resolution|---                         |DUPLICATE

--- Comment #9 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
So I think we can close this as a dup; PR61534 has detailed info what needs to
be fixed.

*** This bug has been marked as a duplicate of bug 61534 ***