This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH v2] C++: Add fix-it hints for -Wold-style-cast
- From: David Malcolm <dmalcolm at redhat dot com>
- To: Nathan Sidwell <nathan at acm dot org>
- Cc: Marek Polacek <polacek at redhat dot com>, Volker Reichelt <v dot reichelt at netcologne dot de>, gcc-patches at gcc dot gnu dot org, David Malcolm <dmalcolm at redhat dot com>
- Date: Wed, 3 May 2017 09:51:09 -0400
- Subject: [PATCH v2] C++: Add fix-it hints for -Wold-style-cast
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=dmalcolm at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7CB6779BE2
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7CB6779BE2
- References: <20170427210337.GF4255@redhat.com>
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 "" } */
+}
--
1.8.5.3