Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Mon, May 12, 2014 at 03:44:42PM +, Joseph S. Myers wrote:
On Sun, 11 May 2014, Marek Polacek wrote:
FAIL: c-c++-common/pr50459.c -std=gnu++1y (test for excess errors)
FAIL: c-c++-common/pr50459.c -Wc++-compat (test for excess errors)
The errors are
/opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:8:1: error:
constructor priorities are not supported
/opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:9:1: error: destructor
priorities are not supported
Ah. The following untested patch should skip that test on Darwin.
Ok?
I don't think the whole test should be skipped for that issue; I think the
part requiring this feature should be split out into a separate testcase,
so that as much as possible is still tested on Darwin.
Yeah, I should've done that in the first place, sorry. Is the
following ok then?
2014-05-13 Marek Polacek pola...@redhat.com
* c-c++-common/pr50459.c: Move cdtor tests to a separate testcase.
* c-c++-common/pr50459-2.c: New test.
diff --git gcc/testsuite/c-c++-common/pr50459-2.c
gcc/testsuite/c-c++-common/pr50459-2.c
index e69de29..0e8fec3 100644
--- gcc/testsuite/c-c++-common/pr50459-2.c
+++ gcc/testsuite/c-c++-common/pr50459-2.c
@@ -0,0 +1,7 @@
+/* PR c/50459 */
+/* { dg-do compile { target init_priority } } */
+/* { dg-options -Wall -Wextra } */
+
+enum { A = 128, B = 1 };
+void fn3 (void) __attribute__((constructor (A)));
+void fn4 (void) __attribute__((destructor (A)));
diff --git gcc/testsuite/c-c++-common/pr50459.c
gcc/testsuite/c-c++-common/pr50459.c
index f837b63..8d75228 100644
--- gcc/testsuite/c-c++-common/pr50459.c
+++ gcc/testsuite/c-c++-common/pr50459.c
@@ -5,8 +5,6 @@
enum { A = 128, B = 1 };
void *fn1 (void) __attribute__((assume_aligned (A)));
void *fn2 (void) __attribute__((assume_aligned (A, 4)));
-void fn3 (void) __attribute__((constructor (A)));
-void fn4 (void) __attribute__((destructor (A)));
void *fn5 (int) __attribute__((alloc_size (B)));
void *fn6 (int) __attribute__((alloc_align (B)));
void fn7 (const char *, ...) __attribute__ ((sentinel (B)));
Marek
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Tue, 13 May 2014, Marek Polacek wrote: Yeah, I should've done that in the first place, sorry. Is the following ok then? 2014-05-13 Marek Polacek pola...@redhat.com * c-c++-common/pr50459.c: Move cdtor tests to a separate testcase. * c-c++-common/pr50459-2.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
Marek Polacek pola...@redhat.com writes: On Sun, May 11, 2014 at 09:18:47PM +0200, Rainer Orth wrote: No, that's wrong: avoid hardcoding target lists if at all possible. Besides, it's wrong since it doesn't cover the Solaris (and other non-gld linker) case. Use the init_priority effective-target keyword instead. Also, please check if you can use dg-xfail-if instead: if anything changes, the test turns into an XPASS instead of the change going unnoticed with dg-skip-if. I don't see tests using dg-skip-if and init_priority, so this patch No need for examples: as you can see in doc/sourcebuild.texi, both dg-do and dg-skip-if accept the same selectors. does what we do for other tests using cdtor priorities. 2014-05-11 Marek Polacek pola...@redhat.com * c-c++-common/pr50459.c: Require init_priority target. Ok. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Sun, 11 May 2014, Marek Polacek wrote: FAIL: c-c++-common/pr50459.c -std=gnu++1y (test for excess errors) FAIL: c-c++-common/pr50459.c -Wc++-compat (test for excess errors) The errors are /opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:8:1: error: constructor priorities are not supported /opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:9:1: error: destructor priorities are not supported Ah. The following untested patch should skip that test on Darwin. Ok? I don't think the whole test should be skipped for that issue; I think the part requiring this feature should be split out into a separate testcase, so that as much as possible is still tested on Darwin. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
Joseph,
I don't think the whole test should be skipped for that issue; I think the
part requiring this feature should be split out into a separate testcase,
so that as much as possible is still tested on Darwin.
Is the following patch
--- ../_clean/gcc/testsuite/c-c++-common/pr50459.c 2014-05-09
10:34:03.0 +0200
+++ gcc/testsuite/c-c++-common/pr50459.c2014-05-12 17:55:04.0
+0200
@@ -5,8 +5,8 @@
enum { A = 128, B = 1 };
void *fn1 (void) __attribute__((assume_aligned (A)));
void *fn2 (void) __attribute__((assume_aligned (A, 4)));
-void fn3 (void) __attribute__((constructor (A)));
-void fn4 (void) __attribute__((destructor (A)));
+void fn3 (void) __attribute__((constructor (A))); /* { dg-error constructor
priorities are not supported { target *-apple-darwin* } } */
+void fn4 (void) __attribute__((destructor (A))); /* { dg-error destructor
priorities are not supported { target *-apple-darwin* } } */
void *fn5 (int) __attribute__((alloc_size (B)));
void *fn6 (int) __attribute__((alloc_align (B)));
void fn7 (const char *, ...) __attribute__ ((sentinel (B)));
what you have in mind?
Dominique
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Mon, May 12, 2014 at 06:11:16PM +0200, Dominique Dhumieres wrote:
Joseph,
I don't think the whole test should be skipped for that issue; I think the
part requiring this feature should be split out into a separate testcase,
so that as much as possible is still tested on Darwin.
Is the following patch
--- ../_clean/gcc/testsuite/c-c++-common/pr50459.c2014-05-09
10:34:03.0 +0200
+++ gcc/testsuite/c-c++-common/pr50459.c 2014-05-12 17:55:04.0
+0200
@@ -5,8 +5,8 @@
enum { A = 128, B = 1 };
void *fn1 (void) __attribute__((assume_aligned (A)));
void *fn2 (void) __attribute__((assume_aligned (A, 4)));
-void fn3 (void) __attribute__((constructor (A)));
-void fn4 (void) __attribute__((destructor (A)));
+void fn3 (void) __attribute__((constructor (A))); /* { dg-error constructor
priorities are not supported { target *-apple-darwin* } } */
+void fn4 (void) __attribute__((destructor (A))); /* { dg-error destructor
priorities are not supported { target *-apple-darwin* } } */
void *fn5 (int) __attribute__((alloc_size (B)));
void *fn6 (int) __attribute__((alloc_align (B)));
void fn7 (const char *, ...) __attribute__ ((sentinel (B)));
what you have in mind?
I don't think so, we should split the pr50459.c into two .c files, the first
containing only the cdtor tests and requiring init_priotity targets,
the second one the rest (and not requiring init_priotity targets).
Marek
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Sat, May 10, 2014 at 04:10:47PM +0200, Dominique Dhumieres wrote:
...
Tested again x86_64-linux, ok now?
2014-05-02 Marek Polacek pola...@redhat.com
PR c/50459
This caused on x86_64-apple-darwin13
FAIL: c-c++-common/pr50459.c -std=gnu++98 (test for excess errors)
FAIL: c-c++-common/pr50459.c -std=gnu++11 (test for excess errors)
FAIL: c-c++-common/pr50459.c -std=gnu++1y (test for excess errors)
FAIL: c-c++-common/pr50459.c -Wc++-compat (test for excess errors)
The errors are
/opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:8:1: error: constructor
priorities are not supported
/opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:9:1: error: destructor
priorities are not supported
Ah. The following untested patch should skip that test on Darwin.
Ok?
2014-05-11 Marek Polacek pola...@redhat.com
* c-c++-common/pr50459.c: Skip test on Darwin.
diff --git gcc/testsuite/c-c++-common/pr50459.c
gcc/testsuite/c-c++-common/pr50459.c
index f837b63..1216fd6 100644
--- gcc/testsuite/c-c++-common/pr50459.c
+++ gcc/testsuite/c-c++-common/pr50459.c
@@ -1,6 +1,7 @@
/* PR c/50459 */
/* { dg-do compile } */
/* { dg-options -Wall -Wextra } */
+/* { dg-skip-if Darwin does not support cdtor priorities { *-*-darwin* } } */
enum { A = 128, B = 1 };
void *fn1 (void) __attribute__((assume_aligned (A)));
Marek
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
Marek Polacek pola...@redhat.com writes:
The errors are
/opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:8:1: error: constructor
priorities are not supported
/opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:9:1: error: destructor
priorities are not supported
Ah. The following untested patch should skip that test on Darwin.
Ok?
2014-05-11 Marek Polacek pola...@redhat.com
* c-c++-common/pr50459.c: Skip test on Darwin.
diff --git gcc/testsuite/c-c++-common/pr50459.c
gcc/testsuite/c-c++-common/pr50459.c
index f837b63..1216fd6 100644
--- gcc/testsuite/c-c++-common/pr50459.c
+++ gcc/testsuite/c-c++-common/pr50459.c
@@ -1,6 +1,7 @@
/* PR c/50459 */
/* { dg-do compile } */
/* { dg-options -Wall -Wextra } */
+/* { dg-skip-if Darwin does not support cdtor priorities { *-*-darwin* } }
*/
No, that's wrong: avoid hardcoding target lists if at all possible.
Besides, it's wrong since it doesn't cover the Solaris (and other
non-gld linker) case. Use the init_priority effective-target keyword
instead. Also, please check if you can use dg-xfail-if instead: if
anything changes, the test turns into an XPASS instead of the change
going unnoticed with dg-skip-if.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Sun, May 11, 2014 at 09:18:47PM +0200, Rainer Orth wrote:
No, that's wrong: avoid hardcoding target lists if at all possible.
Besides, it's wrong since it doesn't cover the Solaris (and other
non-gld linker) case. Use the init_priority effective-target keyword
instead. Also, please check if you can use dg-xfail-if instead: if
anything changes, the test turns into an XPASS instead of the change
going unnoticed with dg-skip-if.
I don't see tests using dg-skip-if and init_priority, so this patch
does what we do for other tests using cdtor priorities.
2014-05-11 Marek Polacek pola...@redhat.com
* c-c++-common/pr50459.c: Require init_priority target.
diff --git gcc/testsuite/c-c++-common/pr50459.c
gcc/testsuite/c-c++-common/pr50459.c
index f837b63..ee586b4 100644
--- gcc/testsuite/c-c++-common/pr50459.c
+++ gcc/testsuite/c-c++-common/pr50459.c
@@ -1,5 +1,5 @@
/* PR c/50459 */
-/* { dg-do compile } */
+/* { dg-do compile { target init_priority } } */
/* { dg-options -Wall -Wextra } */
enum { A = 128, B = 1 };
Marek
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
... Tested again x86_64-linux, ok now? 2014-05-02 Marek Polacek pola...@redhat.com PR c/50459 This caused on x86_64-apple-darwin13 FAIL: c-c++-common/pr50459.c -std=gnu++98 (test for excess errors) FAIL: c-c++-common/pr50459.c -std=gnu++11 (test for excess errors) FAIL: c-c++-common/pr50459.c -std=gnu++1y (test for excess errors) FAIL: c-c++-common/pr50459.c -Wc++-compat (test for excess errors) The errors are /opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:8:1: error: constructor priorities are not supported /opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:9:1: error: destructor priorities are not supported Dominique
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
Ping.
On Fri, May 02, 2014 at 11:28:54AM +0200, Marek Polacek wrote:
On Thu, May 01, 2014 at 11:20:25PM +, Joseph S. Myers wrote:
On Wed, 23 Apr 2014, Marek Polacek wrote:
diff --git gcc/testsuite/c-c++-common/attributes-1.c
gcc/testsuite/c-c++-common/attributes-1.c
index af4dd12..8458e47 100644
--- gcc/testsuite/c-c++-common/attributes-1.c
+++ gcc/testsuite/c-c++-common/attributes-1.c
@@ -9,7 +9,7 @@ typedef char vec __attribute__((vector_size(bar))); /* {
dg-warning ignored }
void f1(char*) __attribute__((nonnull(bar))); /* { dg-error invalid
operand } */
void f2(char*) __attribute__((nonnull(1,bar))); /* { dg-error invalid
operand } */
-void g() __attribute__((aligned(bar))); /* { dg-error invalid value|not
an integer } */
+void g() __attribute__((aligned(bar)));
I don't think it's appropriate to remove any test assertion that this
invalid code gets diagnosed.
If the only diagnostic is now one swallowed by the dg-prune-output in this
test, either that dg-prune-output needs to be removed (and corresponding
more detailed error expectations added), or a separate test needs adding
for this erroneous use of this attribute (that separate test not using
dg-prune-output).
Yeah, that was a weird thing to do. I yanked that particular test to
a new testcase. Otherwise no changes.
Tested again x86_64-linux, ok now?
2014-05-02 Marek Polacek pola...@redhat.com
PR c/50459
c-family/
* c-common.c (check_user_alignment): Return -1 if alignment is error
node.
(handle_aligned_attribute): Don't call default_conversion on
FUNCTION_DECLs.
(handle_vector_size_attribute): Likewise.
(handle_tm_wrap_attribute): Handle case when wrap_decl is error node.
(handle_sentinel_attribute): Call default_conversion and allow even
integral types as an argument.
c/
* c-parser.c (c_parser_attributes): Parse the arguments as an
expression-list if the attribute takes identifier.
testsuite/
* c-c++-common/attributes-1.c: Move test line to a new test.
* c-c++-common/attributes-2.c: New test.
* c-c++-common/pr50459.c: New test.
* c-c++-common/pr59280.c: Add undeclared to dg-error.
* gcc.dg/nonnull-2.c: Likewise.
* gcc.dg/pr55570.c: Modify dg-error.
* gcc.dg/tm/wrap-2.c: Likewise.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 0ad955d..3ebd960 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -7438,6 +7438,8 @@ check_user_alignment (const_tree align, bool allow_zero)
{
int i;
+ if (error_operand_p (align))
+return -1;
if (TREE_CODE (align) != INTEGER_CST
|| !INTEGRAL_TYPE_P (TREE_TYPE (align)))
{
@@ -7559,7 +7561,8 @@ handle_aligned_attribute (tree *node, tree ARG_UNUSED
(name), tree args,
if (args)
{
align_expr = TREE_VALUE (args);
- if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE)
+ if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE
+TREE_CODE (align_expr) != FUNCTION_DECL)
align_expr = default_conversion (align_expr);
}
else
@@ -8424,9 +8427,11 @@ handle_tm_wrap_attribute (tree *node, tree name, tree
args,
else
{
tree wrap_decl = TREE_VALUE (args);
- if (TREE_CODE (wrap_decl) != IDENTIFIER_NODE
-TREE_CODE (wrap_decl) != VAR_DECL
-TREE_CODE (wrap_decl) != FUNCTION_DECL)
+ if (error_operand_p (wrap_decl))
+;
+ else if (TREE_CODE (wrap_decl) != IDENTIFIER_NODE
+ TREE_CODE (wrap_decl) != VAR_DECL
+ TREE_CODE (wrap_decl) != FUNCTION_DECL)
error (%qE argument not an identifier, name);
else
{
@@ -8553,7 +8558,8 @@ handle_vector_size_attribute (tree *node, tree name,
tree args,
*no_add_attrs = true;
size = TREE_VALUE (args);
- if (size TREE_CODE (size) != IDENTIFIER_NODE)
+ if (size TREE_CODE (size) != IDENTIFIER_NODE
+ TREE_CODE (size) != FUNCTION_DECL)
size = default_conversion (size);
if (!tree_fits_uhwi_p (size))
@@ -8964,8 +8970,12 @@ handle_sentinel_attribute (tree *node, tree name, tree
args,
if (args)
{
tree position = TREE_VALUE (args);
+ if (position TREE_CODE (position) != IDENTIFIER_NODE
+TREE_CODE (position) != FUNCTION_DECL)
+ position = default_conversion (position);
- if (TREE_CODE (position) != INTEGER_CST)
+ if (TREE_CODE (position) != INTEGER_CST
+ || !INTEGRAL_TYPE_P (TREE_TYPE (position)))
{
warning (OPT_Wattributes,
requested position is not an integer constant);
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 7947355..48f8d2f 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -3955,11 +3955,16 @@ c_parser_attributes (c_parser *parser)
In objective-c the identifier may be a
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Fri, 2 May 2014, Marek Polacek wrote: Yeah, that was a weird thing to do. I yanked that particular test to a new testcase. Otherwise no changes. Tested again x86_64-linux, ok now? 2014-05-02 Marek Polacek pola...@redhat.com PR c/50459 c-family/ * c-common.c (check_user_alignment): Return -1 if alignment is error node. (handle_aligned_attribute): Don't call default_conversion on FUNCTION_DECLs. (handle_vector_size_attribute): Likewise. (handle_tm_wrap_attribute): Handle case when wrap_decl is error node. (handle_sentinel_attribute): Call default_conversion and allow even integral types as an argument. c/ * c-parser.c (c_parser_attributes): Parse the arguments as an expression-list if the attribute takes identifier. testsuite/ * c-c++-common/attributes-1.c: Move test line to a new test. * c-c++-common/attributes-2.c: New test. * c-c++-common/pr50459.c: New test. * c-c++-common/pr59280.c: Add undeclared to dg-error. * gcc.dg/nonnull-2.c: Likewise. * gcc.dg/pr55570.c: Modify dg-error. * gcc.dg/tm/wrap-2.c: Likewise. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Thu, May 01, 2014 at 11:20:25PM +, Joseph S. Myers wrote:
On Wed, 23 Apr 2014, Marek Polacek wrote:
diff --git gcc/testsuite/c-c++-common/attributes-1.c
gcc/testsuite/c-c++-common/attributes-1.c
index af4dd12..8458e47 100644
--- gcc/testsuite/c-c++-common/attributes-1.c
+++ gcc/testsuite/c-c++-common/attributes-1.c
@@ -9,7 +9,7 @@ typedef char vec __attribute__((vector_size(bar))); /* {
dg-warning ignored }
void f1(char*) __attribute__((nonnull(bar))); /* { dg-error invalid
operand } */
void f2(char*) __attribute__((nonnull(1,bar))); /* { dg-error invalid
operand } */
-void g() __attribute__((aligned(bar))); /* { dg-error invalid value|not
an integer } */
+void g() __attribute__((aligned(bar)));
I don't think it's appropriate to remove any test assertion that this
invalid code gets diagnosed.
If the only diagnostic is now one swallowed by the dg-prune-output in this
test, either that dg-prune-output needs to be removed (and corresponding
more detailed error expectations added), or a separate test needs adding
for this erroneous use of this attribute (that separate test not using
dg-prune-output).
Yeah, that was a weird thing to do. I yanked that particular test to
a new testcase. Otherwise no changes.
Tested again x86_64-linux, ok now?
2014-05-02 Marek Polacek pola...@redhat.com
PR c/50459
c-family/
* c-common.c (check_user_alignment): Return -1 if alignment is error
node.
(handle_aligned_attribute): Don't call default_conversion on
FUNCTION_DECLs.
(handle_vector_size_attribute): Likewise.
(handle_tm_wrap_attribute): Handle case when wrap_decl is error node.
(handle_sentinel_attribute): Call default_conversion and allow even
integral types as an argument.
c/
* c-parser.c (c_parser_attributes): Parse the arguments as an
expression-list if the attribute takes identifier.
testsuite/
* c-c++-common/attributes-1.c: Move test line to a new test.
* c-c++-common/attributes-2.c: New test.
* c-c++-common/pr50459.c: New test.
* c-c++-common/pr59280.c: Add undeclared to dg-error.
* gcc.dg/nonnull-2.c: Likewise.
* gcc.dg/pr55570.c: Modify dg-error.
* gcc.dg/tm/wrap-2.c: Likewise.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 0ad955d..3ebd960 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -7438,6 +7438,8 @@ check_user_alignment (const_tree align, bool allow_zero)
{
int i;
+ if (error_operand_p (align))
+return -1;
if (TREE_CODE (align) != INTEGER_CST
|| !INTEGRAL_TYPE_P (TREE_TYPE (align)))
{
@@ -7559,7 +7561,8 @@ handle_aligned_attribute (tree *node, tree ARG_UNUSED
(name), tree args,
if (args)
{
align_expr = TREE_VALUE (args);
- if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE)
+ if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE
+ TREE_CODE (align_expr) != FUNCTION_DECL)
align_expr = default_conversion (align_expr);
}
else
@@ -8424,9 +8427,11 @@ handle_tm_wrap_attribute (tree *node, tree name, tree
args,
else
{
tree wrap_decl = TREE_VALUE (args);
- if (TREE_CODE (wrap_decl) != IDENTIFIER_NODE
- TREE_CODE (wrap_decl) != VAR_DECL
- TREE_CODE (wrap_decl) != FUNCTION_DECL)
+ if (error_operand_p (wrap_decl))
+;
+ else if (TREE_CODE (wrap_decl) != IDENTIFIER_NODE
+ TREE_CODE (wrap_decl) != VAR_DECL
+ TREE_CODE (wrap_decl) != FUNCTION_DECL)
error (%qE argument not an identifier, name);
else
{
@@ -8553,7 +8558,8 @@ handle_vector_size_attribute (tree *node, tree name, tree
args,
*no_add_attrs = true;
size = TREE_VALUE (args);
- if (size TREE_CODE (size) != IDENTIFIER_NODE)
+ if (size TREE_CODE (size) != IDENTIFIER_NODE
+ TREE_CODE (size) != FUNCTION_DECL)
size = default_conversion (size);
if (!tree_fits_uhwi_p (size))
@@ -8964,8 +8970,12 @@ handle_sentinel_attribute (tree *node, tree name, tree
args,
if (args)
{
tree position = TREE_VALUE (args);
+ if (position TREE_CODE (position) != IDENTIFIER_NODE
+ TREE_CODE (position) != FUNCTION_DECL)
+ position = default_conversion (position);
- if (TREE_CODE (position) != INTEGER_CST)
+ if (TREE_CODE (position) != INTEGER_CST
+ || !INTEGRAL_TYPE_P (TREE_TYPE (position)))
{
warning (OPT_Wattributes,
requested position is not an integer constant);
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 7947355..48f8d2f 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -3955,11 +3955,16 @@ c_parser_attributes (c_parser *parser)
In objective-c the identifier may be a classname. */
if (c_parser_next_token_is (parser, CPP_NAME)
(c_parser_peek_token (parser)-id_kind == C_ID_ID
-
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Wed, 23 Apr 2014, Marek Polacek wrote:
diff --git gcc/testsuite/c-c++-common/attributes-1.c
gcc/testsuite/c-c++-common/attributes-1.c
index af4dd12..8458e47 100644
--- gcc/testsuite/c-c++-common/attributes-1.c
+++ gcc/testsuite/c-c++-common/attributes-1.c
@@ -9,7 +9,7 @@ typedef char vec __attribute__((vector_size(bar))); /* {
dg-warning ignored }
void f1(char*) __attribute__((nonnull(bar))); /* { dg-error invalid
operand } */
void f2(char*) __attribute__((nonnull(1,bar))); /* { dg-error invalid
operand } */
-void g() __attribute__((aligned(bar))); /* { dg-error invalid value|not an
integer } */
+void g() __attribute__((aligned(bar)));
I don't think it's appropriate to remove any test assertion that this
invalid code gets diagnosed.
If the only diagnostic is now one swallowed by the dg-prune-output in this
test, either that dg-prune-output needs to be removed (and corresponding
more detailed error expectations added), or a separate test needs adding
for this erroneous use of this attribute (that separate test not using
dg-prune-output).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Sat, Apr 19, 2014 at 09:56:02AM -0400, Jason Merrill wrote:
On 04/17/2014 12:00 PM, Marek Polacek wrote:
== CPP_CLOSE_PAREN)))
{
tree arg1 = c_parser_peek_token (parser)-value;
+ if (!attr_takes_id_p)
+{
+ /* This is for enum values, so that they can be used as
+ an attribute parameter; lookup_name will find their
+ CONST_DECLs. */
+ tree ln = lookup_name (arg1);
+ if (ln)
+arg1 = ln;
+}
c_parser_consume_token (parser);
Instead, we should add !attr_takes_id_p to the if condition
immediately above so that we parse the arguments as an
expression-list.
Ah, indeed. So like this? I had to add some ugliness because of
Obj-C and also tweak a few tests, since we now print slightly
different error message if the identifier in attribute argument
isn't declared.
Regtested/bootstrapped on x86_64-linux.
2014-04-22 Marek Polacek pola...@redhat.com
PR c/50459
c-family/
* c-common.c (check_user_alignment): Return -1 if alignment is error
node.
(handle_aligned_attribute): Don't call default_conversion on
FUNCTION_DECLs.
(handle_vector_size_attribute): Likewise.
(handle_tm_wrap_attribute): Handle case when wrap_decl is error node.
(handle_sentinel_attribute): Call default_conversion and allow even
integral types as an argument.
c/
* c-parser.c (c_parser_attributes): Parse the arguments as an
expression-list if the attribute takes identifier.
testsuite/
* c-c++-common/attributes-1.c: Remove dg-error line.
* c-c++-common/pr50459.c: New test.
* c-c++-common/pr59280.c: Add undeclared to dg-error.
* gcc.dg/nonnull-2.c: Likewise.
* gcc.dg/pr55570.c: Modify dg-error.
* gcc.dg/tm/wrap-2.c: Likewise.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index c0e247b..df44faa 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -7418,6 +7418,8 @@ check_user_alignment (const_tree align, bool allow_zero)
{
int i;
+ if (error_operand_p (align))
+return -1;
if (TREE_CODE (align) != INTEGER_CST
|| !INTEGRAL_TYPE_P (TREE_TYPE (align)))
{
@@ -7539,7 +7541,8 @@ handle_aligned_attribute (tree *node, tree ARG_UNUSED
(name), tree args,
if (args)
{
align_expr = TREE_VALUE (args);
- if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE)
+ if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE
+ TREE_CODE (align_expr) != FUNCTION_DECL)
align_expr = default_conversion (align_expr);
}
else
@@ -8404,9 +8407,11 @@ handle_tm_wrap_attribute (tree *node, tree name, tree
args,
else
{
tree wrap_decl = TREE_VALUE (args);
- if (TREE_CODE (wrap_decl) != IDENTIFIER_NODE
- TREE_CODE (wrap_decl) != VAR_DECL
- TREE_CODE (wrap_decl) != FUNCTION_DECL)
+ if (error_operand_p (wrap_decl))
+;
+ else if (TREE_CODE (wrap_decl) != IDENTIFIER_NODE
+ TREE_CODE (wrap_decl) != VAR_DECL
+ TREE_CODE (wrap_decl) != FUNCTION_DECL)
error (%qE argument not an identifier, name);
else
{
@@ -8533,7 +8538,8 @@ handle_vector_size_attribute (tree *node, tree name, tree
args,
*no_add_attrs = true;
size = TREE_VALUE (args);
- if (size TREE_CODE (size) != IDENTIFIER_NODE)
+ if (size TREE_CODE (size) != IDENTIFIER_NODE
+ TREE_CODE (size) != FUNCTION_DECL)
size = default_conversion (size);
if (!tree_fits_uhwi_p (size))
@@ -8944,8 +8950,12 @@ handle_sentinel_attribute (tree *node, tree name, tree
args,
if (args)
{
tree position = TREE_VALUE (args);
+ if (position TREE_CODE (position) != IDENTIFIER_NODE
+ TREE_CODE (position) != FUNCTION_DECL)
+ position = default_conversion (position);
- if (TREE_CODE (position) != INTEGER_CST)
+ if (TREE_CODE (position) != INTEGER_CST
+ || !INTEGRAL_TYPE_P (TREE_TYPE (position)))
{
warning (OPT_Wattributes,
requested position is not an integer constant);
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 5653e49..8d91d6b 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -3943,11 +3943,16 @@ c_parser_attributes (c_parser *parser)
In objective-c the identifier may be a classname. */
if (c_parser_next_token_is (parser, CPP_NAME)
(c_parser_peek_token (parser)-id_kind == C_ID_ID
- || (c_dialect_objc ()
- c_parser_peek_token (parser)-id_kind ==
C_ID_CLASSNAME))
+ || (c_dialect_objc ()
+ c_parser_peek_token (parser)-id_kind
+== C_ID_CLASSNAME))
((c_parser_peek_2nd_token (parser)-type == CPP_COMMA)
|| (c_parser_peek_2nd_token
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On 04/17/2014 12:00 PM, Marek Polacek wrote:
== CPP_CLOSE_PAREN)))
{
tree arg1 = c_parser_peek_token (parser)-value;
+ if (!attr_takes_id_p)
+ {
+ /* This is for enum values, so that they can be used as
+an attribute parameter; lookup_name will find their
+CONST_DECLs. */
+ tree ln = lookup_name (arg1);
+ if (ln)
+ arg1 = ln;
+ }
c_parser_consume_token (parser);
Instead, we should add !attr_takes_id_p to the if condition immediately
above so that we parse the arguments as an expression-list.
Jason
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Wed, Apr 16, 2014 at 01:40:22PM -0400, Jason Merrill wrote:
On 04/15/2014 03:56 PM, Marek Polacek wrote:
The testsuite doesn't hit this code with C++, but does hit this code
with C. The thing is, if we have e.g.
enum { A = 128 };
void *fn1 (void) __attribute__((assume_aligned (A)));
then handle_assume_aligned_attribute walks the attribute arguments
and gets the argument via TREE_VALUE. If this argument is an enum
value, then for C the argument is identifier_node that contains const_decl,
Ah. Then I think the C parser should be fixed to check
attribute_takes_identifier_p and look up the argument if false.
Ok, thanks, I didn't know about attribute_takes_identifier_p. Should be done
in the following. Regtested/bootstrapped on x86_64-linux. Ok now?
2014-04-17 Marek Polacek pola...@redhat.com
PR c/50459
c-family/
* c-common.c (handle_aligned_attribute): Don't call default_conversion
on FUNCTION_DECLs.
(handle_vector_size_attribute): Likewise.
(handle_sentinel_attribute): Call default_conversion and allow even
integral types as an argument.
c/
* c-parser.c (c_parser_attributes): If the attribute doesn't take an
identifier, call lookup_name for arguments.
testsuite/
* c-c++-common/pr50459.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index c0e247b..1443914 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -7539,7 +7539,8 @@ handle_aligned_attribute (tree *node, tree ARG_UNUSED
(name), tree args,
if (args)
{
align_expr = TREE_VALUE (args);
- if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE)
+ if (align_expr TREE_CODE (align_expr) != IDENTIFIER_NODE
+ TREE_CODE (align_expr) != FUNCTION_DECL)
align_expr = default_conversion (align_expr);
}
else
@@ -8533,7 +8534,8 @@ handle_vector_size_attribute (tree *node, tree name, tree
args,
*no_add_attrs = true;
size = TREE_VALUE (args);
- if (size TREE_CODE (size) != IDENTIFIER_NODE)
+ if (size TREE_CODE (size) != IDENTIFIER_NODE
+ TREE_CODE (size) != FUNCTION_DECL)
size = default_conversion (size);
if (!tree_fits_uhwi_p (size))
@@ -8944,8 +8946,12 @@ handle_sentinel_attribute (tree *node, tree name, tree
args,
if (args)
{
tree position = TREE_VALUE (args);
+ if (position TREE_CODE (position) != IDENTIFIER_NODE
+ TREE_CODE (position) != FUNCTION_DECL)
+ position = default_conversion (position);
- if (TREE_CODE (position) != INTEGER_CST)
+ if (TREE_CODE (position) != INTEGER_CST
+ || !INTEGRAL_TYPE_P (TREE_TYPE (position)))
{
warning (OPT_Wattributes,
requested position is not an integer constant);
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 5653e49..f8fe424 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -3912,6 +3912,7 @@ c_parser_attributes (c_parser *parser)
|| c_parser_next_token_is (parser, CPP_KEYWORD))
{
tree attr, attr_name, attr_args;
+ bool attr_takes_id_p;
vectree, va_gc *expr_list;
if (c_parser_next_token_is (parser, CPP_COMMA))
{
@@ -3922,6 +3923,7 @@ c_parser_attributes (c_parser *parser)
attr_name = c_parser_attribute_any_word (parser);
if (attr_name == NULL)
break;
+ attr_takes_id_p = attribute_takes_identifier_p (attr_name);
if (is_cilkplus_vector_p (attr_name))
{
c_token *v_token = c_parser_peek_token (parser);
@@ -3950,6 +3952,15 @@ c_parser_attributes (c_parser *parser)
== CPP_CLOSE_PAREN)))
{
tree arg1 = c_parser_peek_token (parser)-value;
+ if (!attr_takes_id_p)
+ {
+ /* This is for enum values, so that they can be used as
+an attribute parameter; lookup_name will find their
+CONST_DECLs. */
+ tree ln = lookup_name (arg1);
+ if (ln)
+ arg1 = ln;
+ }
c_parser_consume_token (parser);
if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
attr_args = build_tree_list (NULL_TREE, arg1);
diff --git gcc/testsuite/c-c++-common/pr50459.c
gcc/testsuite/c-c++-common/pr50459.c
index e69de29..f954b32 100644
--- gcc/testsuite/c-c++-common/pr50459.c
+++ gcc/testsuite/c-c++-common/pr50459.c
@@ -0,0 +1,14 @@
+/* PR c/50459 */
+/* { dg-do compile } */
+/* { dg-options -Wall -Wextra } */
+
+enum { A = 128, B = 1 };
+void *fn1 (void) __attribute__((assume_aligned (A)));
+void *fn2 (void) __attribute__((assume_aligned (A, 4)));
+void fn3 (void) __attribute__((constructor (A)));
+void fn4 (void) __attribute__((destructor (A)));
+void *fn5 (int) __attribute__((alloc_size (B)));
+void *fn6 (int) __attribute__((alloc_align (B)));
+void fn7 (const char *,
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On 04/15/2014 03:56 PM, Marek Polacek wrote:
The testsuite doesn't hit this code with C++, but does hit this code
with C. The thing is, if we have e.g.
enum { A = 128 };
void *fn1 (void) __attribute__((assume_aligned (A)));
then handle_assume_aligned_attribute walks the attribute arguments
and gets the argument via TREE_VALUE. If this argument is an enum
value, then for C the argument is identifier_node that contains const_decl,
Ah. Then I think the C parser should be fixed to check
attribute_takes_identifier_p and look up the argument if false.
Jason
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On 04/14/2014 10:32 AM, Marek Polacek wrote:
+ if (TREE_CODE (val) != IDENTIFIER_NODE
+TREE_CODE (val) != FUNCTION_DECL)
+ val = default_conversion (val);
+ else if (TREE_CODE (val) == IDENTIFIER_NODE)
+ {
+ tree t = lookup_name (val);
+ if (t TREE_CODE (t) == CONST_DECL)
+ val = default_conversion (t);
+ }
In addition to Jason's comment, a general style point:
if (X != A X != B)
...
else if (X == A)
...
should be written
if (X == A)
...
else if (X != B)
...
As a general rule, positive tests are easier to reason with than negative tests.
r~
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On 04/14/2014 11:10 AM, Marek Polacek wrote:
+ else if (TREE_CODE (val) == IDENTIFIER_NODE)
+{
+ tree t = lookup_name (val);
+ if (t TREE_CODE (t) == CONST_DECL)
+ return DECL_INITIAL (t);
+}
I'm uncomfortable with this; we should have looked up any attributes in
the parser. Does the testsuite hit this code?
Jason
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Tue, Apr 15, 2014 at 09:59:10AM -0400, Jason Merrill wrote:
On 04/14/2014 11:10 AM, Marek Polacek wrote:
+ else if (TREE_CODE (val) == IDENTIFIER_NODE)
+{
+ tree t = lookup_name (val);
+ if (t TREE_CODE (t) == CONST_DECL)
+return DECL_INITIAL (t);
+}
I'm uncomfortable with this; we should have looked up any attributes
in the parser. Does the testsuite hit this code?
Thanks for looking at it.
So the newer version of the patch contains:
+ else if (TREE_CODE (val) == IDENTIFIER_NODE)
+ {
+ tree t = lookup_name (val);
+ if (t TREE_CODE (t) == CONST_DECL)
+ val = default_conversion (t);
+ }
The testsuite doesn't hit this code with C++, but does hit this code
with C. The thing is, if we have e.g.
enum { A = 128 };
void *fn1 (void) __attribute__((assume_aligned (A)));
then handle_assume_aligned_attribute walks the attribute arguments
and gets the argument via TREE_VALUE. If this argument is an enum
value, then for C the argument is identifier_node that contains const_decl,
but for C++ the argument is directly const_decl. That means for C++ in
get_attrib_value we just call default_conversion as before, but for C we
call lookup_name firstly.
Does this answer your question?
Marek
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Mon, 14 Apr 2014, Marek Polacek wrote:
Currently it is not possible to use an enum value (or const int in
C++) as an attribute argument (where compiler wants INTEGER_CST). This
is because the values are represented by CONST_DECL nodes -- and
we don't deal with them. I think it's viable to accept them too,
and that is what this patch does along with some clean ups.
Thanks.
+/* Perform default promotions or extract the integer constant from
+ an enum value. This function is used for getting a value from
+ an attribute argument. */
+
+static tree
+get_attrib_value (tree val)
+{
+ if (val
+ TREE_CODE (val) != IDENTIFIER_NODE
+ TREE_CODE (val) != FUNCTION_DECL)
+val = default_conversion (val);
+ else if (TREE_CODE (val) == IDENTIFIER_NODE)
+{
+ tree t = lookup_name (val);
+ if (t TREE_CODE (t) == CONST_DECL)
+ return DECL_INITIAL (t);
+}
+ return val;
+}
If val is NULL, it seems you still end up looking at its TREE_CODE?
(don't know if that can happen)
In C++, default_conversion probably handles IDENTIFIER_NODE just fine, but
having different code for the 2 languages would not be nice, right?
--
Marc Glisse
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Mon, Apr 14, 2014 at 05:26:03PM +0200, Marc Glisse wrote:
If val is NULL, it seems you still end up looking at its TREE_CODE?
(don't know if that can happen)
I didn't hit that while testing, but it looks weird, so fixed.
In C++, default_conversion probably handles IDENTIFIER_NODE just
fine, but having different code for the 2 languages would not be
nice, right?
Right. Incidentally, C's default_conversion can handle CONST_DECLs
too, so I tweaked get_attrib_value routine a bit. Is it any better?
Tested x86_64.
2014-04-14 Marek Polacek pola...@redhat.com
PR c/50459
c-family/
* c-common.c (get_attrib_value): New function.
(get_priority): Call get_attrib_value.
(check_user_alignment): Likewise.
(handle_alloc_size_attribute): Likewise.
(handle_alloc_align_attribute): Likewise.
(handle_assume_aligned_attribute): Likewise.
(handle_vector_size_attribute): Likewise.
(handle_sentinel_attribute): Likewise.
testsuite/
* c-c++-common/pr50459.c: New test.
* g++.dg/ext/pr50459.C: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 1d56bc0..d627462 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -6448,6 +6448,28 @@ attribute_takes_identifier_p (const_tree attr_id)
return targetm.attribute_takes_identifier_p (attr_id);
}
+/* Extract the integer constant from an enum value and/or perform default
+ promotions. This function is used for getting a value from an attribute
+ argument. */
+
+static tree
+get_attrib_value (tree val)
+{
+ if (val)
+{
+ if (TREE_CODE (val) != IDENTIFIER_NODE
+ TREE_CODE (val) != FUNCTION_DECL)
+ val = default_conversion (val);
+ else if (TREE_CODE (val) == IDENTIFIER_NODE)
+ {
+ tree t = lookup_name (val);
+ if (t TREE_CODE (t) == CONST_DECL)
+ val = default_conversion (t);
+ }
+}
+ return val;
+}
+
/* Attribute handlers common to C front ends. */
/* Handle a packed attribute; arguments as in
@@ -7030,11 +7052,10 @@ get_priority (tree args, bool is_destructor)
}
arg = TREE_VALUE (args);
- if (TREE_CODE (arg) == IDENTIFIER_NODE)
-goto invalid;
if (arg == error_mark_node)
return DEFAULT_INIT_PRIORITY;
- arg = default_conversion (arg);
+ arg = get_attrib_value (arg);
+
if (!tree_fits_shwi_p (arg)
|| !INTEGRAL_TYPE_P (TREE_TYPE (arg)))
goto invalid;
@@ -7418,6 +7439,8 @@ check_user_alignment (const_tree align, bool allow_zero)
{
int i;
+ align = get_attrib_value (CONST_CAST_TREE (align));
+
if (TREE_CODE (align) != INTEGER_CST
|| !INTEGRAL_TYPE_P (TREE_TYPE (align)))
{
@@ -8037,11 +8060,7 @@ handle_alloc_size_attribute (tree *node, tree ARG_UNUSED
(name), tree args,
unsigned arg_count = type_num_arguments (*node);
for (; args; args = TREE_CHAIN (args))
{
- tree position = TREE_VALUE (args);
- if (position TREE_CODE (position) != IDENTIFIER_NODE
- TREE_CODE (position) != FUNCTION_DECL)
- position = default_conversion (position);
-
+ tree position = get_attrib_value (TREE_VALUE (args));
if (!tree_fits_uhwi_p (position)
|| !arg_count
|| !IN_RANGE (tree_to_uhwi (position), 1, arg_count))
@@ -8063,10 +8082,7 @@ handle_alloc_align_attribute (tree *node, tree, tree
args, int,
bool *no_add_attrs)
{
unsigned arg_count = type_num_arguments (*node);
- tree position = TREE_VALUE (args);
- if (position TREE_CODE (position) != IDENTIFIER_NODE)
-position = default_conversion (position);
-
+ tree position = get_attrib_value (TREE_VALUE (args));
if (!tree_fits_uhwi_p (position)
|| !arg_count
|| !IN_RANGE (tree_to_uhwi (position), 1, arg_count))
@@ -8088,11 +8104,7 @@ handle_assume_aligned_attribute (tree *, tree, tree
args, int,
{
for (; args; args = TREE_CHAIN (args))
{
- tree position = TREE_VALUE (args);
- if (position TREE_CODE (position) != IDENTIFIER_NODE
- TREE_CODE (position) != FUNCTION_DECL)
- position = default_conversion (position);
-
+ tree position = get_attrib_value (TREE_VALUE (args));
if (TREE_CODE (position) != INTEGER_CST)
{
warning (OPT_Wattributes,
@@ -8532,10 +8544,7 @@ handle_vector_size_attribute (tree *node, tree name,
tree args,
*no_add_attrs = true;
- size = TREE_VALUE (args);
- if (size TREE_CODE (size) != IDENTIFIER_NODE)
-size = default_conversion (size);
-
+ size = get_attrib_value (TREE_VALUE (args));
if (!tree_fits_uhwi_p (size))
{
warning (OPT_Wattributes, %qE attribute ignored, name);
@@ -8943,8 +8952,7 @@ handle_sentinel_attribute (tree *node, tree name, tree
args,
if (args)
{
- tree position = TREE_VALUE (args);
-
+ tree position = get_attrib_value (TREE_VALUE (args));
if (TREE_CODE (position) != INTEGER_CST)
{
