Implement -Wduplicated-branches (PR c/64279) (v3)

Eric Gallager egall@gwmail.gwu.edu
Wed Jan 4 06:49:00 GMT 2017


On 11/3/16, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Nov 03, 2016 at 09:27:55AM -0400, Jason Merrill wrote:
>> On Thu, Nov 3, 2016 at 7:24 AM, Marek Polacek <polacek@redhat.com> wrote:
>> > On Tue, Nov 01, 2016 at 02:53:58PM +0100, Jakub Jelinek wrote:
>> >> On Tue, Nov 01, 2016 at 09:41:20AM -0400, Jason Merrill wrote:
>> >> > On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek <polacek@redhat.com>
>> >> > wrote:
>> >> > > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote:
>> >> > >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote:
>> >> > >> > I found a problem with this patch--we can't call
>> >> > >> > do_warn_duplicated_branches in
>> >> > >> > build_conditional_expr, because that way the C++-specific codes
>> >> > >> > might leak into
>> >> > >> > the hasher.  Instead, I should use operand_equal_p, I think.
>> >> > >> > Let me rework
>> >> > >> > that part of the patch.
>> >> >
>> >> > Hmm, is there a reason not to use operand_equal_p for
>> >> > do_warn_duplicated_branches as well?  I'm concerned about hash
>> >> > collisions leading to false positives.
>> >>
>> >> If the hashing function is iterative_hash_expr / inchash::add_expr,
>> >> then
>> >> that is supposed to pair together with operand_equal_p, we even have
>> >> checking verification of that.
>>
>> Yes, but there could still be hash collisions; we can't guarantee that
>> everything that compares unequal also hashes unequal.
>
> Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0)
> or so (the exact last operand needs to be figured out).
> OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean the
> same thing.  0 is a tiny bit better, but still it will give up on e.g. pure
> and other calls.  OEP_PURE_SAME is tiny bit better than that, but still
> calls with the same arguments to the same function will not be considered
> equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc.
> So maybe we need another OEP_* mode for this.
>
> 	Jakub
>

Pinging this conversation for the new year. Any chances of
-Wduplicated-branches making it in in time for GCC 7?

Thanks,
Eric



More information about the Gcc-patches mailing list