[PING] re [PATCH v2] C++: Add fix-it hints for -Wold-style-cast
David Malcolm
dmalcolm@redhat.com
Fri May 26 19:54:00 GMT 2017
On Wed, 2017-05-03 at 09:51 -0400, David Malcolm wrote:
> On Thu, 2017-04-27 at 23:03 +0200, Marek Polacek wrote:
> > On Thu, Apr 27, 2017 at 05:10:24PM -0400, David Malcolm wrote:
> > > + /* First try const_cast. */
> > > + trial = build_const_cast (dst_type, orig_expr, 0 /* complain
> > > */);
> > > + if (trial != error_mark_node)
> > > + return "const_cast";
> > > +
> > > + /* If that fails, try static_cast. */
> > > + trial = build_static_cast (dst_type, orig_expr, 0 /* complain
> > > */);
> > > + if (trial != error_mark_node)
> > > + return "static_cast";
> > > +
> > > + /* Finally, try reinterpret_cast. */
> > > + trial = build_reinterpret_cast (dst_type, orig_expr, 0 /*
> > > complain */);
> > > + if (trial != error_mark_node)
> > > + return "reinterpret_cast";
> >
> > I think you'll want tf_none instead of 0 /* complain */ in these.
> >
> > Marek
>
> Thanks.
>
> Here's an updated version of the patch.
>
> Changes since v1:
> - updated expected fixit-formatting (the new fix-it printer in
> r247548 handles this properly now)
> - added new test cases as suggested by Florian
> - use "tf_none" rather than "0 /* complain */"
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> * parser.c (get_cast_suggestion): New function.
> (maybe_add_cast_fixit): New function.
> (cp_parser_cast_expression): Capture the location of the
> closing
> parenthesis. Call maybe_add_cast_fixit when emitting warnings
> about old-style casts.
>
> gcc/testsuite/ChangeLog:
> * g++.dg/other/old-style-cast-fixits.C: New test case.
> ---
> gcc/cp/parser.c | 93
> ++++++++++++++++++++-
> gcc/testsuite/g++.dg/other/old-style-cast-fixits.C | 95
> ++++++++++++++++++++++
> 2 files changed, 186 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/other/old-style-cast
> -fixits.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 4714bc6..2f83aa9 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -8633,6 +8633,85 @@ cp_parser_tokens_start_cast_expression
> (cp_parser *parser)
> }
> }
>
> +/* Try to find a legal C++-style cast to DST_TYPE for ORIG_EXPR,
> trying them
> + in the order: const_cast, static_cast, reinterpret_cast.
> +
> + Don't suggest dynamic_cast.
> +
> + Return the first legal cast kind found, or NULL otherwise. */
> +
> +static const char *
> +get_cast_suggestion (tree dst_type, tree orig_expr)
> +{
> + tree trial;
> +
> + /* Reuse the parser logic by attempting to build the various kinds
> of
> + cast, with "complain" disabled.
> + Identify the first such cast that is valid. */
> +
> + /* Don't attempt to run such logic within template processing. */
> + if (processing_template_decl)
> + return NULL;
> +
> + /* First try const_cast. */
> + trial = build_const_cast (dst_type, orig_expr, tf_none);
> + if (trial != error_mark_node)
> + return "const_cast";
> +
> + /* If that fails, try static_cast. */
> + trial = build_static_cast (dst_type, orig_expr, tf_none);
> + if (trial != error_mark_node)
> + return "static_cast";
> +
> + /* Finally, try reinterpret_cast. */
> + trial = build_reinterpret_cast (dst_type, orig_expr, tf_none);
> + if (trial != error_mark_node)
> + return "reinterpret_cast";
> +
> + /* No such cast possible. */
> + return NULL;
> +}
> +
> +/* If -Wold-style-cast is enabled, add fix-its to RICHLOC,
> + suggesting how to convert a C-style cast of the form:
> +
> + (DST_TYPE)ORIG_EXPR
> +
> + to a C++-style cast.
> +
> + The primary range of RICHLOC is asssumed to be that of the
> original
> + expression. OPEN_PAREN_LOC and CLOSE_PAREN_LOC give the
> locations
> + of the parens in the C-style cast. */
> +
> +static void
> +maybe_add_cast_fixit (rich_location *rich_loc, location_t
> open_paren_loc,
> + location_t close_paren_loc, tree orig_expr,
> + tree dst_type)
> +{
> + /* This function is non-trivial, so bail out now if the warning
> isn't
> + going to be emitted. */
> + if (!warn_old_style_cast)
> + return;
> +
> + /* Try to find a legal C++ cast, trying them in order:
> + const_cast, static_cast, reinterpret_cast. */
> + const char *cast_suggestion = get_cast_suggestion (dst_type,
> orig_expr);
> + if (!cast_suggestion)
> + return;
> +
> + /* Replace the open paren with "CAST_SUGGESTION<". */
> + pretty_printer pp;
> + pp_printf (&pp, "%s<", cast_suggestion);
> + rich_loc->add_fixit_replace (open_paren_loc, pp_formatted_text
> (&pp));
> +
> + /* Replace the close paren with "> (". */
> + rich_loc->add_fixit_replace (close_paren_loc, "> (");
> +
> + /* Add a closing paren after the expr (the primary range of
> RICH_LOC). */
> + rich_loc->add_fixit_insert_after (")");
> +}
> +
> +
> /* Parse a cast-expression.
>
> cast-expression:
> @@ -8668,6 +8747,7 @@ cp_parser_cast_expression (cp_parser *parser,
> bool address_p, bool cast_p,
> /* Consume the `('. */
> cp_token *open_paren = cp_lexer_consume_token (parser->lexer);
> location_t open_paren_loc = open_paren->location;
> + location_t close_paren_loc = UNKNOWN_LOCATION;
>
> /* A very tricky bit is that `(struct S) { 3 }' is a
> compound-literal (which we permit in C++ as an extension).
> @@ -8730,7 +8810,10 @@ cp_parser_cast_expression (cp_parser *parser,
> bool address_p, bool cast_p,
> /* Look for the type-id. */
> type = cp_parser_type_id (parser);
> /* Look for the closing `)'. */
> - 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 (close_paren)
> + close_paren_loc = close_paren->location;
> parser->in_type_id_in_expr_p = saved_in_type_id_in_expr_p;
> }
>
> @@ -8760,7 +8843,13 @@ cp_parser_cast_expression (cp_parser *parser,
> bool address_p, bool cast_p,
> && !in_system_header_at (input_location)
> && !VOID_TYPE_P (type)
> && current_lang_name != lang_name_c)
> - warning (OPT_Wold_style_cast, "use of old-style
> cast");
> + {
> + gcc_rich_location rich_loc (input_location);
> + maybe_add_cast_fixit (&rich_loc, open_paren_loc,
> close_paren_loc,
> + expr, type);
> + warning_at_rich_loc (&rich_loc,
> OPT_Wold_style_cast,
> + "use of old-style cast to
> %qT", type);
> + }
>
> /* Only type conversions to integral or enumeration
> types
> can be used in constant-expressions. */
> diff --git a/gcc/testsuite/g++.dg/other/old-style-cast-fixits.C
> b/gcc/testsuite/g++.dg/other/old-style-cast-fixits.C
> new file mode 100644
> index 0000000..a10b623
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/other/old-style-cast-fixits.C
> @@ -0,0 +1,95 @@
> +// { dg-options "-Wold-style-cast -fdiagnostics-show-caret" }
> +
> +struct foo {};
> +struct bar { const foo *field; };
> +
> +void test_1 (void *ptr)
> +{
> + foo *f = (foo *)ptr; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)ptr;
> + ^~~
> + ----------
> + static_cast<foo *> (ptr)
> + { dg-end-multiline-output "" } */
> +}
> +
> +void test_2 (const foo *ptr)
> +{
> + foo *f = (foo *)ptr; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)ptr;
> + ^~~
> + ----------
> + const_cast<foo *> (ptr)
> + { dg-end-multiline-output "" } */
> +}
> +
> +void test_3 (bar *ptr)
> +{
> + foo *f = (foo *)ptr; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)ptr;
> + ^~~
> + ----------
> + reinterpret_cast<foo *> (ptr)
> + { dg-end-multiline-output "" } */
> +}
> +
> +void test_4 (bar *ptr)
> +{
> + foo *f = (foo *)ptr->field; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)ptr->field;
> + ^~~~~
> + -----------------
> + const_cast<foo *> (ptr->field)
> + { dg-end-multiline-output "" } */
> +}
> +
> +void test_5 ()
> +{
> + bar b_inst;
> + foo *f = (foo *)&b_inst; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)&b_inst;
> + ^~~~~~
> + --------------
> + reinterpret_cast<foo *> (&b_inst)
> + { dg-end-multiline-output "" } */
> +}
> +
> +/* We don't offer suggestions for templates. */
> +
> +template <typename T>
> +void test_6 (void *ptr)
> +{
> + foo *f = (foo *)ptr; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)ptr;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +/* We don't offer suggestions where a single C++-style cast can't be
> + used. */
> +
> +void test_7 (const void *ptr)
> +{
> + foo *f = (foo *)ptr; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)ptr;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +/* Likewise, no single C++-style cast is usable here. */
> +
> +void test_8 (const bar &b_inst)
> +{
> + foo *f = (foo *)&b_inst; // { dg-warning "old-style cast" }
> + /* { dg-begin-multiline-output "" }
> + foo *f = (foo *)&b_inst;
> + ^~~~~~
> + { dg-end-multiline-output "" } */
> +}
More information about the Gcc-patches
mailing list