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

[Jason Merrill]

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.

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

Jason