On Thu, Apr 4, 2019 at 11:30 AM Martin Sebor <mse...@gmail.com> 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. > > > > > 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. >
This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89980 -- H.J.