Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On 02/17/2017 09:53 PM, Jason Merrill wrote: On Thu, Feb 16, 2017 at 6:13 PM, Martin Seborwrote: On 02/16/2017 12:49 PM, Jason Merrill wrote: On Thu, Feb 16, 2017 at 11:33 AM, Jakub Jelinek wrote: PR c++/79502 * pt.c (apply_late_template_attributes): If there are no dependent attributes, set *p to attributes. If there were some attributes in *p previously with or without dependent attributes, chain them after the new attributes. Here's the variant of your patch that I'm applying. Sorry to butt in but I feel like I'm missing something basic. Are these attributes (nodiscard, noreturn, maybe_unused, and deprecated) meant to apply to templates? The text in for nodiscard suggests they're not: The attribute-token nodiscard may be applied to the declarator-id in a function declaration or to the declaration of a class or enumeration. Noreturn also doesn't mention templates: The attribute may be applied to the declarator-id in a function declaration. Deprecated explicitly mentions template specializations but not primary templates: The attribute may be applied to the declaration of a class, a typedef-name, a variable, a non-static data member, a function, a namespace, an enumeration, an enumerator, or a template specialization. I can certainly see how applying attributes to the primary template would be useful so it's puzzling to me that the standard seems to preclude it. I don't think it's precluded; a /template-declaration/ syntactically includes a /declaration/, so in general any statement about e.g. a function declaration also applies to a function template declaration. I'm sure you're right that it's not meant to be precluded, otherwise the standard library couldn't declare noreturn the throw_with_nested function template (the only template declared noreturn in the library I could find). FWIW, a context where noreturn clearly is not applicable is function pointers. G++ accepts it there but then ignores it. Clang rejects it. I would think accepting it on function pointers would be useful as well. GCC (in C mode) accepts __attribute__ noreturn on function pointers and treats them as such (G++ does not). Another interesting context is the explicit instantiation directive. Again, the standard seems clear that noreturn isn't allowed but GCC and Microsoft Visual C++ both accept it. Clang and EDG reject it. I think rejecting it here makes sense and accepting is a bug. I ask also because I was just looking at bug 79021 and scratching my head about what to thing about it. While trying to understand how GCC handles attributes for the primary template I came across what doesn't make sense to me. Why would it apply the attribute from the primary to the explicit specialization when the two are distinct entities? Is that a bug? This seems like a Core issue; the standard says nothing about how attributes on a template affect specializations. Yes, I thought so as well. Thanks for confirming that. Unless you would prefer to bring it up yourself let me post it to the core reflector and suggest to open a new issue for it. I think that as a general rule, not applying attributes from the template to specializations makes sense. There will be some exceptions, such as the abi_tag attribute which is a property of the name rather than a particular declaration. That makes sense to me. Thanks Martin
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On Thu, Feb 16, 2017 at 6:13 PM, Martin Seborwrote: > On 02/16/2017 12:49 PM, Jason Merrill wrote: >> >> On Thu, Feb 16, 2017 at 11:33 AM, Jakub Jelinek wrote: >>> >>> PR c++/79502 >>> * pt.c (apply_late_template_attributes): If there are >>> no dependent attributes, set *p to attributes. If there were >>> some attributes in *p previously with or without dependent >>> attributes, chain them after the new attributes. >> >> >> Here's the variant of your patch that I'm applying. > > > Sorry to butt in but I feel like I'm missing something basic. Are > these attributes (nodiscard, noreturn, maybe_unused, and deprecated) > meant to apply to templates? The text in for nodiscard suggests > they're not: > > The attribute-token nodiscard may be applied to the declarator-id > in a function declaration or to the declaration of a class or > enumeration. > > Noreturn also doesn't mention templates: > > The attribute may be applied to the declarator-id in a function > declaration. > > Deprecated explicitly mentions template specializations but not > primary templates: > > The attribute may be applied to the declaration of a class, >a typedef-name, a variable, a non-static data member, a function, >a namespace, an enumeration, an enumerator, or a template >specialization. > > I can certainly see how applying attributes to the primary template > would be useful so it's puzzling to me that the standard seems to > preclude it. I don't think it's precluded; a /template-declaration/ syntactically includes a /declaration/, so in general any statement about e.g. a function declaration also applies to a function template declaration. > I ask also because I was just looking at bug 79021 and scratching > my head about what to thing about it. While trying to understand > how GCC handles attributes for the primary template I came across > what doesn't make sense to me. Why would it apply the attribute > from the primary to the explicit specialization when the two are > distinct entities? Is that a bug? This seems like a Core issue; the standard says nothing about how attributes on a template affect specializations. I think that as a general rule, not applying attributes from the template to specializations makes sense. There will be some exceptions, such as the abi_tag attribute which is a property of the name rather than a particular declaration. Jason
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On 02/16/2017 12:49 PM, Jason Merrill wrote: On Thu, Feb 16, 2017 at 11:33 AM, Jakub Jelinekwrote: PR c++/79502 * pt.c (apply_late_template_attributes): If there are no dependent attributes, set *p to attributes. If there were some attributes in *p previously with or without dependent attributes, chain them after the new attributes. Here's the variant of your patch that I'm applying. Sorry to butt in but I feel like I'm missing something basic. Are these attributes (nodiscard, noreturn, maybe_unused, and deprecated) meant to apply to templates? The text in for nodiscard suggests they're not: The attribute-token nodiscard may be applied to the declarator-id in a function declaration or to the declaration of a class or enumeration. Noreturn also doesn't mention templates: The attribute may be applied to the declarator-id in a function declaration. Deprecated explicitly mentions template specializations but not primary templates: The attribute may be applied to the declaration of a class, a typedef-name, a variable, a non-static data member, a function, a namespace, an enumeration, an enumerator, or a template specialization. I can certainly see how applying attributes to the primary template would be useful so it's puzzling to me that the standard seems to preclude it. I ask also because I was just looking at bug 79021 and scratching my head about what to thing about it. While trying to understand how GCC handles attributes for the primary template I came across what doesn't make sense to me. Why would it apply the attribute from the primary to the explicit specialization when the two are distinct entities? Is that a bug? template [[noreturn]] int f () { throw ""; } template <> int f () { return 0; } t.C: In function ‘int f() [with T = void]’: t.C:4:37: warning: function declared ‘noreturn’ has a ‘return’ statement template <> int f () { return 0; } ^ t.C:4:37: warning: ‘noreturn’ function does return template <> int f () { return 0; } ^ Clang complains on this too with similar errors, but then GCC silently accepts this code (which makes sense to me) which Clang 5 rejects with what looks like a bug: template int g () { return 0; } template <> [[noreturn]] int g () { throw ""; } template <> [[noreturn]] int g () { throw ""; } Thanks Martin
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On Thu, Feb 16, 2017 at 11:33 AM, Jakub Jelinekwrote: > PR c++/79502 > * pt.c (apply_late_template_attributes): If there are > no dependent attributes, set *p to attributes. If there were > some attributes in *p previously with or without dependent > attributes, chain them after the new attributes. Here's the variant of your patch that I'm applying. commit 3a5098ff79743f89c2e0e3cfa6a00ee82ec26b78 Author: Jason Merrill Date: Thu Feb 16 13:05:56 2017 -0500 PR c++/79502 - lost nodiscard attribute * pt.c (apply_late_template_attributes): Do apply non-dependent attributes to types. diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 712fb69..c468268 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -10073,29 +10073,43 @@ apply_late_template_attributes (tree *decl_p, tree attributes, int attr_flags, tree t; tree *p; - for (t = attributes; t; t = TREE_CHAIN (t)) -if (ATTR_IS_DEPENDENT (t)) - { - last_dep = t; - attributes = copy_list (attributes); - break; - } + if (attributes == NULL_TREE) +return; if (DECL_P (*decl_p)) { if (TREE_TYPE (*decl_p) == error_mark_node) return; p = _ATTRIBUTES (*decl_p); + /* DECL_ATTRIBUTES comes from copy_node in tsubst_decl, and identical + to our attributes parameter. */ + gcc_assert (*p == attributes); } else -p = _ATTRIBUTES (*decl_p); +{ + p = _ATTRIBUTES (*decl_p); + /* TYPE_ATTRIBUTES was set up (with abi_tag and may_alias) in +lookup_template_class_1, and should be preserved. */ + gcc_assert (*p != attributes); + while (*p) + p = _CHAIN (*p); +} + for (t = attributes; t; t = TREE_CHAIN (t)) +if (ATTR_IS_DEPENDENT (t)) + { + last_dep = t; + attributes = copy_list (attributes); + break; + } + + *p = attributes; if (last_dep) { tree late_attrs = NULL_TREE; tree *q = _attrs; - for (*p = attributes; *p; ) + for (; *p; ) { t = *p; if (ATTR_IS_DEPENDENT (t)) diff --git a/gcc/testsuite/g++.dg/cpp0x/attrib54.C b/gcc/testsuite/g++.dg/cpp0x/attrib54.C new file mode 100644 index 000..e5817c3 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/attrib54.C @@ -0,0 +1,21 @@ +// { dg-do compile { target c++11 } } + +inline namespace N __attribute__((__abi_tag__ ("foo"))) {} +template struct A {}; +namespace N { +template class B {}; +} +template class __attribute__((__aligned__ (sizeof (T C {}; +template struct D { + template using G = C<_Up>; +}; +template struct F { + template struct H { +typedef typename D::template G I; + }; +}; +template > struct J { + C> L; + typedef F ::H>::I M; + J *a; +}; diff --git a/gcc/testsuite/g++.dg/cpp0x/attrib55.C b/gcc/testsuite/g++.dg/cpp0x/attrib55.C new file mode 100644 index 000..79d0c8c --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/attrib55.C @@ -0,0 +1,21 @@ +// { dg-do compile { target c++11 } } + +inline namespace N __attribute__((__abi_tag__ ("foo"))) {} +template struct A {}; +namespace N { +template class B {}; +} +template class __attribute__((__unused__)) C {}; +template struct D { + template using G = C<_Up>; +}; +template struct F { + template struct H { +typedef typename D::template G I; + }; +}; +template > struct J { + C> L; + typedef F ::H>::I M; + J *a; +}; diff --git a/gcc/testsuite/g++.dg/cpp1z/nodiscard4.C b/gcc/testsuite/g++.dg/cpp1z/nodiscard4.C new file mode 100644 index 000..8a95c94 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/nodiscard4.C @@ -0,0 +1,14 @@ +// PR c++/79502 +// { dg-do compile { target c++11 } } + +template +struct [[nodiscard]] missiles {}; + +missiles make() { return {}; } +missiles (*fnptr)() = make; + +int main() +{ + make(); // { dg-warning "ignoring returned value of type" } + fnptr(); // { dg-warning "ignoring returned value of type" } +} diff --git a/gcc/testsuite/g++.dg/ext/attrib53.C b/gcc/testsuite/g++.dg/ext/attrib53.C new file mode 100644 index 000..408433d --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attrib53.C @@ -0,0 +1,21 @@ +// { dg-do compile { target c++11 } } + +inline namespace N __attribute__((__abi_tag__ ("foo"))) {} +template struct A; +namespace N { +template class B; +} +template class C {}; +template struct D { + template using G = C<_Up>; +}; +template struct F { + template struct H { +typedef typename D::template G I; + }; +}; +template > struct J { + C> L; + typedef F ::H>::I M; + J *a; +};
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On Thu, Feb 16, 2017 at 04:57:54PM +0100, Jakub Jelinek wrote: > Unfortunately it broke bootstrap, *p can already contain some > other attributes (like abi_tag) and then we ICE in > apply_identity_attributes. Note the attrib54.C testcase ICEs even > on vanilla trunk, so if there are any dependent attribute, it is preexisting > issue. > > The following patch bootstrapped successfully, but I see > +FAIL: g++.dg/ext/vector32.C -std=c++* (internal compiler error) > +FAIL: g++.dg/ext/vector32a.C -std=c++* (internal compiler error) > +FAIL: g++.dg/gomp/declare-simd-1.C -std=gnu++* (internal compiler error) > +FAIL: g++.dg/gomp/declare-simd-1.C -std=gnu++* (test for excess errors) > +FAIL: g++.dg/lto/20091219 cp_lto_20091219_0.o assemble, -O3 -flto (internal > compiler error) > +FAIL: g++.dg/vect/simd-clone-4.cc -std=c++* (test for excess errors) > +FAIL: g++.dg/vect/simd-clone-5.cc -std=c++* (test for excess errors) > on i686-linux (x86_64-linux regtest still running). So there are further > issues. The issue with the above is that all those are DECL_ATTRIBUTES which are copied over through copy_node inside of tsubst_decl or so. Preserving the previous attributes in that case is harmful, they can contain dependent attributes etc. That compared to TYPE_ATTRIBUTES which start empty (at least for classes, for enums there is that t = start_enum (TYPE_IDENTIFIER (template_type), NULL_TREE, tsubst (ENUM_UNDERLYING_TYPE (template_type), arglist, complain, in_decl), tsubst_attributes (TYPE_ATTRIBUTES (template_type), arglist, complain, in_decl), SCOPED_ENUM_P (template_type), NULL); ) and then sometimes have something added there (such as the abi_tag attributes) before apply_late_template_attributes is called. With the following patch I have: make check-g++ RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} dg.exp='vector*.C attrib* nodiscard*' gomp.exp=declare-simd* lto.exp=20091219* vect.exp=simd-clone-*.cc" without any FAILs. Not sure about the enums, wonder if the abi_tags attributes couldn't be added even for those. 2017-02-16 Jakub JelinekPR c++/79502 * pt.c (apply_late_template_attributes): If there are no dependent attributes, set *p to attributes. If there were some attributes in *p previously with or without dependent attributes, chain them after the new attributes. * g++.dg/cpp1z/nodiscard4.C: New test. * g++.dg/ext/attrib53.C: New test. * g++.dg/ext/attrib54.C: New test. * g++.dg/ext/attrib55.C: New test. --- gcc/cp/pt.c.jj 2017-02-16 12:00:20.044455757 +0100 +++ gcc/cp/pt.c 2017-02-16 15:07:31.678727294 +0100 @@ -10086,6 +10086,7 @@ apply_late_template_attributes (tree *de if (TREE_TYPE (*decl_p) == error_mark_node) return; p = _ATTRIBUTES (*decl_p); + *p = NULL_TREE; } else p = _ATTRIBUTES (*decl_p); @@ -10094,6 +10095,7 @@ apply_late_template_attributes (tree *de { tree late_attrs = NULL_TREE; tree *q = _attrs; + tree prev = *p; for (*p = attributes; *p; ) { @@ -10110,9 +10112,14 @@ apply_late_template_attributes (tree *de else p = _CHAIN (t); } + *p = prev; cplus_decl_attributes (decl_p, late_attrs, attr_flags); } + else if (*p == NULL) +*p = attributes; + else if (attributes) +*p = chainon (copy_list (attributes), *p); } /* Perform (or defer) access check for typedefs that were referenced --- gcc/testsuite/g++.dg/cpp1z/nodiscard4.C.jj 2017-02-16 14:04:22.602748039 +0100 +++ gcc/testsuite/g++.dg/cpp1z/nodiscard4.C 2017-02-16 14:04:22.601748053 +0100 @@ -0,0 +1,14 @@ +// PR c++/79502 +// { dg-do compile { target c++11 } } + +template +struct [[nodiscard]] missiles {}; + +missiles make() { return {}; } +missiles (*fnptr)() = make; + +int main() +{ + make(); // { dg-warning "ignoring returned value of type" } + fnptr(); // { dg-warning "ignoring returned value of type" } +} --- gcc/testsuite/g++.dg/ext/attrib53.C.jj 2017-02-16 15:08:18.635107840 +0100 +++ gcc/testsuite/g++.dg/ext/attrib53.C 2017-02-16 15:09:50.210899761 +0100 @@ -0,0 +1,21 @@ +// { dg-do compile { target c++11 } } + +inline namespace N __attribute__((__abi_tag__ ("foo"))) {} +template struct A; +namespace N { +template class B; +} +template class C {}; +template struct D { + template using G = C<_Up>; +}; +template struct F { + template struct H { +typedef typename D::template G I; + }; +}; +template > struct J { + C> L; + typedef F ::H>::I M; + J *a; +}; --- gcc/testsuite/g++.dg/cpp0x/attrib54.C.jj2017-02-16 15:08:21.739066892 +0100 +++ gcc/testsuite/g++.dg/cpp0x/attrib54.C 2017-02-16 15:10:02.461738146 +0100 @@ -0,0 +1,21 @@ +// {
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On Wed, Feb 15, 2017 at 03:19:23PM -0500, Jason Merrill wrote: > OK. > > > 2017-02-15 Jakub Jelinek> > > > PR c++/79502 > > * pt.c (apply_late_template_attributes): If there are > > no dependent attributes, set *p to attributes. > > > > * g++.dg/cpp1z/nodiscard4.C: New test. Unfortunately it broke bootstrap, *p can already contain some other attributes (like abi_tag) and then we ICE in apply_identity_attributes. Note the attrib54.C testcase ICEs even on vanilla trunk, so if there are any dependent attribute, it is preexisting issue. The following patch bootstrapped successfully, but I see +FAIL: g++.dg/ext/vector32.C -std=c++* (internal compiler error) +FAIL: g++.dg/ext/vector32a.C -std=c++* (internal compiler error) +FAIL: g++.dg/gomp/declare-simd-1.C -std=gnu++* (internal compiler error) +FAIL: g++.dg/gomp/declare-simd-1.C -std=gnu++* (test for excess errors) +FAIL: g++.dg/lto/20091219 cp_lto_20091219_0.o assemble, -O3 -flto (internal compiler error) +FAIL: g++.dg/vect/simd-clone-4.cc -std=c++* (test for excess errors) +FAIL: g++.dg/vect/simd-clone-5.cc -std=c++* (test for excess errors) on i686-linux (x86_64-linux regtest still running). So there are further issues. 2017-02-16 Jakub Jelinek PR c++/79502 * pt.c (apply_late_template_attributes): If there are no dependent attributes, set *p to attributes. If there were some attributes in *p previously with or without dependent attributes, chain them after the new attributes. * g++.dg/cpp1z/nodiscard4.C: New test. * g++.dg/ext/attrib53.C: New test. * g++.dg/ext/attrib54.C: New test. * g++.dg/ext/attrib55.C: New test. --- gcc/cp/pt.c.jj 2017-02-16 12:00:20.044455757 +0100 +++ gcc/cp/pt.c 2017-02-16 15:07:31.678727294 +0100 @@ -10094,6 +10094,7 @@ apply_late_template_attributes (tree *de { tree late_attrs = NULL_TREE; tree *q = _attrs; + tree prev = *p; for (*p = attributes; *p; ) { @@ -10110,9 +10111,14 @@ apply_late_template_attributes (tree *de else p = _CHAIN (t); } + *p = prev; cplus_decl_attributes (decl_p, late_attrs, attr_flags); } + else if (*p == NULL) +*p = attributes; + else if (attributes) +*p = chainon (copy_list (attributes), *p); } /* Perform (or defer) access check for typedefs that were referenced --- gcc/testsuite/g++.dg/cpp1z/nodiscard4.C.jj 2017-02-16 14:04:22.602748039 +0100 +++ gcc/testsuite/g++.dg/cpp1z/nodiscard4.C 2017-02-16 14:04:22.601748053 +0100 @@ -0,0 +1,14 @@ +// PR c++/79502 +// { dg-do compile { target c++11 } } + +template +struct [[nodiscard]] missiles {}; + +missiles make() { return {}; } +missiles (*fnptr)() = make; + +int main() +{ + make(); // { dg-warning "ignoring returned value of type" } + fnptr(); // { dg-warning "ignoring returned value of type" } +} --- gcc/testsuite/g++.dg/ext/attrib53.C.jj 2017-02-16 15:08:18.635107840 +0100 +++ gcc/testsuite/g++.dg/ext/attrib53.C 2017-02-16 15:09:50.210899761 +0100 @@ -0,0 +1,21 @@ +// { dg-do compile { target c++11 } } + +inline namespace N __attribute__((__abi_tag__ ("foo"))) {} +template struct A; +namespace N { +template class B; +} +template class C {}; +template struct D { + template using G = C<_Up>; +}; +template struct F { + template struct H { +typedef typename D::template G I; + }; +}; +template > struct J { + C> L; + typedef F ::H>::I M; + J *a; +}; --- gcc/testsuite/g++.dg/cpp0x/attrib54.C.jj2017-02-16 15:08:21.739066892 +0100 +++ gcc/testsuite/g++.dg/cpp0x/attrib54.C 2017-02-16 15:10:02.461738146 +0100 @@ -0,0 +1,21 @@ +// { dg-do compile { target c++11 } } + +inline namespace N __attribute__((__abi_tag__ ("foo"))) {} +template struct A {}; +namespace N { +template class B {}; +} +template class __attribute__((__aligned__ (sizeof (T C {}; +template struct D { + template using G = C<_Up>; +}; +template struct F { + template struct H { +typedef typename D::template G I; + }; +}; +template > struct J { + C> L; + typedef F ::H>::I M; + J *a; +}; --- gcc/testsuite/g++.dg/cpp0x/attrib55.C.jj2017-02-16 15:08:26.801000114 +0100 +++ gcc/testsuite/g++.dg/cpp0x/attrib55.C 2017-02-16 15:10:11.422619933 +0100 @@ -0,0 +1,21 @@ +// { dg-do compile { target c++11 } } + +inline namespace N __attribute__((__abi_tag__ ("foo"))) {} +template struct A {}; +namespace N { +template class B {}; +} +template class __attribute__((__unused__)) C {}; +template struct D { + template using G = C<_Up>; +}; +template struct F { + template struct H { +typedef typename D::template G I; + }; +}; +template > struct J { + C> L; + typedef F ::H>::I M; + J *a; +}; Jakub
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
OK. On Wed, Feb 15, 2017 at 12:30 PM, Jakub Jelinekwrote: > On Wed, Feb 15, 2017 at 11:56:30AM -0500, Jason Merrill wrote: >> On Tue, Feb 14, 2017 at 3:13 PM, Jakub Jelinek wrote: >> > The following testcase fails, because while we have the nodiscard >> > attribute on the template, we actually never propagate it to the >> > instantiation, which is where it is checked (I'm really surprised about >> > this). >> >> Normally we propagate attributes when instantiating the class; see the >> call to apply_late_template_attributes in >> instantiate_class_template_1. I'm not sure why that wouldn't be > > Yes, instantiate_class_template_1 calls apply_late_template_attributes, > but that actually does nothing if there are no dependent attributes. > If there are any, it sets TYPE_ATTRIBUTES (or DECL_ATTRIBUTES) to > a copy of the attributes list, removes all the dependent attributes > from there and calls cplus_decl_attributes on the late attrs (after > tsubst_attribute them). So setting {TYPE,DECL}_ATTRIBUTES to the > attributes list unmodified if there are no dependent ones matches > the behavior for non-dependent ones if there is at least one dependent. > > So, does the following patch look better? > > 2017-02-15 Jakub Jelinek > > PR c++/79502 > * pt.c (apply_late_template_attributes): If there are > no dependent attributes, set *p to attributes. > > * g++.dg/cpp1z/nodiscard4.C: New test. > > --- gcc/cp/pt.c.jj 2017-02-14 09:23:49.0 +0100 > +++ gcc/cp/pt.c 2017-02-15 18:21:45.581055915 +0100 > @@ -10113,6 +10113,8 @@ apply_late_template_attributes (tree *de > >cplus_decl_attributes (decl_p, late_attrs, attr_flags); > } > + else > +*p = attributes; > } > > /* Perform (or defer) access check for typedefs that were referenced > --- gcc/testsuite/g++.dg/cpp1z/nodiscard4.C.jj 2017-02-15 18:11:33.281135469 > +0100 > +++ gcc/testsuite/g++.dg/cpp1z/nodiscard4.C 2017-02-15 18:11:33.281135469 > +0100 > @@ -0,0 +1,14 @@ > +// PR c++/79502 > +// { dg-do compile { target c++11 } } > + > +template > +struct [[nodiscard]] missiles {}; > + > +missiles make() { return {}; } > +missiles (*fnptr)() = make; > + > +int main() > +{ > + make(); // { dg-warning "ignoring returned value of type" } > + fnptr(); // { dg-warning "ignoring returned value of type" } > +} > > > Jakub
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On Wed, Feb 15, 2017 at 11:56:30AM -0500, Jason Merrill wrote: > On Tue, Feb 14, 2017 at 3:13 PM, Jakub Jelinekwrote: > > The following testcase fails, because while we have the nodiscard > > attribute on the template, we actually never propagate it to the > > instantiation, which is where it is checked (I'm really surprised about > > this). > > Normally we propagate attributes when instantiating the class; see the > call to apply_late_template_attributes in > instantiate_class_template_1. I'm not sure why that wouldn't be Yes, instantiate_class_template_1 calls apply_late_template_attributes, but that actually does nothing if there are no dependent attributes. If there are any, it sets TYPE_ATTRIBUTES (or DECL_ATTRIBUTES) to a copy of the attributes list, removes all the dependent attributes from there and calls cplus_decl_attributes on the late attrs (after tsubst_attribute them). So setting {TYPE,DECL}_ATTRIBUTES to the attributes list unmodified if there are no dependent ones matches the behavior for non-dependent ones if there is at least one dependent. So, does the following patch look better? 2017-02-15 Jakub Jelinek PR c++/79502 * pt.c (apply_late_template_attributes): If there are no dependent attributes, set *p to attributes. * g++.dg/cpp1z/nodiscard4.C: New test. --- gcc/cp/pt.c.jj 2017-02-14 09:23:49.0 +0100 +++ gcc/cp/pt.c 2017-02-15 18:21:45.581055915 +0100 @@ -10113,6 +10113,8 @@ apply_late_template_attributes (tree *de cplus_decl_attributes (decl_p, late_attrs, attr_flags); } + else +*p = attributes; } /* Perform (or defer) access check for typedefs that were referenced --- gcc/testsuite/g++.dg/cpp1z/nodiscard4.C.jj 2017-02-15 18:11:33.281135469 +0100 +++ gcc/testsuite/g++.dg/cpp1z/nodiscard4.C 2017-02-15 18:11:33.281135469 +0100 @@ -0,0 +1,14 @@ +// PR c++/79502 +// { dg-do compile { target c++11 } } + +template +struct [[nodiscard]] missiles {}; + +missiles make() { return {}; } +missiles (*fnptr)() = make; + +int main() +{ + make(); // { dg-warning "ignoring returned value of type" } + fnptr(); // { dg-warning "ignoring returned value of type" } +} Jakub
Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)
On Tue, Feb 14, 2017 at 3:13 PM, Jakub Jelinekwrote: > The following testcase fails, because while we have the nodiscard > attribute on the template, we actually never propagate it to the > instantiation, which is where it is checked (I'm really surprised about > this). Normally we propagate attributes when instantiating the class; see the call to apply_late_template_attributes in instantiate_class_template_1. I'm not sure why that wouldn't be happening here; are we calling maybe_warn_nodiscard before instantiating the class? > Unfortunately, this patch regresses > FAIL: g++.dg/ext/visibility/template8.C -std=gnu++{11,14,98} scan-hidden > hidden[ \\t_]*_Z1gI1AI1BEEvT_ > It expects that the visibility attribute from the template never > makes it to the implementation or something, is that correct? Or do > we need to handle visibility in some special way? Visibility is handled specially; determine_visibility looks up the visibility of the template if the specialization doesn't specify its own visibility. > Regarding the first hunk, it is just a wild guess, I couldn't trigger > that code by make check-c++-all. Is there a way to get it through > some partial instantiation of scoped enum with/without attributes or > something similar? Hmm, not sure. > Anyway, except for that template8.C the patch passed bootstrap/regtest > on x86_64-linux and i686-linux. But it really puzzles me that the > attributes aren't instantiated, what happens e.g. with abi_tag > attribute? abi_tag and may_alias are applied specifically in lookup_class_template_1. Jason
[C++ RFC] Fix up attribute handling in templates (PR c++/79502)
Hi! The following testcase fails, because while we have the nodiscard attribute on the template, we actually never propagate it to the instantiation, which is where it is checked (I'm really surprised about this). Unfortunately, this patch regresses FAIL: g++.dg/ext/visibility/template8.C -std=gnu++{11,14,98} scan-hidden hidden[ \\t_]*_Z1gI1AI1BEEvT_ It expects that the visibility attribute from the template never makes it to the implementation or something, is that correct? Or do we need to handle visibility in some special way? Regarding the first hunk, it is just a wild guess, I couldn't trigger that code by make check-c++-all. Is there a way to get it through some partial instantiation of scoped enum with/without attributes or something similar? Anyway, except for that template8.C the patch passed bootstrap/regtest on x86_64-linux and i686-linux. But it really puzzles me that the attributes aren't instantiated, what happens e.g. with abi_tag attribute? 2017-02-14 Jakub JelinekPR c++/79502 * pt.c (lookup_template_class_1): Set TYPE_ATTRIBUTES on class instantiations as well as dependent enumeral instantiations. Set ENUM_UNDERLYING_TYPE on the latter too. * g++.dg/cpp1z/nodiscard4.C: New test. --- gcc/cp/pt.c.jj 2017-02-10 21:35:30.0 +0100 +++ gcc/cp/pt.c 2017-02-14 08:36:35.459265103 +0100 @@ -8759,7 +8759,13 @@ lookup_template_class_1 (tree d1, tree a template parameters. And, no one should be interested in the enumeration constants for such a type. */ t = cxx_make_type (ENUMERAL_TYPE); + ENUM_UNDERLYING_TYPE (t) + = tsubst (ENUM_UNDERLYING_TYPE (template_type), + arglist, complain, in_decl); SET_SCOPED_ENUM_P (t, SCOPED_ENUM_P (template_type)); + TYPE_ATTRIBUTES (t) + = tsubst_attributes (TYPE_ATTRIBUTES (template_type), +arglist, complain, in_decl); } SET_OPAQUE_ENUM_P (t, OPAQUE_ENUM_P (template_type)); ENUM_FIXED_UNDERLYING_TYPE_P (t) @@ -8786,6 +8792,10 @@ lookup_template_class_1 (tree d1, tree a equality testing, so this template class requires structural equality testing. */ SET_TYPE_STRUCTURAL_EQUALITY (t); + + TYPE_ATTRIBUTES (t) + = tsubst_attributes (TYPE_ATTRIBUTES (template_type), +arglist, complain, in_decl); } else gcc_unreachable (); --- gcc/testsuite/g++.dg/cpp1z/nodiscard4.C.jj 2017-02-14 08:42:12.275765748 +0100 +++ gcc/testsuite/g++.dg/cpp1z/nodiscard4.C 2017-02-14 08:40:00.0 +0100 @@ -0,0 +1,14 @@ +// PR c++/79502 +// { dg-do compile { target c++11 } } + +template +struct [[nodiscard]] missiles {}; + +missiles make() { return {}; } +missiles (*fnptr)() = make; + +int main() +{ + make(); // { dg-warning "ignoring returned value of type" } + fnptr(); // { dg-warning "ignoring returned value of type" } +} Jakub