* [PATCH] c++/58109 - alignas() fails to compile with constant expression
@ 2015-12-23 2:32 Martin Sebor
2015-12-23 7:53 ` Marc Glisse
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Martin Sebor @ 2015-12-23 2:32 UTC (permalink / raw)
To: Gcc Patch List; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 212 bytes --]
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
[-- Attachment #2: gcc-58109-69022.patch --]
[-- Type: text/x-patch, Size: 3838 bytes --]
gcc/testsuite/ChangeLog:
2015-12-22 Martin Sebor <msebor@redhat.com>
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 <msebor@redhat.com>
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 <typename T>
+struct Base {
+ static const int Align = sizeof (T);
+};
+
+// Never instantiated.
+template <typename T>
+struct Derived: Base<T>
+{
+#if __cplusplus >= 201102L
+ // This is the meat of the (simplified) regression test for c++/58109.
+ using B = Base<T>;
+ using B::Align;
+
+ alignas (Align) char a [1];
+ alignas (Align) T b [1];
+#else
+ // Fake the test for C++ 98.
+# define Align Base<T>::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 <typename T>
+struct InstDerived: Base<T>
+{
+#if __cplusplus >= 201102L
+ using B = Base<T>;
+ 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<int> 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 <int N>
+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)&asrt; } while (0)
+#endif
+
+template <class T, int N>
+struct B: A<N>
+{
+#if __cplusplus >= 201202L
+ using A<N>::X;
+# define VecSize X
+#else
+# define VecSize A<N>::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<N>::X)));
+ ASSERT (sizeof c2 == A<N>::X);
+
+ T d1 __attribute__ ((vector_size (VecSize)));
+ ASSERT (sizeof d1 == VecSize);
+
+ T d2 __attribute__ ((vector_size (A<N>::X)));
+ ASSERT (sizeof d2 == A<N>::X);
+ }
+};
+
+void bar ()
+{
+ B<int, 16>::foo ();
+ B<int, 16>::bar ();
+}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2015-12-23 2:32 [PATCH] c++/58109 - alignas() fails to compile with constant expression Martin Sebor
@ 2015-12-23 7:53 ` Marc Glisse
2016-01-05 4:49 ` Martin Sebor
2016-01-12 5:20 ` Jason Merrill
2 siblings, 0 replies; 10+ messages in thread
From: Marc Glisse @ 2015-12-23 7:53 UTC (permalink / raw)
To: Martin Sebor; +Cc: Gcc Patch List, Jason Merrill
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<scalar,size> (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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2015-12-23 2:32 [PATCH] c++/58109 - alignas() fails to compile with constant expression Martin Sebor
2015-12-23 7:53 ` Marc Glisse
@ 2016-01-05 4:49 ` Martin Sebor
2016-01-12 0:46 ` PING #2 " Martin Sebor
2016-01-12 5:20 ` Jason Merrill
2 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2016-01-05 4:49 UTC (permalink / raw)
To: Gcc Patch List; +Cc: Jason Merrill
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* PING #2 [PATCH] c++/58109 - alignas() fails to compile with constant expression
2016-01-05 4:49 ` Martin Sebor
@ 2016-01-12 0:46 ` Martin Sebor
0 siblings, 0 replies; 10+ messages in thread
From: Martin Sebor @ 2016-01-12 0:46 UTC (permalink / raw)
To: Gcc Patch List; +Cc: Jason Merrill
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2015-12-23 2:32 [PATCH] c++/58109 - alignas() fails to compile with constant expression Martin Sebor
2015-12-23 7:53 ` Marc Glisse
2016-01-05 4:49 ` Martin Sebor
@ 2016-01-12 5:20 ` Jason Merrill
2016-01-12 18:11 ` Martin Sebor
2 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2016-01-12 5:20 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2016-01-12 5:20 ` Jason Merrill
@ 2016-01-12 18:11 ` Martin Sebor
2016-01-15 17:39 ` Martin Sebor
2016-01-18 16:11 ` Jason Merrill
0 siblings, 2 replies; 10+ messages in thread
From: Martin Sebor @ 2016-01-12 18:11 UTC (permalink / raw)
To: Jason Merrill, Gcc Patch List
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2016-01-12 18:11 ` Martin Sebor
@ 2016-01-15 17:39 ` Martin Sebor
2016-01-18 16:11 ` Jason Merrill
1 sibling, 0 replies; 10+ messages in thread
From: Martin Sebor @ 2016-01-15 17:39 UTC (permalink / raw)
To: Jason Merrill, Gcc Patch List
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2016-01-12 18:11 ` Martin Sebor
2016-01-15 17:39 ` Martin Sebor
@ 2016-01-18 16:11 ` Jason Merrill
2016-01-20 23:04 ` Martin Sebor
1 sibling, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2016-01-18 16:11 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2016-01-18 16:11 ` Jason Merrill
@ 2016-01-20 23:04 ` Martin Sebor
2016-01-22 20:41 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2016-01-20 23:04 UTC (permalink / raw)
To: Jason Merrill, Gcc Patch List
[-- Attachment #1: Type: text/plain, Size: 844 bytes --]
> 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
[-- Attachment #2: gcc-58109.patch --]
[-- Type: text/x-patch, Size: 3604 bytes --]
gcc/testsuite/ChangeLog:
2016-01-20 Martin Sebor <msebor@redhat.com>
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 <msebor@redhat.com>
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 0000000..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 <typename T>
+struct Base {
+ static const int Align = sizeof (T);
+};
+
+// Never instantiated.
+template <typename T>
+struct Derived: Base<T>
+{
+#if __cplusplus >= 201102L
+ // This is the meat of the (simplified) regression test for c++/58109.
+ using B = Base<T>;
+ using B::Align;
+
+ alignas (Align) char a [1];
+ alignas (Align) T b [1];
+#else
+ // Fake the test for C++ 98.
+# define Align Base<T>::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 <typename T>
+struct InstDerived: Base<T>
+{
+#if __cplusplus >= 201102L
+ using B = Base<T>;
+ 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<int> dx;
diff --git a/gcc/testsuite/g++.dg/ext/vector29.C b/gcc/testsuite/g++.dg/ext/vector29.C
new file mode 100644
index 0000000..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 <int N>
+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)&asrt; } while (0)
+#endif
+
+template <class T, int N>
+struct B: A<N>
+{
+#if __cplusplus >= 201202L
+ using A<N>::X;
+# define VecSize X
+#else
+# define VecSize A<N>::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<N>::X)));
+ ASSERT (sizeof c2 == A<N>::X);
+
+ T d1 __attribute__ ((vector_size (VecSize)));
+ ASSERT (sizeof d1 == VecSize);
+
+ T d2 __attribute__ ((vector_size (A<N>::X)));
+ ASSERT (sizeof d2 == A<N>::X);
+ }
+};
+
+void bar ()
+{
+ B<int, 16>::foo ();
+ B<int, 16>::bar ();
+}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
2016-01-20 23:04 ` Martin Sebor
@ 2016-01-22 20:41 ` Jason Merrill
0 siblings, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2016-01-22 20:41 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-01-22 20:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-23 2:32 [PATCH] c++/58109 - alignas() fails to compile with constant expression Martin Sebor
2015-12-23 7:53 ` Marc Glisse
2016-01-05 4:49 ` Martin Sebor
2016-01-12 0:46 ` PING #2 " Martin Sebor
2016-01-12 5:20 ` Jason Merrill
2016-01-12 18:11 ` Martin Sebor
2016-01-15 17:39 ` Martin Sebor
2016-01-18 16:11 ` Jason Merrill
2016-01-20 23:04 ` Martin Sebor
2016-01-22 20:41 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).