| Message ID | 1450198309-5325-1-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 15, 2015 at 11:51:49AM -0500, David Malcolm wrote: > When issuing diagnostics for _Static_assert, we currently ignore the > location/range of the asserted expression, and instead use the > location/range of the first token within it, which can be > incorrect for compound expressions: > > error: expression in static assertion is not constant > _Static_assert (param > 0, "message"); > ^~~~~ > > This patch changes things to use EXPR_LOC_OR_LOC, so we use the > location/range of the expression if it has one, falling back to the old > behavior if it doesn't, giving: > > error: expression in static assertion is not constant > _Static_assert (param > 0, "message"); > ~~~~~~^~~ > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu > > OK for trunk in stage 3? > > [a much earlier version of this was posted as part of: > "[PATCH 16/22] C/C++ frontend: use tree ranges in various diagnostics" > https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00745.html > but this patch bears no resemblence apart from the testcase, due to > changes in representation] > > gcc/c/ChangeLog: > * c-parser.c (c_parser_static_assert_declaration_no_semi): Use the > expression location, falling back on the first token location, > rather than always using the latter. > > gcc/testsuite/ChangeLog: > * gcc.dg/diagnostic-range-static-assert.c: New test case. Looks ok to me. Marek
On 12/15/2015 09:51 AM, David Malcolm wrote: > When issuing diagnostics for _Static_assert, we currently ignore the > location/range of the asserted expression, and instead use the > location/range of the first token within it, which can be > incorrect for compound expressions: > > error: expression in static assertion is not constant > _Static_assert (param > 0, "message"); > ^~~~~ > > This patch changes things to use EXPR_LOC_OR_LOC, so we use the > location/range of the expression if it has one, falling back to the old > behavior if it doesn't, giving: > > error: expression in static assertion is not constant > _Static_assert (param > 0, "message"); > ~~~~~~^~~ > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu > > OK for trunk in stage 3? > > [a much earlier version of this was posted as part of: > "[PATCH 16/22] C/C++ frontend: use tree ranges in various diagnostics" > https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00745.html > but this patch bears no resemblence apart from the testcase, due to > changes in representation] > > gcc/c/ChangeLog: > * c-parser.c (c_parser_static_assert_declaration_no_semi): Use the > expression location, falling back on the first token location, > rather than always using the latter. > > gcc/testsuite/ChangeLog: > * gcc.dg/diagnostic-range-static-assert.c: New test case. IMHO, this is a bugfix and thus entirely appropriate at this stage. OK for the trunk. Jeff
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 124c30b..5c32f45 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -2097,8 +2097,9 @@ c_parser_static_assert_declaration_no_semi (c_parser *parser) c_parser_consume_token (parser); if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) return; - value_loc = c_parser_peek_token (parser)->location; + location_t value_tok_loc = c_parser_peek_token (parser)->location; value = c_parser_expr_no_commas (parser, NULL).value; + value_loc = EXPR_LOC_OR_LOC (value, value_tok_loc); parser->lex_untranslated_string = true; if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>")) { diff --git a/gcc/testsuite/gcc.dg/diagnostic-range-static-assert.c b/gcc/testsuite/gcc.dg/diagnostic-range-static-assert.c new file mode 100644 index 0000000..6f75476 --- /dev/null +++ b/gcc/testsuite/gcc.dg/diagnostic-range-static-assert.c @@ -0,0 +1,24 @@ +/* { dg-options "-fdiagnostics-show-caret" } */ + +void test_nonconst_static_assert (int param) +{ + int local = 0; + + _Static_assert (param > 0, "message"); /* { dg-error "expression in static assertion is not constant" } */ +/* { dg-begin-multiline-output "" } + _Static_assert (param > 0, "message"); + ~~~~~~^~~ +{ dg-end-multiline-output "" } */ + + _Static_assert (param, "message"); /* { dg-error "expression in static assertion is not constant" } */ +/* { dg-begin-multiline-output "" } + _Static_assert (param, "message"); + ^~~~~ +{ dg-end-multiline-output "" } */ + + _Static_assert (local, "message"); /* { dg-error "expression in static assertion is not constant" } */ +/* { dg-begin-multiline-output "" } + _Static_assert (local, "message"); + ^~~~~ +{ dg-end-multiline-output "" } */ +}
When issuing diagnostics for _Static_assert, we currently ignore the location/range of the asserted expression, and instead use the location/range of the first token within it, which can be incorrect for compound expressions: error: expression in static assertion is not constant _Static_assert (param > 0, "message"); ^~~~~ This patch changes things to use EXPR_LOC_OR_LOC, so we use the location/range of the expression if it has one, falling back to the old behavior if it doesn't, giving: error: expression in static assertion is not constant _Static_assert (param > 0, "message"); ~~~~~~^~~ Successfully bootstrapped®rtested on x86_64-pc-linux-gnu OK for trunk in stage 3? [a much earlier version of this was posted as part of: "[PATCH 16/22] C/C++ frontend: use tree ranges in various diagnostics" https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00745.html but this patch bears no resemblence apart from the testcase, due to changes in representation] gcc/c/ChangeLog: * c-parser.c (c_parser_static_assert_declaration_no_semi): Use the expression location, falling back on the first token location, rather than always using the latter. gcc/testsuite/ChangeLog: * gcc.dg/diagnostic-range-static-assert.c: New test case. --- gcc/c/c-parser.c | 3 ++- .../gcc.dg/diagnostic-range-static-assert.c | 24 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/diagnostic-range-static-assert.c