OK.
On Wed, Jun 28, 2017 at 7:13 PM, David Malcolm <dmalc...@redhat.com> wrote:
> 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
>
