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

[Marek Polacek <polacek at redhat dot com>]

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; else if (++a) return 2; else if (a) return 3;
> >>     return 0;
> >>   }
> >
> >But the last branch here can never be reached, right?  If a == 0, foo
> >returns 2, otherwise it just returns 1.  So I think we should diagnose
> >this.
> 
> It probably wasn't the best example. The general issue here is
> that the second condition has a side-effect that can change (in
> this case clearly does) the value of the expression.
> 
> Here's a better example:
> 
>     int a;
> 
>     int bar (void) { a = 1; return 0; }
> 
>     int foo (void) {
>         if (a) return 1;
>         else if (foo ()) return 2;
>         else if (a) return 3;
>         return 0;
>     }
> 
> 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 something similar in the codebase:
libsupc++/eh_catch.cc has

  int count = header->handlerCount;
  if (count < 0)
    {   
      // This exception was rethrown.  Decrement the (inverted) catch
      // count and remove it from the chain when it reaches zero.
      if (++count == 0)
        globals->caughtExceptions = header->nextException;
    }   
  else if (--count == 0)
    {   
      // Handling for this exception is complete.  Destroy the object.
      globals->caughtExceptions = header->nextException;
      _Unwind_DeleteException (&header->unwindHeader);
      return;
    }   
  else if (count < 0)
    // A bug in the exception handling library or compiler.
    std::terminate ();

Here all arms are reachable.  I guess I need to kill the chain of conditions
when we find something with side-effects, exactly as you suggested.

Again, thanks for your comments.

	Marek