On 4/4/19 2:30 PM, Martin Sebor wrote:
On 4/4/19 10:50 AM, Jason Merrill wrote:
On 4/4/19 12:29 PM, Martin Sebor wrote:
On 4/4/19 8:57 AM, Jason Merrill wrote:
On 4/3/19 10:34 PM, Martin Sebor wrote:
On 4/1/19 11:27 AM, Jason Merrill wrote:
On 3/31/19 10:17 PM, Martin Sebor wrote:
To fix PR 89833, a P1 regression, the attached patch tries to
handle string literals as C++ 2a non-type template arguments
by treating them the same as brace enclosed initializer lists
(where the latter are handled correctly).  The solution makes
sure equivalent forms of array initializers such as for char[5]:

   "\0\1\2"
   "\0\1\2\0"
   { 0, 1, 2 }
   { 0, 1, 2, 0 }
   { 0, 1, 2, 0, 0 }

are treated as the same, both for overloading and mangling.
Ditto for the following equivalent forms:

   ""
   "\0"
   "\0\0\0\0"
   { }
   { 0 }
   { 0, 0, 0, 0, 0 }

and for these of struct { char a[5], b[5], c[5]; }:

   { {0,0,0,0,0}, {0,0,0,0}, {0,0,0} }
   { { 0 }, { } }
   { "" }

Since this is not handled correctly by the current code (see PR
89876 for a test case) the patch also fixes that.

I'm not at all confident the handling of classes with user-defined
constexpr ctors is 100% correct.  (I use triviality to constrain
the solution for strings but that was more of an off-the-cuff guess
than a carefully considered decision).

You could use TYPE_HAS_TRIVIAL_DFLT since we don't care about the triviality of other operations.

Done (I think).


I wouldn't worry about trying to omit user-defined constexpr ctors.

The g++.dg/abi/mangle71.C
test is all I've got in terms of verifying it works correctly.
I'm quite sure the C++ 2a testing could stand to be beefed up.

The patch passes x86_64-linux bootstrap and regression tests.
There are a few failures in check-c++-all tests that don't look
related to the changes (I see them with an unpatched GCC as well):

   g++.dg/spellcheck-macro-ordering-2.C
   g++.dg/cpp0x/auto52.C
   g++.dg/cpp1y/auto-neg1.C
   g++.dg/other/crash-9.C

You probably need to check zero_init_p to properly handle pointers to data members, where a null value is integer -1; given

struct A { int i; };

constexpr A::* pm = &A::i;
int A::* pma[] = { pm, pm };

we don't want to discard the initializers because they look like zeros, as then digest_init will add back -1s.

I added it but it wasn't doing the right thing.  It mangled { } and
{ 0 } differently:

   typedef int A::*pam_t;
   struct B { pam_t a[2]; };
   template <B> struct Y { };

   void f (Y<B{{ }}>) { }     // _Z1f1YIXtl1BtlA2_M1AiLS2_0EEEEE
   void f (Y<B{{ 0 }}>) { }   // _Z1f1YIXtl1BtlA2_M1AiLS2_0ELS2_0EEEEE

because the zeros didn't get removed.  They need to be mangled the same,
don't they?

I've added three tests to exercise this (array51.C, nontype-class16.C, and mangle72.C).  They pass without the zero_init_p() calls but some
fail with it (depending on where it's added).  Please check to see
that the tests really do exercise what they should.

If you think a zero_init_p() check really is needed I will need some
guidance where (a test case would be great).

Hmm, let me poke at this a bit.

Here's a test case showing that mangling null member pointers
as zero leads to conflicts:

   struct A { int i; };
   typedef int A::*pam_t;
   struct B { pam_t a[2]; };
   template <B> struct Y { };

   // both mangle as _Z2.31YIXtl1BtlA2_M1AiLS2_0ELS2_0EEEEE
   void f (Y<B{{ 0, 0 }}>) { }
   void g (Y<B{{ 0, &A::i }}>) { }

This happens both with trunk and with my patch.  They probably
need to mangle as negative one, don't you think?  (There's
a comment saying mangling them as zeros is intentional but not
why.)

Hmm.  If we leave out f, then g mangles as 0 and &A::i, which is fine.
Need to figure out why creating a B before the declaration of g breaks that.

Let me know if you want me to start looking into this or if you
are on it yourself.

Sure, go ahead.

The rationale for mangling as 0 would have been to reflect what you would write in the source: (int A::*)0 does give a null member pointer. But then we really can't use that representation 0 for anything else, we have to use a symbolic representation.   This patch doesn't need to fix that.

Okay.  It doesn't look like it would be hard to change but I'm
happy to leave it for later.

Feel free to fix it in a followup patch, along with investigating the above.

Attached is a patch that just simplifies the STRING_CST traversal.

OK.

Jason

Reply via email to