From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 95903 invoked by alias); 3 May 2017 13:20:02 -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 95822 invoked by uid 89); 3 May 2017 13:20:00 -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,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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; Wed, 03 May 2017 13:19:58 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7CB6779BE2; Wed, 3 May 2017 13:19:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7CB6779BE2 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@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7CB6779BE2 Received: from c64.redhat.com (ovpn-112-30.phx2.redhat.com [10.3.112.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6F21217D5B; Wed, 3 May 2017 13:19:55 +0000 (UTC) From: David Malcolm To: Nathan Sidwell Cc: Marek Polacek , Volker Reichelt , gcc-patches@gcc.gnu.org, David Malcolm Subject: [PATCH v2] C++: Add fix-it hints for -Wold-style-cast Date: Wed, 03 May 2017 13:32:00 -0000 Message-Id: <1493819469-38154-1-git-send-email-dmalcolm@redhat.com> In-Reply-To: <20170427210337.GF4255@redhat.com> References: <20170427210337.GF4255@redhat.com> X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00205.txt.bz2 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 "" } */ +} -- 1.8.5.3