[PATCH] handle strings as template arguments (PR 47488, 89833, 89876)

Martin Sebor msebor@gmail.com
Thu Apr 4 18:30:00 GMT 2019 

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.

> 
> 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.

Attached is a patch that just simplifies the STRING_CST traversal.
I suspect I got confused into thinking that TREE_STRING_LENGTH is
the length of the string literal as opposed to its size, even
after all this time working with GCC strings. I wonder if
the macro could be renamed to something like TREE_STRING_SIZE.

I also added more tests for member pointers, including those to
member functions and managed to trigger another ICE in the process:

   struct A { void (A::*p)(); };
   template <A> struct X { };
   X<A{ 0 }> x;   // ICE in find_substitution during mangling

I opened bug 89974 for it even though the patch happens to avoid
it because it treats a null member function pointer as an empty
initializer list (i.e., { }).  The added test exposes the mangling
problems with pointers to data members.  I xfailed them.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-89833.diff
Type: text/x-patch
Size: 40258 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20190404/cdb233e9/attachment.bin>

More information about the Gcc-patches mailing list