cp: fix location comparison in member_name_cmp

Hi,

After recent changes, the member_name_cmp qsort comparator can indicate
A < B < A (i.e. lacks anti-commutativity) for distinct TYPE_DECL nodes
that have the same source location.  If their order doesn't matter, the
comparator should return 0.

Invoking qsort with improper comparator at best makes the output unpredictable
(pedantically it invokes undefined behavior); this trips qsort checking I'm
preparing to resend.

(it also seems that this and the following comparators use less/greater
pointer comparison, but unless those pointers point into the same array
that invokes undefined behavior too, although benign in practice)

Bootstrapped and regtested on x86-64, OK to apply?

Thanks.
Alexander

	* name-lookup.c (member_name_cmp): Return 0 if locations compare equal.

On 09/19/2017 07:06 AM, Alexander Monakov wrote:
> Hi,
> 
> After recent changes, the member_name_cmp qsort comparator can indicate
> A < B < A (i.e. lacks anti-commutativity) for distinct TYPE_DECL nodes
> that have the same source location.  If their order doesn't matter, the
> comparator should return 0.

Where do we get such type decls?

nathan

On Tue, 19 Sep 2017, Nathan Sidwell wrote:

> On 09/19/2017 07:06 AM, Alexander Monakov wrote:
> > Hi,
> > 
> > After recent changes, the member_name_cmp qsort comparator can indicate
> > A < B < A (i.e. lacks anti-commutativity) for distinct TYPE_DECL nodes
> > that have the same source location.  If their order doesn't matter, the
> > comparator should return 0.
> 
> Where do we get such type decls?

The first instance that tripped qsort checking for me was in PCH generation.
I think by adding

  gcc_assert (DECL_SOURCE_LOCATION (a) != DECL_SOURCE_LOCATION (b));

at the appropriate point in this comparator you can see for yourself.

Thanks.
Alexander

On 09/19/2017 07:25 AM, Alexander Monakov wrote:
> On Tue, 19 Sep 2017, Nathan Sidwell wrote:
> 
>> On 09/19/2017 07:06 AM, Alexander Monakov wrote:
>>> Hi,
>>>
>>> After recent changes, the member_name_cmp qsort comparator can indicate
>>> A < B < A (i.e. lacks anti-commutativity) for distinct TYPE_DECL nodes
>>> that have the same source location.  If their order doesn't matter, the
>>> comparator should return 0.
>>
>> Where do we get such type decls?
> 
> The first instance that tripped qsort checking for me was in PCH generation.
> I think by adding

What source example generates such type decls?  That you've encountered 
this suggests one of the invariants I was presuming is not true.

nathan

On Tue, 19 Sep 2017, Nathan Sidwell wrote:
> > > > After recent changes, the member_name_cmp qsort comparator can indicate
> > > > A < B < A (i.e. lacks anti-commutativity) for distinct TYPE_DECL nodes
> > > > that have the same source location.  If their order doesn't matter, the
> > > > comparator should return 0.
> > > 
> > > Where do we get such type decls?
> > 
> > The first instance that tripped qsort checking for me was in PCH generation.
> > I think by adding
> 
> What source example generates such type decls?  That you've encountered this
> suggests one of the invariants I was presuming is not true.

You can use the gcc_assert mentioned in the previous email on GCC
bootstrap/regtest to find examples.  For me, the following example breaks (no
command line flags needed, just bare 'cc1plus t.i'):

struct
{
  int a, b, c, d;
  union
    {
      int e;
    };
} s;

1436      gcc_assert (DECL_SOURCE_LOCATION (a) != DECL_SOURCE_LOCATION (b));
(gdb) call debug_tree(a)
 <type_decl 0x7ffff19f68e8 ._1
    type <union_type 0x7ffff19f77e0 ._1 type_5 SI
        size <integer_cst 0x7ffff18c0120 constant 32>
        unit-size <integer_cst 0x7ffff18c0138 constant 4>
        align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff19f7348
        fields <field_decl 0x7ffff19f6980 e type <integer_type 0x7ffff18bd5e8 int>
            nonlocal decl_3 SI t.i:6:11 size <integer_cst 0x7ffff18c0120 32> unit-size <integer_cst 0x7ffff18c0138 4>
            align:32 warn_if_not_align:0 offset_align 128
            offset <integer_cst 0x7ffff18a0f00 constant 0>
            bit-offset <integer_cst 0x7ffff18a0f48 constant 0> context <union_type 0x7ffff19f7348 ._1> chain <type_decl 0x7ffff19f68e8 ._1>> context <record_type 0x7ffff19f7f18 ._0>
        full-name "union<unnamed struct>::<unnamed>"

        chain <type_decl 0x7ffff19f6850 ._1>>
    nonlocal decl_4 VOID t.i:5:5
    align:1 warn_if_not_align:0 context <union_type 0x7ffff19f7348 ._1>
    result <union_type 0x7ffff19f7348 ._1 type_5 SI size <integer_cst 0x7ffff18c0120 32> unit-size <integer_cst 0x7ffff18c0138 4>
        align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff19f7348 fields <field_decl 0x7ffff19f6980 e> context <record_type 0x7ffff19f7f18 ._0>
        full-name "union<unnamed struct>::<unnamed>"

        chain <type_decl 0x7ffff19f6850 ._1>>
   >
$1 = 10
(gdb) call debug_tree(b)
 <type_decl 0x7ffff19f6850 ._1
    type <union_type 0x7ffff19f7348 ._1 type_5 SI
        size <integer_cst 0x7ffff18c0120 constant 32>
        unit-size <integer_cst 0x7ffff18c0138 constant 4>
        align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff19f7348
        fields <field_decl 0x7ffff19f6980 e type <integer_type 0x7ffff18bd5e8 int>
            nonlocal decl_3 SI t.i:6:11 size <integer_cst 0x7ffff18c0120 32> unit-size <integer_cst 0x7ffff18c0138 4>
            align:32 warn_if_not_align:0 offset_align 128
            offset <integer_cst 0x7ffff18a0f00 constant 0>
            bit-offset <integer_cst 0x7ffff18a0f48 constant 0> context <union_type 0x7ffff19f7348 ._1> chain <type_decl 0x7ffff19f68e8 ._1>> context <record_type 0x7ffff19f7f18 ._0>
        full-name "union<unnamed struct>::<unnamed>"

        chain <type_decl 0x7ffff19f6850 ._1>>
    decl_2 decl_5 VOID t.i:5:5
    align:8 warn_if_not_align:0 context <record_type 0x7ffff19f7f18 ._0>>
$2 = 10

On 09/19/2017 06:07 AM, Alexander Monakov wrote:
> On Tue, 19 Sep 2017, Nathan Sidwell wrote:
>>>>> After recent changes, the member_name_cmp qsort comparator can indicate
>>>>> A < B < A (i.e. lacks anti-commutativity) for distinct TYPE_DECL nodes
>>>>> that have the same source location.  If their order doesn't matter, the
>>>>> comparator should return 0.
>>>>
>>>> Where do we get such type decls?
>>>
>>> The first instance that tripped qsort checking for me was in PCH generation.
>>> I think by adding
>>
>> What source example generates such type decls?  That you've encountered this
>> suggests one of the invariants I was presuming is not true.
> 
> You can use the gcc_assert mentioned in the previous email on GCC
> bootstrap/regtest to find examples.  For me, the following example breaks (no
> command line flags needed, just bare 'cc1plus t.i'):
> 
> struct
> {
>    int a, b, c, d;
>    union
>      {
>        int e;
>      };
> } s;
> 
> 1436      gcc_assert (DECL_SOURCE_LOCATION (a) != DECL_SOURCE_LOCATION (b));

Where are these two decls for the same type being generated?  The bug 
seems to be there.

nathan

On Wed, 20 Sep 2017, Nathan Sidwell wrote:
> > You can use the gcc_assert mentioned in the previous email on GCC
> > bootstrap/regtest to find examples.  For me, the following example breaks
> > (no
> > command line flags needed, just bare 'cc1plus t.i'):
> > 
> > struct
> > {
> >    int a, b, c, d;
> >    union
> >      {
> >        int e;
> >      };
> > } s;
> > 
> > 1436      gcc_assert (DECL_SOURCE_LOCATION (a) != DECL_SOURCE_LOCATION (b));
> 
> Where are these two decls for the same type being generated?  The bug seems to
> be there.

Don't ask me, I'm not a C++ frontend maintainer ;)

The patch in the opening message of this thread doesn't affect functionality
and is required to unblock qsort checking work. Can I install it on trunk?
If anyone wants to investigate how such decls appear in the first place, that
can be done independently of this patch.

Thanks.
Alexander