From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114622 invoked by alias); 20 Jun 2017 14:03:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 114599 invoked by uid 89); 20 Jun 2017 14:03:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*M:168, 86686 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Jun 2017 14:03:28 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ACCC37CE14; Tue, 20 Jun 2017 14:03:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com ACCC37CE14 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=dmalcolm@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com ACCC37CE14 Received: from ovpn-116-74.phx2.redhat.com (ovpn-116-74.phx2.redhat.com [10.3.116.74]) by smtp.corp.redhat.com (Postfix) with ESMTP id CE3C6709E4; Tue, 20 Jun 2017 14:03:19 +0000 (UTC) Message-ID: <1497967399.7551.168.camel@redhat.com> Subject: [PING^3] re [PATCH v2] C++: Add fix-it hints for -Wold-style-cast From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: Marek Polacek , Volker Reichelt , Nathan Sidwell Date: Tue, 20 Jun 2017 14:03:00 -0000 In-Reply-To: <1496680878.7551.97.camel@redhat.com> References: <20170427210337.GF4255@redhat.com> <1493819469-38154-1-git-send-email-dmalcolm@redhat.com> <1495827302.9289.77.camel@redhat.com> <1496680878.7551.97.camel@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg01465.txt.bz2 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 (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 (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 (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 (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 (&b_inst) > > > + { dg-end-multiline-output "" } */ > > > +} > > > + > > > +/* We don't offer suggestions for templates. */ > > > + > > > +template > > > +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 "" } */ > > > +}