Message ID | 20140825114333.GI15033@redhat.com |
---|---|
State | New |
Headers | show |
On Mon, 25 Aug 2014, Marek Polacek wrote: > PR62024 reports that we can't use __atomic_always_lock_free in > a static assert, as the FEs say it's not a constant expression. Yet the > docs say that the result of __atomic_always_lock_free is a compile time > constant. > We can fix this pretty easily. While fold folds __atomic_always_lock_free > to a constant, that constant is wrapped in NOP_EXPR - and static assert > code is unhappy. > I think we can just STRIP_TYPE_NOPS - we don't expect an lvalue in the > static assert code. This is done in both C and C++ FEs. What do you think? > In C, we'd still pedwarn on such code, and in C++ we'd still reject > non-constexpr functions that are not builtin functions. Is this NOP_EXPR (for C) the one left by c_fully_fold to carry TREE_NO_WARNING information? If so, the C front-end part of this patch is OK, but at least in principle this issue could affect various other places that give a pedwarn-if-pedantic for something that's not an integer constant expression but folds to one.
On 08/25/2014 07:43 AM, Marek Polacek wrote:
> * semantics.c (finish_static_assert): Strip no-op conversions.
I think I'd rather strip these in cxx_eval_builtin_function_call so that
we don't have to deal with them in various consumers.
Jason
On Wed, Aug 27, 2014 at 05:59:17PM +0000, Joseph S. Myers wrote: > On Mon, 25 Aug 2014, Marek Polacek wrote: > > > PR62024 reports that we can't use __atomic_always_lock_free in > > a static assert, as the FEs say it's not a constant expression. Yet the > > docs say that the result of __atomic_always_lock_free is a compile time > > constant. > > We can fix this pretty easily. While fold folds __atomic_always_lock_free > > to a constant, that constant is wrapped in NOP_EXPR - and static assert > > code is unhappy. > > I think we can just STRIP_TYPE_NOPS - we don't expect an lvalue in the > > static assert code. This is done in both C and C++ FEs. What do you think? > > In C, we'd still pedwarn on such code, and in C++ we'd still reject > > non-constexpr functions that are not builtin functions. > > Is this NOP_EXPR (for C) the one left by c_fully_fold to carry > TREE_NO_WARNING information? Yes. This NOP_EXPR can naturally also carry a location. > If so, the C front-end part of this patch is OK, but at least in principle > this issue could affect various other places that give a > pedwarn-if-pedantic for something that's not an integer constant > expression but folds to one. Exactly. In this particular patch I've tried to limit this to _Static_assert only. Thanks, Marek
diff --git gcc/c/c-parser.c gcc/c/c-parser.c index d634bb1..fc7bbaf 100644 --- gcc/c/c-parser.c +++ gcc/c/c-parser.c @@ -2058,6 +2058,8 @@ c_parser_static_assert_declaration_no_semi (c_parser *parser) if (TREE_CODE (value) != INTEGER_CST) { value = c_fully_fold (value, false, NULL); + /* Strip no-op conversions. */ + STRIP_TYPE_NOPS (value); if (TREE_CODE (value) == INTEGER_CST) pedwarn (value_loc, OPT_Wpedantic, "expression in static assertion " "is not an integer constant expression"); diff --git gcc/cp/semantics.c gcc/cp/semantics.c index a54011f..b87d3ba 100644 --- gcc/cp/semantics.c +++ gcc/cp/semantics.c @@ -6870,6 +6870,9 @@ finish_static_assert (tree condition, tree message, location_t location, condition = cp_convert (boolean_type_node, condition, tf_warning_or_error); condition = maybe_constant_value (condition); + /* Strip no-op conversions. */ + STRIP_TYPE_NOPS (condition); + if (TREE_CODE (condition) == INTEGER_CST && !integer_zerop (condition)) /* Do nothing; the condition is satisfied. */ ; diff --git gcc/testsuite/g++.dg/cpp0x/pr62024.C gcc/testsuite/g++.dg/cpp0x/pr62024.C index e69de29..5f0640a 100644 --- gcc/testsuite/g++.dg/cpp0x/pr62024.C +++ gcc/testsuite/g++.dg/cpp0x/pr62024.C @@ -0,0 +1,7 @@ +// PR c/62024 +// { dg-do compile { target c++11 } } +// { dg-require-effective-target sync_char_short } + +int *p; +static_assert (__atomic_always_lock_free (1, p), ""); +static_assert (__atomic_always_lock_free (1, 0), ""); diff --git gcc/testsuite/gcc.dg/pr62024.c gcc/testsuite/gcc.dg/pr62024.c index e69de29..79a0b79 100644 --- gcc/testsuite/gcc.dg/pr62024.c +++ gcc/testsuite/gcc.dg/pr62024.c @@ -0,0 +1,8 @@ +/* PR c/62024 */ +/* { dg-do compile } */ +/* { dg-options "-std=gnu11 -Wpedantic" } */ +/* { dg-require-effective-target sync_char_short } */ + +int *p; +_Static_assert (__atomic_always_lock_free (1, p), ""); /* { dg-warning "is not an integer constant" } */ +_Static_assert (__atomic_always_lock_free (1, 0), ""); /* { dg-warning "is not an integer constant" } */