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

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


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

> 
>>>> +      unsigned n = TREE_STRING_LENGTH (value);
>>>> +      const char *str = TREE_STRING_POINTER (value);
>>>>
>>>> +      /* Count the number of trailing nuls and subtract them from
>>>> +         STRSIZE because they don't need to be mangled.  */
>>>> +      tree strsizenode = TYPE_SIZE_UNIT (TREE_TYPE (value));
>>>> +      unsigned strsize = tree_to_uhwi (strsizenode);
>>>> +      if (strsize > n)
>>>> +        strsize = n;
>>>
>>> Why not just use TREE_STRING_LENGTH?
>>
>> The length reflects the size of the string literal, including
>> any trailing nuls (like in "x\0y\0\0").  We only want to mangle
>> the leading part up to (but not including) the first trailing
>> nul.
> 
> Yes, I meant why look at TYPE_SIZE_UNIT at all?  TREE_STRING_LENGTH 
> seems like the right starting value of strsize.

Ah.  Let me see about changing that.

Martin



More information about the Gcc-patches mailing list