From: David Malcolm <dmalcolm@redhat.com>
To: gcc-patches@gcc.gnu.org
Cc: Marek Polacek <polacek@redhat.com>,
Volker Reichelt <v.reichelt@netcologne.de>,
Nathan Sidwell <nathan@acm.org>
Subject: [PING^3] re [PATCH v2] C++: Add fix-it hints for -Wold-style-cast
Date: Tue, 20 Jun 2017 14:03:00 -0000 [thread overview]
Message-ID: <1497967399.7551.168.camel@redhat.com> (raw)
In-Reply-To: <1496680878.7551.97.camel@redhat.com>
Ping re this patch:
https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00204.html
(more description can be seen in v1 of the patch here:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01429.html
)
On Mon, 2017-06-05 at 12:41 -0400, David Malcolm wrote:
> Ping re this patch:
>
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00204.html
>
> On Fri, 2017-05-26 at 15:35 -0400, David Malcolm wrote:
> > 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 "" } */
> > > +}
next prev parent reply other threads:[~2017-06-20 14:03 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-27 8:00 [PATCH] Improved diagnostics for casts and enums Volker Reichelt
2017-04-27 12:37 ` Nathan Sidwell
2017-04-27 20:58 ` [PATCH] C++: Add fix-it hints for -Wold-style-cast David Malcolm
2017-04-27 21:05 ` Florian Weimer
2017-04-27 22:05 ` Marek Polacek
2017-05-03 13:32 ` [PATCH v2] " David Malcolm
2017-05-26 19:54 ` [PING] re " David Malcolm
2017-06-05 16:41 ` [PING^2] " David Malcolm
2017-06-20 14:03 ` David Malcolm [this message]
2017-06-20 18:39 ` Jason Merrill
2017-04-27 16:59 ` [PATCH] Improved diagnostics for casts and enums Martin Sebor
2017-04-28 8:42 ` Volker Reichelt
2017-04-30 17:54 ` Volker Reichelt
2017-06-20 16:10 ` Martin Sebor
2017-07-10 16:01 ` [PING #2] " Martin Sebor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1497967399.7551.168.camel@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=nathan@acm.org \
--cc=polacek@redhat.com \
--cc=v.reichelt@netcologne.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).