| Message ID | 20210113233933.1356791-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: ICE when mangling operator name [PR98545] | expand |
On 1/13/21 6:39 PM, Marek Polacek wrote: > r11-6301 added some asserts in mangle.c, and now we trip over one of > them. In particular, it's the one asserting that we didn't get > IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. > > As this testcase shows, it's possible to get that, so turn the assert > into an if and write "on". That changes the mangling in the following > way: > > With this patch: > > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > G++10: > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > clang++/icc: > $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ > decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > I'm not sure why we differ in the "(*this)." part Is there a PR for that? > but at least the > suffix "onclspcvT__EEEDpS2_" is the same for all three compilers. So > I hope the following fix makes sense. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > gcc/cp/ChangeLog: > > PR c++/98545 > * mangle.c (write_expression): When the expression is a dependent name > and an operator name, write "on" before writing its name. > > gcc/testsuite/ChangeLog: > > PR c++/98545 > * g++.dg/abi/mangle76.C: New test. > --- > gcc/cp/mangle.c | 3 ++- > gcc/testsuite/g++.dg/abi/mangle76.C | 39 +++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C > > diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c > index 11eb8962d28..bb3c4b76d33 100644 > --- a/gcc/cp/mangle.c > +++ b/gcc/cp/mangle.c > @@ -3349,7 +3349,8 @@ write_expression (tree expr) > else if (dependent_name (expr)) > { > tree name = dependent_name (expr); > - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); > + if (IDENTIFIER_ANY_OP_P (name)) > + write_string ("on"); Any mangling change needs to handle different -fabi-versions; see the similar code in write_member_name. And why doesn't this go through write_member_name? > write_unqualified_id (name); > } > else > diff --git a/gcc/testsuite/g++.dg/abi/mangle76.C b/gcc/testsuite/g++.dg/abi/mangle76.C > new file mode 100644 > index 00000000000..0c2964cbecb > --- /dev/null > +++ b/gcc/testsuite/g++.dg/abi/mangle76.C > @@ -0,0 +1,39 @@ > +// PR c++/98545 > +// { dg-do compile { target c++11 } } > + > +class a { > +public: > + a(); > + template <typename b> a(b); > +}; > +template <class = double> using c = a; > +class f { > +protected: > + template <class d, class e> void operator()(d, double, e); > +}; > +class i : f { > +public: > + template <class... g> > + [[gnu::used]] auto h(g...) -> decltype(operator()(g()...)) {} > +// { dg-final { scan-assembler "_ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_" } } > +}; > +template <class> class C { > +public: > + template <class j> C(j); > + i k() const; > + int operator()() { > + int l = 10; > + c<> m, n; > + operator()(m, l, n); > + return 0; > + } > + int operator()(c<> &, c<> const &, c<> const &) const; > + template <class d, class e> void k(d m, double gamma, e o) const { > + k().h(m, gamma, o); > + } > +}; > +template <class r> int C<r>::operator()(c<> &, c<> const &, c<> const &) const { > + [&](c<> m, double gamma, c<> o) { k(m, gamma, o); }; > + return 0; > +} > +c<> p = C<double>(p)(); > > base-commit: 796ead19f85372e59217c9888db688a2fe11b54f >
On Tue, Jan 19, 2021 at 03:47:47PM -0500, Jason Merrill via Gcc-patches wrote: > On 1/13/21 6:39 PM, Marek Polacek wrote: > > r11-6301 added some asserts in mangle.c, and now we trip over one of > > them. In particular, it's the one asserting that we didn't get > > IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. > > > > As this testcase shows, it's possible to get that, so turn the assert > > into an if and write "on". That changes the mangling in the following > > way: > > > > With this patch: > > > > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ > > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > > > G++10: > > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ > > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > > > clang++/icc: > > $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ > > decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > > > I'm not sure why we differ in the "(*this)." part > > Is there a PR for that? I just opened 98756, because I didn't find any. I can investigate where that (*this) comes from, though it's not readily clear to me if this is a bug or not. > > but at least the > > suffix "onclspcvT__EEEDpS2_" is the same for all three compilers. So > > I hope the following fix makes sense. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > gcc/cp/ChangeLog: > > > > PR c++/98545 > > * mangle.c (write_expression): When the expression is a dependent name > > and an operator name, write "on" before writing its name. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/98545 > > * g++.dg/abi/mangle76.C: New test. > > --- > > gcc/cp/mangle.c | 3 ++- > > gcc/testsuite/g++.dg/abi/mangle76.C | 39 +++++++++++++++++++++++++++++ > > 2 files changed, 41 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C > > > > diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c > > index 11eb8962d28..bb3c4b76d33 100644 > > --- a/gcc/cp/mangle.c > > +++ b/gcc/cp/mangle.c > > @@ -3349,7 +3349,8 @@ write_expression (tree expr) > > else if (dependent_name (expr)) > > { > > tree name = dependent_name (expr); > > - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); > > + if (IDENTIFIER_ANY_OP_P (name)) > > + write_string ("on"); > > Any mangling change needs to handle different -fabi-versions; see the > similar code in write_member_name. Ah, I only looked at the unguarded IDENTIFIER_ANY_OP_P checks. But now I have a possibly stupid question: what version should I check? We have Version 11 for which the manual already says "corrects the mangling of sizeof... expressions and *operator names*", so perhaps I could tag along and check abi_version_at_least (11). Or should I check Version 15 and update the manual? > And why doesn't this go through write_member_name? We go through write_member_name: #0 fancy_abort (file=0x2b98ef8 "/home/mpolacek/src/gcc/gcc/cp/mangle.c", line=3352, function=0x2b99751 "write_expression") at /home/mpolacek/src/gcc/gcc/diagnostic.c:1884 #1 0x0000000000bee91b in write_expression (expr=<overload 0x7fffea02eb20>) at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3352 #2 0x0000000000beb3e2 in write_member_name (member=<baselink 0x7fffea043ae0>) at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2892 #3 0x0000000000beee70 in write_expression (expr=<component_ref 0x7fffea043b40>) at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3405 #4 0x0000000000bef1be in write_expression (expr=<call_expr 0x7fffe9ede118>) at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3455 #5 0x0000000000be858a in write_type (type=<decltype_type 0x7fffea04b348>) at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2343 so in write_member_name MEMBER is a BASELINK so we don't enter the identifier_p block. Marek
On Tue, Jan 19, 2021 at 05:38:20PM -0500, Marek Polacek via Gcc-patches wrote: > On Tue, Jan 19, 2021 at 03:47:47PM -0500, Jason Merrill via Gcc-patches wrote: > > On 1/13/21 6:39 PM, Marek Polacek wrote: > > > r11-6301 added some asserts in mangle.c, and now we trip over one of > > > them. In particular, it's the one asserting that we didn't get > > > IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. > > > > > > As this testcase shows, it's possible to get that, so turn the assert > > > into an if and write "on". That changes the mangling in the following > > > way: > > > > > > With this patch: > > > > > > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ > > > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > > > > > G++10: > > > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ > > > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > > > > > clang++/icc: > > > $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ > > > decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > > > > > I'm not sure why we differ in the "(*this)." part > > > > Is there a PR for that? > > I just opened 98756, because I didn't find any. I can investigate where that > (*this) comes from, though it's not readily clear to me if this is a bug or not. > > > > but at least the > > > suffix "onclspcvT__EEEDpS2_" is the same for all three compilers. So > > > I hope the following fix makes sense. > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/98545 > > > * mangle.c (write_expression): When the expression is a dependent name > > > and an operator name, write "on" before writing its name. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/98545 > > > * g++.dg/abi/mangle76.C: New test. > > > --- > > > gcc/cp/mangle.c | 3 ++- > > > gcc/testsuite/g++.dg/abi/mangle76.C | 39 +++++++++++++++++++++++++++++ > > > 2 files changed, 41 insertions(+), 1 deletion(-) > > > create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C > > > > > > diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c > > > index 11eb8962d28..bb3c4b76d33 100644 > > > --- a/gcc/cp/mangle.c > > > +++ b/gcc/cp/mangle.c > > > @@ -3349,7 +3349,8 @@ write_expression (tree expr) > > > else if (dependent_name (expr)) > > > { > > > tree name = dependent_name (expr); > > > - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); > > > + if (IDENTIFIER_ANY_OP_P (name)) > > > + write_string ("on"); > > > > Any mangling change needs to handle different -fabi-versions; see the > > similar code in write_member_name. > > Ah, I only looked at the unguarded IDENTIFIER_ANY_OP_P checks. But now > I have a possibly stupid question: what version should I check? We have > Version 11 for which the manual already says "corrects the mangling of > sizeof... expressions and *operator names*", so perhaps I could tag along > and check abi_version_at_least (11). Or should I check Version 15 and > update the manual? The latter seems to be true, therefore a new patch is attached. > > And why doesn't this go through write_member_name? > > We go through write_member_name: > > #0 fancy_abort (file=0x2b98ef8 "/home/mpolacek/src/gcc/gcc/cp/mangle.c", line=3352, > function=0x2b99751 "write_expression") at /home/mpolacek/src/gcc/gcc/diagnostic.c:1884 > #1 0x0000000000bee91b in write_expression (expr=<overload 0x7fffea02eb20>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3352 > #2 0x0000000000beb3e2 in write_member_name (member=<baselink 0x7fffea043ae0>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2892 > #3 0x0000000000beee70 in write_expression (expr=<component_ref 0x7fffea043b40>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3405 > #4 0x0000000000bef1be in write_expression (expr=<call_expr 0x7fffe9ede118>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3455 > #5 0x0000000000be858a in write_type (type=<decltype_type 0x7fffea04b348>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2343 > > so in write_member_name MEMBER is a BASELINK so we don't enter the > identifier_p block. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- r11-6301 added some asserts in mangle.c, and now we trip over one of them. In particular, it's the one asserting that we didn't get IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. As this testcase shows, it's possible to get that, so turn the assert into an if and write "on". That changes the mangling in the following way: With this patch: $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) G++10: $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) clang++/icc: $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) This is now tracked in PR98756. gcc/cp/ChangeLog: PR c++/98545 * mangle.c (write_expression): When the expression is a dependent name and an operator name, write "on" before writing its name. gcc/ChangeLog: PR c++/98545 * doc/invoke.texi: Update C++ ABI Version 15 description. gcc/testsuite/ChangeLog: PR c++/98545 * g++.dg/abi/mangle76.C: New test. --- gcc/cp/mangle.c | 7 ++++- gcc/doc/invoke.texi | 3 ++- gcc/testsuite/g++.dg/abi/mangle76.C | 40 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c index 11eb8962d28..9b358bbc5a1 100644 --- a/gcc/cp/mangle.c +++ b/gcc/cp/mangle.c @@ -3349,7 +3349,12 @@ write_expression (tree expr) else if (dependent_name (expr)) { tree name = dependent_name (expr); - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); + if (abi_version_at_least (15) && IDENTIFIER_ANY_OP_P (name)) + { + write_string ("on"); + if (abi_warn_or_compat_version_crosses (15)) + G.need_abi_warning = 1; + } write_unqualified_id (name); } else diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 5f4a06625eb..99d25462e48 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -2837,7 +2837,8 @@ Version 14, which first appeared in G++ 10, corrects the mangling of the nullptr expression. Version 15, which first appeared in G++ 11, changes the mangling of -@code{__alignof__} to be distinct from that of @code{alignof}. +@code{__alignof__} to be distinct from that of @code{alignof}, and +dependent operator names. See also @option{-Wabi}. diff --git a/gcc/testsuite/g++.dg/abi/mangle76.C b/gcc/testsuite/g++.dg/abi/mangle76.C new file mode 100644 index 00000000000..fe326e6c689 --- /dev/null +++ b/gcc/testsuite/g++.dg/abi/mangle76.C @@ -0,0 +1,40 @@ +// PR c++/98545 +// { dg-do compile { target c++11 } } +// { dg-options "-Wabi=14" } + +class a { +public: + a(); + template <typename b> a(b); +}; +template <class = double> using c = a; +class f { +protected: + template <class d, class e> void operator()(d, double, e); +}; +class i : f { +public: + template <class... g> + [[gnu::used]] auto h(g...) -> decltype(operator()(g()...)) {} // { dg-warning "mangled name" } +// { dg-final { scan-assembler "_ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_" } } +}; +template <class> class C { +public: + template <class j> C(j); + i k() const; + int operator()() { + int l = 10; + c<> m, n; + operator()(m, l, n); + return 0; + } + int operator()(c<> &, c<> const &, c<> const &) const; + template <class d, class e> void k(d m, double gamma, e o) const { + k().h(m, gamma, o); + } +}; +template <class r> int C<r>::operator()(c<> &, c<> const &, c<> const &) const { + [&](c<> m, double gamma, c<> o) { k(m, gamma, o); }; + return 0; +} +c<> p = C<double>(p)(); base-commit: 0fb7aa205afebe178c06683037ccd4c41104337a
On 1/19/21 5:38 PM, Marek Polacek wrote: > On Tue, Jan 19, 2021 at 03:47:47PM -0500, Jason Merrill via Gcc-patches wrote: >> On 1/13/21 6:39 PM, Marek Polacek wrote: >>> r11-6301 added some asserts in mangle.c, and now we trip over one of >>> them. In particular, it's the one asserting that we didn't get >>> IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. >>> >>> As this testcase shows, it's possible to get that, so turn the assert >>> into an if and write "on". That changes the mangling in the following >>> way: >>> >>> With this patch: >>> >>> $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ >>> decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) >>> >>> G++10: >>> $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ >>> decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) >>> >>> clang++/icc: >>> $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ >>> decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) >>> >>> I'm not sure why we differ in the "(*this)." part >> >> Is there a PR for that? > > I just opened 98756, because I didn't find any. I can investigate where that > (*this) comes from, though it's not readily clear to me if this is a bug or not. I think it is; in general, the mangling tries to be close to the expression as written. We're talking about adjusting that a bit to reflect the result of name lookup more, but that wouldn't make a difference to this case. >>> but at least the >>> suffix "onclspcvT__EEEDpS2_" is the same for all three compilers. So >>> I hope the following fix makes sense. >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/98545 >>> * mangle.c (write_expression): When the expression is a dependent name >>> and an operator name, write "on" before writing its name. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/98545 >>> * g++.dg/abi/mangle76.C: New test. >>> --- >>> gcc/cp/mangle.c | 3 ++- >>> gcc/testsuite/g++.dg/abi/mangle76.C | 39 +++++++++++++++++++++++++++++ >>> 2 files changed, 41 insertions(+), 1 deletion(-) >>> create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C >>> >>> diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c >>> index 11eb8962d28..bb3c4b76d33 100644 >>> --- a/gcc/cp/mangle.c >>> +++ b/gcc/cp/mangle.c >>> @@ -3349,7 +3349,8 @@ write_expression (tree expr) >>> else if (dependent_name (expr)) >>> { >>> tree name = dependent_name (expr); >>> - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); >>> + if (IDENTIFIER_ANY_OP_P (name)) >>> + write_string ("on"); >> >> Any mangling change needs to handle different -fabi-versions; see the >> similar code in write_member_name. > > Ah, I only looked at the unguarded IDENTIFIER_ANY_OP_P checks. But now > I have a possibly stupid question: what version should I check? We have > Version 11 for which the manual already says "corrects the mangling of > sizeof... expressions and *operator names*", so perhaps I could tag along > and check abi_version_at_least (11). Or should I check Version 15 and > update the manual? As discussed on our call today, we don't want to change the behavior of older versions, so version 15 is the answer. >> And why doesn't this go through write_member_name? > > We go through write_member_name: > > #0 fancy_abort (file=0x2b98ef8 "/home/mpolacek/src/gcc/gcc/cp/mangle.c", line=3352, > function=0x2b99751 "write_expression") at /home/mpolacek/src/gcc/gcc/diagnostic.c:1884 > #1 0x0000000000bee91b in write_expression (expr=<overload 0x7fffea02eb20>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3352 > #2 0x0000000000beb3e2 in write_member_name (member=<baselink 0x7fffea043ae0>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2892 > #3 0x0000000000beee70 in write_expression (expr=<component_ref 0x7fffea043b40>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3405 > #4 0x0000000000bef1be in write_expression (expr=<call_expr 0x7fffe9ede118>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3455 > #5 0x0000000000be858a in write_type (type=<decltype_type 0x7fffea04b348>) > at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2343 > > so in write_member_name MEMBER is a BASELINK so we don't enter the > identifier_p block. Aha. Jason
On 1/21/21 2:44 PM, Marek Polacek wrote: > On Tue, Jan 19, 2021 at 05:38:20PM -0500, Marek Polacek via Gcc-patches wrote: >> On Tue, Jan 19, 2021 at 03:47:47PM -0500, Jason Merrill via Gcc-patches wrote: >>> On 1/13/21 6:39 PM, Marek Polacek wrote: >>>> r11-6301 added some asserts in mangle.c, and now we trip over one of >>>> them. In particular, it's the one asserting that we didn't get >>>> IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. >>>> >>>> As this testcase shows, it's possible to get that, so turn the assert >>>> into an if and write "on". That changes the mangling in the following >>>> way: >>>> >>>> With this patch: >>>> >>>> $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ >>>> decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) >>>> >>>> G++10: >>>> $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ >>>> decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) >>>> >>>> clang++/icc: >>>> $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ >>>> decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) >>>> >>>> I'm not sure why we differ in the "(*this)." part >>> >>> Is there a PR for that? >> >> I just opened 98756, because I didn't find any. I can investigate where that >> (*this) comes from, though it's not readily clear to me if this is a bug or not. >> >>>> but at least the >>>> suffix "onclspcvT__EEEDpS2_" is the same for all three compilers. So >>>> I hope the following fix makes sense. >>>> >>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/98545 >>>> * mangle.c (write_expression): When the expression is a dependent name >>>> and an operator name, write "on" before writing its name. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/98545 >>>> * g++.dg/abi/mangle76.C: New test. >>>> --- >>>> gcc/cp/mangle.c | 3 ++- >>>> gcc/testsuite/g++.dg/abi/mangle76.C | 39 +++++++++++++++++++++++++++++ >>>> 2 files changed, 41 insertions(+), 1 deletion(-) >>>> create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C >>>> >>>> diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c >>>> index 11eb8962d28..bb3c4b76d33 100644 >>>> --- a/gcc/cp/mangle.c >>>> +++ b/gcc/cp/mangle.c >>>> @@ -3349,7 +3349,8 @@ write_expression (tree expr) >>>> else if (dependent_name (expr)) >>>> { >>>> tree name = dependent_name (expr); >>>> - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); >>>> + if (IDENTIFIER_ANY_OP_P (name)) >>>> + write_string ("on"); >>> >>> Any mangling change needs to handle different -fabi-versions; see the >>> similar code in write_member_name. >> >> Ah, I only looked at the unguarded IDENTIFIER_ANY_OP_P checks. But now >> I have a possibly stupid question: what version should I check? We have >> Version 11 for which the manual already says "corrects the mangling of >> sizeof... expressions and *operator names*", so perhaps I could tag along >> and check abi_version_at_least (11). Or should I check Version 15 and >> update the manual? > > The latter seems to be true, therefore a new patch is attached. > >>> And why doesn't this go through write_member_name? >> >> We go through write_member_name: >> >> #0 fancy_abort (file=0x2b98ef8 "/home/mpolacek/src/gcc/gcc/cp/mangle.c", line=3352, >> function=0x2b99751 "write_expression") at /home/mpolacek/src/gcc/gcc/diagnostic.c:1884 >> #1 0x0000000000bee91b in write_expression (expr=<overload 0x7fffea02eb20>) >> at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3352 >> #2 0x0000000000beb3e2 in write_member_name (member=<baselink 0x7fffea043ae0>) >> at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2892 >> #3 0x0000000000beee70 in write_expression (expr=<component_ref 0x7fffea043b40>) >> at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3405 >> #4 0x0000000000bef1be in write_expression (expr=<call_expr 0x7fffe9ede118>) >> at /home/mpolacek/src/gcc/gcc/cp/mangle.c:3455 >> #5 0x0000000000be858a in write_type (type=<decltype_type 0x7fffea04b348>) >> at /home/mpolacek/src/gcc/gcc/cp/mangle.c:2343 >> >> so in write_member_name MEMBER is a BASELINK so we don't enter the >> identifier_p block. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > r11-6301 added some asserts in mangle.c, and now we trip over one of > them. In particular, it's the one asserting that we didn't get > IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. > > As this testcase shows, it's possible to get that, so turn the assert > into an if and write "on". That changes the mangling in the following > way: > > With this patch: > > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > G++10: > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > clang++/icc: > $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ > decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > This is now tracked in PR98756. > > gcc/cp/ChangeLog: > > PR c++/98545 > * mangle.c (write_expression): When the expression is a dependent name > and an operator name, write "on" before writing its name. > > gcc/ChangeLog: > > PR c++/98545 > * doc/invoke.texi: Update C++ ABI Version 15 description. > > gcc/testsuite/ChangeLog: > > PR c++/98545 > * g++.dg/abi/mangle76.C: New test. > --- > gcc/cp/mangle.c | 7 ++++- > gcc/doc/invoke.texi | 3 ++- > gcc/testsuite/g++.dg/abi/mangle76.C | 40 +++++++++++++++++++++++++++++ > 3 files changed, 48 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C > > diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c > index 11eb8962d28..9b358bbc5a1 100644 > --- a/gcc/cp/mangle.c > +++ b/gcc/cp/mangle.c > @@ -3349,7 +3349,12 @@ write_expression (tree expr) > else if (dependent_name (expr)) > { > tree name = dependent_name (expr); > - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); > + if (abi_version_at_least (15) && IDENTIFIER_ANY_OP_P (name)) > + { > + write_string ("on"); > + if (abi_warn_or_compat_version_crosses (15)) > + G.need_abi_warning = 1; We also want the warning if the version is less than 15 and the compat version is 15 or greater. Ah, it seems write_member_name gets this wrong, too. The abi_warn_or_compat_version_crosses check should not be guarded by abi_version_at_least; all the other uses I'm seeing get this right. > + } > write_unqualified_id (name); > } > else > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 5f4a06625eb..99d25462e48 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -2837,7 +2837,8 @@ Version 14, which first appeared in G++ 10, corrects the mangling of > the nullptr expression. > > Version 15, which first appeared in G++ 11, changes the mangling of > -@code{__alignof__} to be distinct from that of @code{alignof}. > +@code{__alignof__} to be distinct from that of @code{alignof}, and > +dependent operator names. > > See also @option{-Wabi}. > > diff --git a/gcc/testsuite/g++.dg/abi/mangle76.C b/gcc/testsuite/g++.dg/abi/mangle76.C > new file mode 100644 > index 00000000000..fe326e6c689 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/abi/mangle76.C > @@ -0,0 +1,40 @@ > +// PR c++/98545 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wabi=14" } > + > +class a { > +public: > + a(); > + template <typename b> a(b); > +}; > +template <class = double> using c = a; > +class f { > +protected: > + template <class d, class e> void operator()(d, double, e); > +}; > +class i : f { > +public: > + template <class... g> > + [[gnu::used]] auto h(g...) -> decltype(operator()(g()...)) {} // { dg-warning "mangled name" } > +// { dg-final { scan-assembler "_ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_" } } > +}; > +template <class> class C { > +public: > + template <class j> C(j); > + i k() const; > + int operator()() { > + int l = 10; > + c<> m, n; > + operator()(m, l, n); > + return 0; > + } > + int operator()(c<> &, c<> const &, c<> const &) const; > + template <class d, class e> void k(d m, double gamma, e o) const { > + k().h(m, gamma, o); > + } > +}; > +template <class r> int C<r>::operator()(c<> &, c<> const &, c<> const &) const { > + [&](c<> m, double gamma, c<> o) { k(m, gamma, o); }; > + return 0; > +} > +c<> p = C<double>(p)(); > > base-commit: 0fb7aa205afebe178c06683037ccd4c41104337a >
On Thu, Jan 21, 2021 at 05:41:06PM -0500, Jason Merrill via Gcc-patches wrote: > On 1/21/21 2:44 PM, Marek Polacek wrote: > > @@ -3349,7 +3349,12 @@ write_expression (tree expr) > > else if (dependent_name (expr)) > > { > > tree name = dependent_name (expr); > > - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); > > + if (abi_version_at_least (15) && IDENTIFIER_ANY_OP_P (name)) > > + { > > + write_string ("on"); > > + if (abi_warn_or_compat_version_crosses (15)) > > + G.need_abi_warning = 1; > > We also want the warning if the version is less than 15 and the compat > version is 15 or greater. Ah, it seems write_member_name gets this wrong, > too. The abi_warn_or_compat_version_crosses check should not be guarded by > abi_version_at_least; all the other uses I'm seeing get this right. Ah, makes sense. Fixed both here: Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- r11-6301 added some asserts in mangle.c, and now we trip over one of them. In particular, it's the one asserting that we didn't get IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. As this testcase shows, it's possible to get that, so turn the assert into an if and write "on". That changes the mangling in the following way: With this patch: $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) G++10: $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) clang++/icc: $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) This is now tracked in PR98756. gcc/cp/ChangeLog: PR c++/98545 * mangle.c (write_member_name): Emit abi_warn_or_compat_version_crosses warnings regardless of abi_version_at_least. (write_expression): When the expression is a dependent name and an operator name, write "on" before writing its name. gcc/ChangeLog: PR c++/98545 * doc/invoke.texi: Update C++ ABI Version 15 description. gcc/testsuite/ChangeLog: PR c++/98545 * g++.dg/abi/mangle76.C: New test. --- gcc/cp/mangle.c | 13 +++++++--- gcc/doc/invoke.texi | 3 ++- gcc/testsuite/g++.dg/abi/mangle76.C | 40 +++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c index 11eb8962d28..0a9e5aa79a0 100644 --- a/gcc/cp/mangle.c +++ b/gcc/cp/mangle.c @@ -2868,9 +2868,10 @@ write_member_name (tree member) { if (identifier_p (member)) { - if (abi_version_at_least (11) && IDENTIFIER_ANY_OP_P (member)) + if (IDENTIFIER_ANY_OP_P (member)) { - write_string ("on"); + if (abi_version_at_least (11)) + write_string ("on"); if (abi_warn_or_compat_version_crosses (11)) G.need_abi_warning = 1; } @@ -3349,7 +3350,13 @@ write_expression (tree expr) else if (dependent_name (expr)) { tree name = dependent_name (expr); - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); + if (IDENTIFIER_ANY_OP_P (name)) + { + if (abi_version_at_least (15)) + write_string ("on"); + if (abi_warn_or_compat_version_crosses (15)) + G.need_abi_warning = 1; + } write_unqualified_id (name); } else diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 5f4a06625eb..99d25462e48 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -2837,7 +2837,8 @@ Version 14, which first appeared in G++ 10, corrects the mangling of the nullptr expression. Version 15, which first appeared in G++ 11, changes the mangling of -@code{__alignof__} to be distinct from that of @code{alignof}. +@code{__alignof__} to be distinct from that of @code{alignof}, and +dependent operator names. See also @option{-Wabi}. diff --git a/gcc/testsuite/g++.dg/abi/mangle76.C b/gcc/testsuite/g++.dg/abi/mangle76.C new file mode 100644 index 00000000000..fe326e6c689 --- /dev/null +++ b/gcc/testsuite/g++.dg/abi/mangle76.C @@ -0,0 +1,40 @@ +// PR c++/98545 +// { dg-do compile { target c++11 } } +// { dg-options "-Wabi=14" } + +class a { +public: + a(); + template <typename b> a(b); +}; +template <class = double> using c = a; +class f { +protected: + template <class d, class e> void operator()(d, double, e); +}; +class i : f { +public: + template <class... g> + [[gnu::used]] auto h(g...) -> decltype(operator()(g()...)) {} // { dg-warning "mangled name" } +// { dg-final { scan-assembler "_ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_" } } +}; +template <class> class C { +public: + template <class j> C(j); + i k() const; + int operator()() { + int l = 10; + c<> m, n; + operator()(m, l, n); + return 0; + } + int operator()(c<> &, c<> const &, c<> const &) const; + template <class d, class e> void k(d m, double gamma, e o) const { + k().h(m, gamma, o); + } +}; +template <class r> int C<r>::operator()(c<> &, c<> const &, c<> const &) const { + [&](c<> m, double gamma, c<> o) { k(m, gamma, o); }; + return 0; +} +c<> p = C<double>(p)(); base-commit: eee8ed2f22b36dfe64a9516171871476e8ede477
On 1/22/21 12:02 PM, Marek Polacek wrote: > On Thu, Jan 21, 2021 at 05:41:06PM -0500, Jason Merrill via Gcc-patches wrote: >> On 1/21/21 2:44 PM, Marek Polacek wrote: >>> @@ -3349,7 +3349,12 @@ write_expression (tree expr) >>> else if (dependent_name (expr)) >>> { >>> tree name = dependent_name (expr); >>> - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); >>> + if (abi_version_at_least (15) && IDENTIFIER_ANY_OP_P (name)) >>> + { >>> + write_string ("on"); >>> + if (abi_warn_or_compat_version_crosses (15)) >>> + G.need_abi_warning = 1; >> >> We also want the warning if the version is less than 15 and the compat >> version is 15 or greater. Ah, it seems write_member_name gets this wrong, >> too. The abi_warn_or_compat_version_crosses check should not be guarded by >> abi_version_at_least; all the other uses I'm seeing get this right. > > Ah, makes sense. Fixed both here: > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > -- >8 -- > r11-6301 added some asserts in mangle.c, and now we trip over one of > them. In particular, it's the one asserting that we didn't get > IDENTIFIER_ANY_OP_P when mangling an expression with a dependent name. > > As this testcase shows, it's possible to get that, so turn the assert > into an if and write "on". That changes the mangling in the following > way: > > With this patch: > > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_ > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > G++10: > $ c++filt _ZN1i1hIJ1adS1_EEEDTcldtdefpTclspcvT__EEEDpS2_ > decltype (((*this).(operator()))((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > clang++/icc: > $ c++filt _ZN1i1hIJ1adS1_EEEDTclonclspcvT__EEEDpS2_ > decltype ((operator())((a)(), (double)(), (a)())) i::h<a, double, a>(a, double, a) > > This is now tracked in PR98756. > > gcc/cp/ChangeLog: > > PR c++/98545 > * mangle.c (write_member_name): Emit abi_warn_or_compat_version_crosses > warnings regardless of abi_version_at_least. > (write_expression): When the expression is a dependent name > and an operator name, write "on" before writing its name. > > gcc/ChangeLog: > > PR c++/98545 > * doc/invoke.texi: Update C++ ABI Version 15 description. > > gcc/testsuite/ChangeLog: > > PR c++/98545 > * g++.dg/abi/mangle76.C: New test. > --- > gcc/cp/mangle.c | 13 +++++++--- > gcc/doc/invoke.texi | 3 ++- > gcc/testsuite/g++.dg/abi/mangle76.C | 40 +++++++++++++++++++++++++++++ > 3 files changed, 52 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/abi/mangle76.C > > diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c > index 11eb8962d28..0a9e5aa79a0 100644 > --- a/gcc/cp/mangle.c > +++ b/gcc/cp/mangle.c > @@ -2868,9 +2868,10 @@ write_member_name (tree member) > { > if (identifier_p (member)) > { > - if (abi_version_at_least (11) && IDENTIFIER_ANY_OP_P (member)) > + if (IDENTIFIER_ANY_OP_P (member)) > { > - write_string ("on"); > + if (abi_version_at_least (11)) > + write_string ("on"); > if (abi_warn_or_compat_version_crosses (11)) > G.need_abi_warning = 1; > } > @@ -3349,7 +3350,13 @@ write_expression (tree expr) > else if (dependent_name (expr)) > { > tree name = dependent_name (expr); > - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); > + if (IDENTIFIER_ANY_OP_P (name)) > + { > + if (abi_version_at_least (15)) > + write_string ("on"); > + if (abi_warn_or_compat_version_crosses (15)) > + G.need_abi_warning = 1; > + } > write_unqualified_id (name); > } > else > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 5f4a06625eb..99d25462e48 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -2837,7 +2837,8 @@ Version 14, which first appeared in G++ 10, corrects the mangling of > the nullptr expression. > > Version 15, which first appeared in G++ 11, changes the mangling of > -@code{__alignof__} to be distinct from that of @code{alignof}. > +@code{__alignof__} to be distinct from that of @code{alignof}, and > +dependent operator names. > > See also @option{-Wabi}. > > diff --git a/gcc/testsuite/g++.dg/abi/mangle76.C b/gcc/testsuite/g++.dg/abi/mangle76.C > new file mode 100644 > index 00000000000..fe326e6c689 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/abi/mangle76.C > @@ -0,0 +1,40 @@ > +// PR c++/98545 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wabi=14" } > + > +class a { > +public: > + a(); > + template <typename b> a(b); > +}; > +template <class = double> using c = a; > +class f { > +protected: > + template <class d, class e> void operator()(d, double, e); > +}; > +class i : f { > +public: > + template <class... g> > + [[gnu::used]] auto h(g...) -> decltype(operator()(g()...)) {} // { dg-warning "mangled name" } > +// { dg-final { scan-assembler "_ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_" } } > +}; > +template <class> class C { > +public: > + template <class j> C(j); > + i k() const; > + int operator()() { > + int l = 10; > + c<> m, n; > + operator()(m, l, n); > + return 0; > + } > + int operator()(c<> &, c<> const &, c<> const &) const; > + template <class d, class e> void k(d m, double gamma, e o) const { > + k().h(m, gamma, o); > + } > +}; > +template <class r> int C<r>::operator()(c<> &, c<> const &, c<> const &) const { > + [&](c<> m, double gamma, c<> o) { k(m, gamma, o); }; > + return 0; > +} > +c<> p = C<double>(p)(); > > base-commit: eee8ed2f22b36dfe64a9516171871476e8ede477 >
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c index 11eb8962d28..bb3c4b76d33 100644 --- a/gcc/cp/mangle.c +++ b/gcc/cp/mangle.c @@ -3349,7 +3349,8 @@ write_expression (tree expr) else if (dependent_name (expr)) { tree name = dependent_name (expr); - gcc_assert (!IDENTIFIER_ANY_OP_P (name)); + if (IDENTIFIER_ANY_OP_P (name)) + write_string ("on"); write_unqualified_id (name); } else diff --git a/gcc/testsuite/g++.dg/abi/mangle76.C b/gcc/testsuite/g++.dg/abi/mangle76.C new file mode 100644 index 00000000000..0c2964cbecb --- /dev/null +++ b/gcc/testsuite/g++.dg/abi/mangle76.C @@ -0,0 +1,39 @@ +// PR c++/98545 +// { dg-do compile { target c++11 } } + +class a { +public: + a(); + template <typename b> a(b); +}; +template <class = double> using c = a; +class f { +protected: + template <class d, class e> void operator()(d, double, e); +}; +class i : f { +public: + template <class... g> + [[gnu::used]] auto h(g...) -> decltype(operator()(g()...)) {} +// { dg-final { scan-assembler "_ZN1i1hIJ1adS1_EEEDTcldtdefpTonclspcvT__EEEDpS2_" } } +}; +template <class> class C { +public: + template <class j> C(j); + i k() const; + int operator()() { + int l = 10; + c<> m, n; + operator()(m, l, n); + return 0; + } + int operator()(c<> &, c<> const &, c<> const &) const; + template <class d, class e> void k(d m, double gamma, e o) const { + k().h(m, gamma, o); + } +}; +template <class r> int C<r>::operator()(c<> &, c<> const &, c<> const &) const { + [&](c<> m, double gamma, c<> o) { k(m, gamma, o); }; + return 0; +} +c<> p = C<double>(p)();