On Wed, 2017-03-15 at 16:35 -0400, Jason Merrill wrote: > On Tue, Mar 14, 2017 at 9:05 PM, David Malcolm <dmalc...@redhat.com> > wrote: > > OK for trunk now, or should this wait until stage 1? > > Stage 1. > > > + cp_token *close_paren = cp_parser_require (parser, > > CPP_CLOSE_PAREN, > > + RT_CLOSE_PAREN); > > + location_t end_loc = close_paren ? > > + close_paren->location : UNKNOWN_LOCATION; > > /* If all went well, simply lookup the type-id. */ > > if (cp_parser_parse_definitely (parser)) > > postfix_expression = get_typeid (type, > > tf_warning_or_error); > > @@ -6527,13 +6530,23 @@ cp_parser_postfix_expression (cp_parser > > *parser, bool address_p, bool cast_p, > > /* Compute its typeid. */ > > postfix_expression = build_typeid (expression, > > tf_warning_or_error); > > /* Look for the `)' token. */ > > - cp_parser_require (parser, CPP_CLOSE_PAREN, > > RT_CLOSE_PAREN); > > + close_paren > > + = cp_parser_require (parser, CPP_CLOSE_PAREN, > > RT_CLOSE_PAREN); > > + end_loc = close_paren ? close_paren->location : > > UNKNOWN_LOCATION; > > In both cases you're setting end_loc from close_paren, so how about > only computing it once, closer to where it's used? > > Jason
Here's an updated version of the patch; it only looks up the location of the close paren once (and adds another test case). I also replaced the ?: with an if () to avoid setting finish to UNKNOWN_LOCATION. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? Blurb from v1: PR c++/80014 reports an issue where no caret is printed when issuing an error for this bogus code: !typeid(void); Root cause is that we're not setting up the location for the cp_expr for the typeid expression, so that !typeid(void) has start == caret at the '!', but finish == UNKNOWN_LOCATION, and so the layout ctor within diagnostic-show-locus.c filters out the primary range, and hence layout::should_print_annotation_line_p returns false. This patch fixes things by setting up a location for the typeid expression when parsing it. gcc/cp/ChangeLog: PR c++/80014 * parser.c (cp_parser_postfix_expression): Construct a location for typeid expressions. gcc/testsuite/ChangeLog: PR c++/80014 * g++.dg/plugin/diagnostic-test-expressions-1.C (std::type_info): Add declaration. (test_typeid): New test function. --- gcc/cp/parser.c | 18 ++++++++++-- .../g++.dg/plugin/diagnostic-test-expressions-1.C | 32 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 4adf9aa..49003e4 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -6517,7 +6517,8 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p, /* Look for the `)' token. Otherwise, we can't be sure that we're not looking at an expression: consider `typeid (int (3))', for example. */ - cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN); + cp_token *close_paren = cp_parser_require (parser, CPP_CLOSE_PAREN, + RT_CLOSE_PAREN); /* If all went well, simply lookup the type-id. */ if (cp_parser_parse_definitely (parser)) postfix_expression = get_typeid (type, tf_warning_or_error); @@ -6531,13 +6532,26 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p, /* Compute its typeid. */ postfix_expression = build_typeid (expression, tf_warning_or_error); /* Look for the `)' token. */ - cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN); + close_paren + = cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN); } /* Restore the saved message. */ parser->type_definition_forbidden_message = saved_message; /* `typeid' may not appear in an integral constant expression. */ if (cp_parser_non_integral_constant_expression (parser, NIC_TYPEID)) postfix_expression = error_mark_node; + + /* Construct a location e.g. : + typeid (expr) + ^~~~~~~~~~~~~ + ranging from the start of the "typeid" token to the final closing + paren, with the caret at the start. */ + if (close_paren) + { + location_t typeid_loc + = make_location (start_loc, start_loc, close_paren->location); + postfix_expression.set_location (typeid_loc); + } } break; diff --git a/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C b/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C index 2c004f3..62d3c36 100644 --- a/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C +++ b/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C @@ -848,3 +848,35 @@ tests::test_method_calls () ~~~~~~~~~~~~~~~~~~^~ { dg-end-multiline-output "" } */ } + +namespace std +{ + class type_info { public: int foo; }; +} + +void test_typeid (int i) +{ + __emit_expression_range (0, &typeid(i)); /* { dg-warning "range" } */ +/* { dg-begin-multiline-output "" } + __emit_expression_range (0, &typeid(i)); + ^~~~~~~~~~ + { dg-end-multiline-output "" } */ + + __emit_expression_range (0, &typeid(int)); /* { dg-warning "range" } */ +/* { dg-begin-multiline-output "" } + __emit_expression_range (0, &typeid(int)); + ^~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + + __emit_expression_range (0, &typeid(i * 2)); /* { dg-warning "range" } */ +/* { dg-begin-multiline-output "" } + __emit_expression_range (0, &typeid(i * 2)); + ^~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + + __emit_expression_range (0, typeid(int).foo); /* { dg-warning "range" } */ +/* { dg-begin-multiline-output "" } + __emit_expression_range (0, typeid(int).foo); + ~~~~~~~~~~~~^~~ + { dg-end-multiline-output "" } */ +} -- 1.8.5.3