Re: [C++ RFC] Fix up attribute handling in templates (PR c++/79502)

2017-02-19 Thread Martin Sebor 

On 02/17/2017 09:53 PM, Jason Merrill wrote:

On Thu, Feb 16, 2017 at 6:13 PM, Martin Sebor  wrote:

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)

2017-02-17 Thread Jason Merrill 
On Thu, Feb 16, 2017 at 6:13 PM, Martin Sebor  wrote:
> 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)

2017-02-16 Thread Martin Sebor 

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 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)

2017-02-16 Thread Jason Merrill 
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.
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)

2017-02-16 Thread Jakub Jelinek 
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 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
@@ -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)

2017-02-16 Thread Jakub Jelinek 
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)

2017-02-15 Thread Jason Merrill 
OK.

On Wed, Feb 15, 2017 at 12:30 PM, Jakub Jelinek  wrote:
> 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)

2017-02-15 Thread Jakub Jelinek 
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)

2017-02-15 Thread Jason Merrill 
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
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)

2017-02-14 Thread Jakub Jelinek 
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 Jelinek  

PR 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