Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Jason Merrill]

On 01/20/2016 06:04 PM, Martin Sebor wrote:

Right.  The problem is this code in is_late_template_attribute:


  /* If the first attribute argument is an identifier, only consider
 second and following arguments.  Attributes like mode, format,
 cleanup and several target specific attributes aren't late
 just because they have an IDENTIFIER_NODE as first
argument.  */
  if (arg == args && identifier_p (t))
continue;


It shouldn't skip an initial identifier if !attribute_takes_identifier_p.


That seems backwards. I expected attribute_takes_identifier_p()
to return true for attribute aligned since the attribute does
take one.


There are some attributes (mode, format, cleanup) that have magic 
handling of identifiers; aligned treats its argument as an expression 
whether or not that expression takes the form of an identifier.



In any case, I changed the patch as you suggest and retested it
on x86_64.  I saw the email about stage 3 having ended but I'm
not sure it applies to changes that are still in progress.


I wouldn't think so; certainly not for something this simple.  The patch 
is OK.


Jason

Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Martin Sebor]

Right.  The problem is this code in is_late_template_attribute:


  /* If the first attribute argument is an identifier, only consider
 second and following arguments.  Attributes like mode, format,
 cleanup and several target specific attributes aren't late
 just because they have an IDENTIFIER_NODE as first argument.  */
  if (arg == args && identifier_p (t))
continue;


It shouldn't skip an initial identifier if !attribute_takes_identifier_p.


That seems backwards. I expected attribute_takes_identifier_p()
to return true for attribute aligned since the attribute does
take one.

In any case, I changed the patch as you suggest and retested it
on x86_64.  I saw the email about stage 3 having ended but I'm
not sure it applies to changes that are still in progress.

Martin
gcc/testsuite/ChangeLog:
2016-01-20  Martin Sebor  

	PR c++/58109
	PR c++/69022
	* g++.dg/cpp0x/alignas5.C: New test.
	* g++.dg/ext/vector29.C: Same.

gcc/cp/ChangeLog:
2016-01-20  Martin Sebor  

	PR c++/58109
	PR c++/69022
	* decl2.c (is_late_template_attribute): Handle dependent argument
	to attribute align and attribute vector_size.

diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index a7212ca0..7d68961 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1193,7 +1193,8 @@ is_late_template_attribute (tree attr, tree decl)
 	 second and following arguments.  Attributes like mode, format,
 	 cleanup and several target specific attributes aren't late
 	 just because they have an IDENTIFIER_NODE as first argument.  */
-  if (arg == args && identifier_p (t))
+  if (arg == args && attribute_takes_identifier_p (name)
+	  && identifier_p (t))
 	continue;
 
   if (value_dependent_expression_p (t)
diff --git a/gcc/testsuite/g++.dg/cpp0x/alignas5.C b/gcc/testsuite/g++.dg/cpp0x/alignas5.C
new file mode 100644
index 000..2dcc41f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alignas5.C
@@ -0,0 +1,45 @@
+// PR c++/58109  - alignas() fails to compile with constant expression
+// { dg-do compile }
+
+template 
+struct Base {
+  static const int Align = sizeof (T);
+};
+
+// Never instantiated.
+template 
+struct Derived: Base
+{
+#if __cplusplus >= 201102L
+  // This is the meat of the (simplified) regression test for c++/58109.
+  using B = Base;
+  using B::Align;
+
+  alignas (Align) char a [1];
+  alignas (Align) T b [1];
+#else
+  // Fake the test for C++ 98.
+#  define Align Base::Align
+#endif
+
+  char __attribute__ ((aligned (Align))) c [1];
+  T __attribute__ ((aligned (Align))) d [1];
+};
+
+// Instantiated to verify that the code is accepted even when instantiated.
+template 
+struct InstDerived: Base
+{
+#if __cplusplus >= 201102L
+  using B = Base;
+  using B::Align;
+
+  alignas (Align) char a [1];
+  alignas (Align) T b [1];
+#endif
+
+  char __attribute__ ((aligned (Align))) c [1];
+  T __attribute__ ((aligned (Align))) d [1];
+};
+
+InstDerived dx;
diff --git a/gcc/testsuite/g++.dg/ext/vector29.C b/gcc/testsuite/g++.dg/ext/vector29.C
new file mode 100644
index 000..4a13009
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/vector29.C
@@ -0,0 +1,53 @@
+// PR c++/69022 - attribute vector_size ignored with dependent bytes
+// { dg-do compile }
+
+template 
+struct A { static const int X = N; };
+
+#if __cplusplus >= 201202L
+#  define ASSERT(e) static_assert (e, #e)
+#else
+#  define ASSERT(e)   \
+  do { struct S { bool: !!(e); } asrt; (void) } while (0)
+#endif
+
+template 
+struct B: A
+{
+#if __cplusplus >= 201202L
+  using A::X;
+#  define VecSize X
+#else
+#  define VecSize A::X
+#endif
+
+static void foo ()
+{
+char a __attribute__ ((vector_size (N)));
+ASSERT (sizeof a == N);
+
+T b __attribute__ ((vector_size (N)));
+ASSERT (sizeof b == N);
+}
+
+static void bar ()
+{
+char c1 __attribute__ ((vector_size (VecSize)));
+ASSERT (sizeof c1 == VecSize);
+
+char c2 __attribute__ ((vector_size (A::X)));
+ASSERT (sizeof c2 == A::X);
+
+T d1 __attribute__ ((vector_size (VecSize)));
+ASSERT (sizeof d1 == VecSize);
+
+T d2 __attribute__ ((vector_size (A::X)));
+ASSERT (sizeof d2 == A::X);
+}
+};
+
+void bar ()
+{
+B::foo ();
+B::bar ();
+}

Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Jason Merrill]

On 01/12/2016 01:11 PM, Martin Sebor wrote:

On 01/11/2016 10:20 PM, Jason Merrill wrote:

On 12/22/2015 09:32 PM, Martin Sebor wrote:

+  if (is_attribute_p ("aligned", name)
+  || is_attribute_p ("vector_size", name))
+{
+  /* Attribute argument may be a dependent indentifier.  */
+  if (tree t = args ? TREE_VALUE (args) : NULL_TREE)
+if (value_dependent_expression_p (t)
+|| type_dependent_expression_p (t))
+  return true;
+}


Instead of this, is_late_template_attribute should be fixed to check
attribute_takes_identifier_p.


attribute_takes_identifier_p() returns false for the aligned
attribute and for vector_size (it returns true only for
attributes cleanup, format, and mode, and none others).


Right.  The problem is this code in is_late_template_attribute:


  /* If the first attribute argument is an identifier, only consider
 second and following arguments.  Attributes like mode, format,
 cleanup and several target specific attributes aren't late
 just because they have an IDENTIFIER_NODE as first argument.  */
  if (arg == args && identifier_p (t))
continue;


It shouldn't skip an initial identifier if !attribute_takes_identifier_p.

Jason

Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Martin Sebor]

On 01/12/2016 11:11 AM, Martin Sebor wrote:

On 01/11/2016 10:20 PM, Jason Merrill wrote:

On 12/22/2015 09:32 PM, Martin Sebor wrote:

+  if (is_attribute_p ("aligned", name)
+  || is_attribute_p ("vector_size", name))
+{
+  /* Attribute argument may be a dependent indentifier.  */
+  if (tree t = args ? TREE_VALUE (args) : NULL_TREE)
+if (value_dependent_expression_p (t)
+|| type_dependent_expression_p (t))
+  return true;
+}


Instead of this, is_late_template_attribute should be fixed to check
attribute_takes_identifier_p.


attribute_takes_identifier_p() returns false for the aligned
attribute and for vector_size (it returns true only for
attributes cleanup, format, and mode, and none others).

Are you suggesting to also change attribute_takes_identifier_p
to return true for these attributes?  That would likely mean
changes to the C front end as well.)


Jason, can you please clarify what you had in mind?  I realize this
isn't as severe as a codegen problem but I'd like to try to wrap it
up in between higher priority tasks.



Thanks
Martin

Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Martin Sebor]

On 01/11/2016 10:20 PM, Jason Merrill wrote:

On 12/22/2015 09:32 PM, Martin Sebor wrote:

+  if (is_attribute_p ("aligned", name)
+  || is_attribute_p ("vector_size", name))
+{
+  /* Attribute argument may be a dependent indentifier.  */
+  if (tree t = args ? TREE_VALUE (args) : NULL_TREE)
+if (value_dependent_expression_p (t)
+|| type_dependent_expression_p (t))
+  return true;
+}


Instead of this, is_late_template_attribute should be fixed to check
attribute_takes_identifier_p.


attribute_takes_identifier_p() returns false for the aligned
attribute and for vector_size (it returns true only for
attributes cleanup, format, and mode, and none others).

Are you suggesting to also change attribute_takes_identifier_p
to return true for these attributes?  That would likely mean
changes to the C front end as well.)

Thanks
Martin

PING #2 [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Martin Sebor]

Ping:
https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02074.html

On 01/04/2016 09:49 PM, Martin Sebor wrote:

Ping: looking for review/approval of the patch below:
   https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02074.html

Thanks
Martin

On 12/22/2015 07:32 PM, Martin Sebor wrote:

The attached patch adds handling of dependent arguments to
attribute aligned and attribute vector_size, fixing c++/58109
and 69022 - attribute vector_size ignored with dependent bytes.

Tested on x86_64.

Martin

Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Jason Merrill]

On 12/22/2015 09:32 PM, Martin Sebor wrote:

+  if (is_attribute_p ("aligned", name)
+  || is_attribute_p ("vector_size", name))
+{
+  /* Attribute argument may be a dependent indentifier.  */
+  if (tree t = args ? TREE_VALUE (args) : NULL_TREE)
+   if (value_dependent_expression_p (t)
+   || type_dependent_expression_p (t))
+ return true;
+}


Instead of this, is_late_template_attribute should be fixed to check 
attribute_takes_identifier_p.


Jason

Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Martin Sebor]

Ping: looking for review/approval of the patch below:
  https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02074.html

Thanks
Martin

On 12/22/2015 07:32 PM, Martin Sebor wrote:

The attached patch adds handling of dependent arguments to
attribute aligned and attribute vector_size, fixing c++/58109
and 69022 - attribute vector_size ignored with dependent bytes.

Tested on x86_64.

Martin

[PATCH] c++/58109 - alignas() fails to compile with constant expression

[Martin Sebor]

The attached patch adds handling of dependent arguments to
attribute aligned and attribute vector_size, fixing c++/58109
and 69022 - attribute vector_size ignored with dependent bytes.

Tested on x86_64.

Martin
gcc/testsuite/ChangeLog:
2015-12-22  Martin Sebor  

	PR c++/58109
	PR c++/69022
	* g++.dg/cpp0x/alignas5.C: New test.
	* g++.dg/ext/vector29.C: Same.

gcc/cp/ChangeLog:
2015-12-22  Martin Sebor  

	PR c++/58109
	PR c++/69022
	* decl2.c (is_late_template_attribute): Handle dependent argument
	to attribute align and attribute vector_size.

Index: gcc/cp/decl2.c
===
--- gcc/cp/decl2.c	(revision 231903)
+++ gcc/cp/decl2.c	(working copy)
@@ -1183,6 +1183,16 @@
   if (args && PACK_EXPANSION_P (args))
 return true;
 
+  if (is_attribute_p ("aligned", name)
+  || is_attribute_p ("vector_size", name))
+{
+  /* Attribute argument may be a dependent indentifier.  */
+  if (tree t = args ? TREE_VALUE (args) : NULL_TREE)
+	if (value_dependent_expression_p (t)
+	|| type_dependent_expression_p (t))
+	  return true;
+}
+  
   /* If any of the arguments are dependent expressions, we can't evaluate
  the attribute until instantiation time.  */
   for (arg = args; arg; arg = TREE_CHAIN (arg))
Index: gcc/testsuite/g++.dg/cpp0x/alignas5.C
===
--- gcc/testsuite/g++.dg/cpp0x/alignas5.C	(revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/alignas5.C	(working copy)
@@ -0,0 +1,45 @@
+// PR c++/58109  - alignas() fails to compile with constant expression
+// { dg-do compile }
+
+template 
+struct Base {
+  static const int Align = sizeof (T);
+};
+
+// Never instantiated.
+template 
+struct Derived: Base
+{
+#if __cplusplus >= 201102L
+  // This is the meat of the (simplified) regression test for c++/58109.
+  using B = Base;
+  using B::Align;
+
+  alignas (Align) char a [1];
+  alignas (Align) T b [1];
+#else
+  // Fake the test for C++ 98.
+#  define Align Base::Align
+#endif
+
+  char __attribute__ ((aligned (Align))) c [1];
+  T __attribute__ ((aligned (Align))) d [1];
+};
+
+// Instantiated to verify that the code is accepted even when instantiated.
+template 
+struct InstDerived: Base
+{
+#if __cplusplus >= 201102L
+  using B = Base;
+  using B::Align;
+
+  alignas (Align) char a [1];
+  alignas (Align) T b [1];
+#endif
+
+  char __attribute__ ((aligned (Align))) c [1];
+  T __attribute__ ((aligned (Align))) d [1];
+};
+
+InstDerived dx;
Index: gcc/testsuite/g++.dg/ext/vector29.C
===
--- gcc/testsuite/g++.dg/ext/vector29.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/vector29.C	(working copy)
@@ -0,0 +1,53 @@
+// PR c++/69022 - attribute vector_size ignored with dependent bytes
+// { dg-do compile }
+
+template 
+struct A { static const int X = N; };
+
+#if __cplusplus >= 201202L
+#  define ASSERT(e) static_assert (e, #e)
+#else
+#  define ASSERT(e)   \
+  do { struct S { bool: !!(e); } asrt; (void) } while (0)
+#endif
+
+template 
+struct B: A
+{
+#if __cplusplus >= 201202L
+  using A::X;
+#  define VecSize X
+#else
+#  define VecSize A::X
+#endif
+
+static void foo ()
+{
+char a __attribute__ ((vector_size (N)));
+ASSERT (sizeof a == N);
+
+T b __attribute__ ((vector_size (N)));
+ASSERT (sizeof b == N);
+}
+
+static void bar ()
+{
+char c1 __attribute__ ((vector_size (VecSize)));
+ASSERT (sizeof c1 == VecSize);
+
+char c2 __attribute__ ((vector_size (A::X)));
+ASSERT (sizeof c2 == A::X);
+
+T d1 __attribute__ ((vector_size (VecSize)));
+ASSERT (sizeof d1 == VecSize);
+
+T d2 __attribute__ ((vector_size (A::X)));
+ASSERT (sizeof d2 == A::X);
+}
+};
+
+void bar ()
+{
+B::foo ();
+B::bar ();
+}

Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression

[Marc Glisse]

On Tue, 22 Dec 2015, Martin Sebor wrote:


The attached patch adds handling of dependent arguments to
attribute aligned and attribute vector_size, fixing c++/58109
and 69022 - attribute vector_size ignored with dependent bytes.


In the longer term, Gaby used to suggest that internally we should 
represent the vector_size attribute as some kind of template 
__vector (half-way between a template class and a template 
alias). The goal would be to avoid duplicating all the logic from 
templates into attributes.


Of course that's much more work (even assuming that there isn't some big 
road-block, which there might be), and a little bit more code duplication 
as in your patch seems appropriate at this stage.


--
Marc Glisse