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.
