[Bug c/60759] New: -Wlogical-op should perhaps warn about two-way implicit conversions

[Bug c/60759] New: -Wlogical-op should perhaps warn about two-way implicit conversions

[giuliano.procida at googlemail dot com]

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

            Bug ID: 60759
           Summary: -Wlogical-op should perhaps warn about two-way
                    implicit conversions
           Product: gcc
           Version: 4.6.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: giuliano.procida at googlemail dot com

Expressions of the form "a && b" or "a || b" that involve both implicit
bool->integer and implicit integer->bool coercion, represent a probable misuse
of a logical operator.

These should perhaps generate warnings, particularly for C++ code:

int foo(int x);

int a = foo(1) || foo(2);  // Confusion with Perl, Python behaviour,
int one = 2 && 3;          // or perhaps bitwise operation was intended.

These should not probably generate warnings:

while (foo(1) || foo(2));
int zero = (1 != 2) && (3 == 4);

Example:

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.6/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro
4.6.3-1ubuntu5' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.6 --enable-shared --enable-linker-build-id
--with-system-zlib --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6
--libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object
--enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686
--with-tune=generic --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) 

$ cat foo.c
static int x = 2 || 3;
int main() { return x; }

$ gcc -Wall -Wextra -Wlogical-op foo.c
$ g++ -Wall -Wextra -Wlogical-op foo.c
[no warnings]

[Bug c/60759] -Wlogical-op should perhaps warn about two-way implicit conversions

[mpolacek at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpolacek at gcc dot gnu.org

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
We could perhaps implement clang's -Wconstant-logical-operand, but I must say I
don't like it very much.

[Bug c/60759] -Wlogical-op should perhaps warn about two-way implicit conversions

[manu at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org

--- Comment #2 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #1)
> We could perhaps implement clang's -Wconstant-logical-operand, but I must
> say I don't like it very much.

So work on something that you like, there so much to do in GCC, that you can be
choosy ;-) In particular, Wlogical-op is not enabled by Wextra because of some
known bugs. If those were fixed it would be extremely more useful than adding
more stuff to it.

What does Wconstant-logical-operand do?
>From gcc-bugs-return-448298-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Apr 04 15:03:20 2014
Return-Path: <gcc-bugs-return-448298-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 29127 invoked by alias); 4 Apr 2014 15:03:20 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 28512 invoked by uid 55); 4 Apr 2014 15:03:11 -0000
From: "wschmidt at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/60203] Support long double/_Decimal128 direct move on power8
Date: Fri, 04 Apr 2014 15:03:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: wschmidt at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: meissner at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-60203-4-S9Fn38wlNE@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-60203-4@http.gcc.gnu.org/bugzilla/>
References: <bug-60203-4@http.gcc.gnu.org/bugzilla/>
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-04/txt/msg00318.txt.bz2
Content-length: 2359

http://gcc.gnu.org/bugzilla/show_bug.cgi?id`203

--- Comment #8 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Author: wschmidt
Date: Fri Apr  4 15:02:38 2014
New Revision: 209111

URL: http://gcc.gnu.org/viewcvs?rev 9111&root=gcc&view=rev
Log:
[gcc]

2014-04-04  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

    Backport from mainline r207699.
    2014-02-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

    PR target/60137
    * config/rs6000/rs6000.md (128-bit GPR splitter): Add a splitter
    for VSX/Altivec vectors that land in GPR registers.

    Backport from mainline r207808.
    2014-02-15  Michael Meissner  <meissner@linux.vnet.ibm.com>

    PR target/60203
    * config/rs6000/rs6000.md (rreg): Add TFmode, TDmode constraints.
    (mov<mode>_internal, TFmode/TDmode): Split TFmode/TDmode moves
    into 64-bit and 32-bit moves.  On 64-bit moves, add support for
    using direct move instructions on ISA 2.07.  Also adjust
    instruction length for 64-bit.
    (mov<mode>_64bit, TFmode/TDmode): Likewise.
    (mov<mode>_32bit, TFmode/TDmode): Likewise.

    Backport from mainline r207868.
    2014-02-18  Michael Meissner  <meissner@linux.vnet.ibm.com>

    PR target/60203
    * config/rs6000/rs6000.md (mov<mode>_64bit, TF/TDmode moves):
    Split 64-bit moves into 2 patterns.  Do not allow the use of
    direct move for TDmode in little endian, since the decimal value
    has little endian bytes within a word, but the 64-bit pieces are
    ordered in a big endian fashion, and normal subreg's of TDmode are
    not allowed.
    (mov<mode>_64bit_dm): Likewise.
    (movtd_64bit_nodm): Likewise.

[gcc/testsuite]

2014-04-04  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

    Backport from mainline r207699.
    2014-02-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

    PR target/60137
    * gcc.target/powerpc/pr60137.c: New file.

    Backport from mainline r207808.
    2014-02-15  Michael Meissner  <meissner@linux.vnet.ibm.com>

    PR target/60203
    * gcc.target/powerpc/pr60203.c: New testsuite.


Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/powerpc/pr60137.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/powerpc/pr60203.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/rs6000/rs6000.md
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

[Bug c/60759] -Wlogical-op should perhaps warn about two-way implicit conversions

[giuliano.procida at googlemail dot com]

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

--- Comment #3 from Giuliano Procida <giuliano.procida at googlemail dot com> ---
I believe the clang warning is:

foo.c:1:18: warning: use of logical '||' with constant operand
[-Wconstant-logical-operand]
static int x = 2 || 3;
                 ^  ~
foo.c:1:18: note: use '|' for a bitwise operation
static int x = 2 || 3;
                 ^~
                 |
1 warning generated.

However, changing the code to:

int two() { return 2; }
int three() { return 3; }
int main() {
  int x = two() || three();
  return x;
}

results in no warnings from either compiler (in either language).

[Bug c/60759] improve -Wlogical-op

[vincent-gcc at vinc17 dot net]

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

Vincent Lefèvre <vincent-gcc at vinc17 dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vincent-gcc at vinc17 dot net

--- Comment #5 from Vincent Lefèvre <vincent-gcc at vinc17 dot net> ---
Clang's -Wconstant-logical-operand is useful: it found a bug in new MPFR code,
which had

  rnd_mode == MPFR_RNDN || MPFR_RNDF

instead of

  rnd_mode == MPFR_RNDN || rnd_mode == MPFR_RNDF

FYI, GCC's -Wlogical-op doesn't warn (20201002 snapshot).

[Bug c/60759] improve -Wlogical-op

[lopezibanez at gmail dot com]

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

--- Comment #6 from Manuel López-Ibáñez <lopezibanez at gmail dot com> ---
I believe this is on purpose to avoid too much noise.  The warning in GCC
needs to be smarter about types and macros and avoid early folding.

[Bug c/60759] improve -Wlogical-op

[vincent-gcc at vinc17 dot net]

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

--- Comment #7 from Vincent Lefèvre <vincent-gcc at vinc17 dot net> ---
(In reply to Manuel López-Ibáñez from comment #6)
> I believe this is on purpose to avoid too much noise.  The warning in GCC
> needs to be smarter about types and macros and avoid early folding.

Well, for the case constant-logical-operand, the warning on X || Y should be on
"true" constants X and Y (which is stricter than what __builtin_constant_p
regards as constants). I don't think that there would be much noise in this
case, or this could be a separate macro like clang's
-Wconstant-logical-operand, thus which can easily be enabled/disabled.