On Wed, 2017-03-15 at 16:35 -0400, Jason Merrill wrote:
> On Tue, Mar 14, 2017 at 9:05 PM, David Malcolm <[email protected]>
> 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
