Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-10-03 Thread Marek Polacek
On Sat, Oct 03, 2015 at 09:07:29AM +0200, Andreas Schwab wrote: > Marek Polacek writes: > > > diff --git gcc/Makefile.in gcc/Makefile.in > > index c2df21d..d7caa76 100644 > > --- gcc/Makefile.in > > +++ gcc/Makefile.in > > @@ -217,6 +217,8 @@ libgcov-merge-tool.o-warn = -Wno-error > > gimple-mat

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-10-03 Thread Andreas Schwab
Marek Polacek writes: > diff --git gcc/Makefile.in gcc/Makefile.in > index c2df21d..d7caa76 100644 > --- gcc/Makefile.in > +++ gcc/Makefile.in > @@ -217,6 +217,8 @@ libgcov-merge-tool.o-warn = -Wno-error > gimple-match.o-warn = -Wno-unused > generic-match.o-warn = -Wno-unused > dfp.o-warn = -W

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)

2015-10-02 Thread Marek Polacek
On Fri, Oct 02, 2015 at 08:43:30AM -0700, H.J. Lu wrote: > On Fri, Oct 2, 2015 at 3:23 AM, Marek Polacek wrote: > > On Wed, Sep 30, 2015 at 12:45:35PM -0600, Jeff Law wrote: > >> On 09/30/2015 09:47 AM, Joseph Myers wrote: > >> >The C front-end changes are OK. > >> The rest are OK as well. > > > >

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)

2015-10-02 Thread H.J. Lu
On Fri, Oct 2, 2015 at 3:23 AM, Marek Polacek wrote: > On Wed, Sep 30, 2015 at 12:45:35PM -0600, Jeff Law wrote: >> On 09/30/2015 09:47 AM, Joseph Myers wrote: >> >The C front-end changes are OK. >> The rest are OK as well. > > Thanks Jeff & Joseph. > > I'm going to apply the patch soon; should it

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)

2015-10-02 Thread Marek Polacek
On Wed, Sep 30, 2015 at 12:45:35PM -0600, Jeff Law wrote: > On 09/30/2015 09:47 AM, Joseph Myers wrote: > >The C front-end changes are OK. > The rest are OK as well. Thanks Jeff & Joseph. I'm going to apply the patch soon; should it draw the ire of users, I'll move the option to -Wextra.

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)

2015-09-30 Thread Jeff Law
On 09/30/2015 09:47 AM, Joseph Myers wrote: The C front-end changes are OK. The rest are OK as well. jeff

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)

2015-09-30 Thread Joseph Myers
The C front-end changes are OK. -- Joseph S. Myers jos...@codesourcery.com

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)

2015-09-30 Thread Marek Polacek
Ping. On Fri, Sep 18, 2015 at 03:44:34PM +0200, Marek Polacek wrote: > On Fri, Sep 18, 2015 at 12:06:06PM +0200, Marek Polacek wrote: > > > Since we don't know bar's side-effects we must assume they change > > > the value of a and so we must avoid diagnosing the third if. > > > > Ok, I'm convince

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-23 Thread Marek Polacek
On Wed, Sep 23, 2015 at 12:21:35PM -0600, Jeff Law wrote: > On 09/23/2015 10:32 AM, Marek Polacek wrote: > >On Tue, Sep 22, 2015 at 03:33:34PM -0600, Martin Sebor wrote: > >>It's fine by me (for whatever it's worth). > > > >Thanks. Let's wait if Jason/Joseph or anyone else wants to chime in. > > >

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-23 Thread Jeff Law
On 09/23/2015 10:32 AM, Marek Polacek wrote: On Tue, Sep 22, 2015 at 03:33:34PM -0600, Martin Sebor wrote: It's fine by me (for whatever it's worth). Thanks. Let's wait if Jason/Joseph or anyone else wants to chime in. Btw., if you're unhappy about having to wipe out the whole chain after e

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-23 Thread Marek Polacek
On Tue, Sep 22, 2015 at 03:33:34PM -0600, Martin Sebor wrote: > It's fine by me (for whatever it's worth). Thanks. Let's wait if Jason/Joseph or anyone else wants to chime in. > Btw., if you're unhappy about having to wipe out the whole chain > after every side-effect it occurred to me that it

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-22 Thread Martin Sebor
On 09/22/2015 06:26 AM, Marek Polacek wrote: On Mon, Sep 21, 2015 at 07:06:01PM +0200, Marek Polacek wrote: I realized that current patch has a minor deficiency: it will start a chain even in case the first condition has a side-effect thus the chain should be invalid. I'll fix this problem soon

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-22 Thread Marek Polacek
On Mon, Sep 21, 2015 at 07:06:01PM +0200, Marek Polacek wrote: > I realized that current patch has a minor deficiency: it will start > a chain even in case the first condition has a side-effect thus the > chain should be invalid. I'll fix this problem soon. I changed my mind, the above mean we'll

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-21 Thread Marek Polacek
On Fri, Sep 18, 2015 at 10:45:33AM -0600, Martin Sebor wrote: > >Done in the below. This version actually bootstraps, because I've added > >-Wno-duplicated-cond for insn-dfatab.o and insn-latencytab.o (don't know > >how to fix these) + I've tweaked a condition in genemit.c. The problem > >here is

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-21 Thread Marek Polacek
On Fri, Sep 18, 2015 at 08:16:52PM +0200, Manuel López-Ibáñez wrote: > On 18/09/15 18:45, Martin Sebor wrote: > >but it makes me wonder how common this pattern is in portable > >code and whether adding workarounds for it is the right solution > >(or if it might prompt people to disable the warning,

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-18 Thread Manuel López-Ibáñez
On 18/09/15 18:45, Martin Sebor wrote: but it makes me wonder how common this pattern is in portable code and whether adding workarounds for it is the right solution (or if it might prompt people to disable the warning, which would be a shame). Perhaps if we are going to warn, we could look for

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-18 Thread Martin Sebor
Done in the below. This version actually bootstraps, because I've added -Wno-duplicated-cond for insn-dfatab.o and insn-latencytab.o (don't know how to fix these) + I've tweaked a condition in genemit.c. The problem here is that we have if (INTVAL (x) == 0) printf ("const0_rtx")

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take

2015-09-18 Thread Marek Polacek
2) Reply-To: In-Reply-To: <20150918100606.gf27...@redhat.com> On Fri, Sep 18, 2015 at 12:06:06PM +0200, Marek Polacek wrote: > > Since we don't know bar's side-effects we must assume they change > > the value of a and so we must avoid diagnosing the third if. > > Ok, I'm convinced now. We have

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249)

2015-09-18 Thread Marek Polacek
On Thu, Sep 17, 2015 at 10:37:40AM -0600, Martin Sebor wrote: > >>The patch currently issues a false positive for the test case > >>below. I suspect the chain might need to be cleared after each > >>condition that involves a side-effect. > >> > >> int foo (int a) > >> { > >> if (a) return 1

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249)

2015-09-17 Thread Martin Sebor
The patch currently issues a false positive for the test case below. I suspect the chain might need to be cleared after each condition that involves a side-effect. int foo (int a) { if (a) return 1; else if (++a) return 2; else if (a) return 3; return 0; } But the last branch

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249)

2015-09-17 Thread Jeff Law
On 09/17/2015 10:05 AM, Marek Polacek wrote: The patch doesn't diagnose some more involved cases like the one below: if (i < 0) return 1; else if (!(i > 0 || i == 0)) return 2; even though it does diagnose some other such cases, including: if (i < 0) return 1; else if (!(i >= 0)) retur

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249)

2015-09-17 Thread Marek Polacek
On Wed, Sep 16, 2015 at 02:59:12PM -0600, Martin Sebor wrote: > On 09/16/2015 09:59 AM, Marek Polacek wrote: > >This patch implements a new warning, -Wduplicated-cond. It warns for > >code such as > > > > if (x) > > // ... > > else if (x) > > // ... > > As usual, I like this improveme

Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249)

2015-09-16 Thread Martin Sebor
On 09/16/2015 09:59 AM, Marek Polacek wrote: This patch implements a new warning, -Wduplicated-cond. It warns for code such as if (x) // ... else if (x) // ... As usual, I like this improvement. I think it's worth extending to conditional expressions (e.g., x ? f() : x ? g() :

[C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249)

2015-09-16 Thread Marek Polacek
This patch implements a new warning, -Wduplicated-cond. It warns for code such as if (x) // ... else if (x) // ... This happened in the GCC codebase as well: . The approach I took for this was to create vectors of conditions