From patchwork Tue May 24 07:25:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 54331 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6117F3857419 for ; Tue, 24 May 2022 07:26:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6117F3857419 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1653377196; bh=65erQjJntwcv4SHspnDQoYyFdiUDh8TGp3NDWVtYDvg=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=yMqX3zappyHbauOg2ohY7Y9nvNU2b+Gnr6Iu1dqyr/UlZvWgoIhOK0UWh3spT92j6 T3EKxbGIhR6zQroBuCZdPqmLjbk3O9m+Wez1555dpiNbuv/WDP6E/KjPu/jnsm/mCZ N3VxH8B/pRygqBo9W61XkSSg1E5lhMv7q090OECU= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0C0253857419 for ; Tue, 24 May 2022 07:26:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0C0253857419 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-421-m6oYkScyPB69aHvI8qr5xA-1; Tue, 24 May 2022 03:26:02 -0400 X-MC-Unique: m6oYkScyPB69aHvI8qr5xA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CFA3A85A5AA; Tue, 24 May 2022 07:26:01 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8D95A401E4C; Tue, 24 May 2022 07:26:01 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 24O7Pw0h2095353 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 24 May 2022 09:25:59 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24O7PwrL2095352; Tue, 24 May 2022 09:25:58 +0200 Date: Tue, 24 May 2022 09:25:57 +0200 To: "Joseph S. Myers" , Marek Polacek Subject: [PATCH] c: Improve build_component_ref diagnostics [PR91134] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jakub Jelinek via Gcc-patches From: Jakub Jelinek Reply-To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi! On the following testcase (the first dg-error line) we emit a weird diagnostics and even fixit on pointerpointer->member where pointerpointer is pointer to pointer to struct and we say 'pointerpointer' is a pointer; did you mean to use '->'? The first part is indeed true, but suggesting -> when the code already does use -> is confusing. The following patch adjusts callers so that they tell it if it is from . parsing or from -> parsing and in the latter case suggests to dereference the left operand instead by adding (* before it and ) after it (before ->). Or would a suggestion to add [0] before -> be better? Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2022-05-24 Jakub Jelinek PR c/91134 gcc/c/ * c-tree.h (build_component_ref): Add ARROW_LOC location_t argument. * c-typeck.cc (build_component_ref): Likewise. If DATUM is INDIRECT_REF and ARROW_LOC isn't UNKNOWN_LOCATION, print a different diagnostics and fixit hint if DATUM has pointer type. * c-parser.cc (c_parser_postfix_expression, c_parser_omp_variable_list): Adjust build_component_ref callers. * gimple-parser.cc (c_parser_gimple_postfix_expression_after_primary): Likewise. gcc/objc/ * objc-act.cc (objc_build_component_ref): Adjust build_component_ref caller. gcc/testsuite/ * gcc.dg/pr91134.c: New test. Jakub --- gcc/c/c-tree.h.jj 2022-05-19 11:48:56.058291437 +0200 +++ gcc/c/c-tree.h 2022-05-23 20:22:05.669515990 +0200 @@ -699,7 +699,8 @@ extern struct c_expr convert_lvalue_to_r extern tree decl_constant_value_1 (tree, bool); extern void mark_exp_read (tree); extern tree composite_type (tree, tree); -extern tree build_component_ref (location_t, tree, tree, location_t); +extern tree build_component_ref (location_t, tree, tree, location_t, + location_t); extern tree build_array_ref (location_t, tree, tree); extern tree build_external_ref (location_t, tree, bool, tree *); extern void pop_maybe_used (bool); --- gcc/c/c-typeck.cc.jj 2022-05-19 11:48:56.077291176 +0200 +++ gcc/c/c-typeck.cc 2022-05-23 20:23:44.713515875 +0200 @@ -2457,11 +2457,12 @@ should_suggest_deref_p (tree datum_type) /* Make an expression to refer to the COMPONENT field of structure or union value DATUM. COMPONENT is an IDENTIFIER_NODE. LOC is the location of the COMPONENT_REF. COMPONENT_LOC is the location - of COMPONENT. */ + of COMPONENT. ARROW_LOC is the location of first -> operand if + it is from -> operator. */ tree build_component_ref (location_t loc, tree datum, tree component, - location_t component_loc) + location_t component_loc, location_t arrow_loc) { tree type = TREE_TYPE (datum); enum tree_code code = TREE_CODE (type); @@ -2577,11 +2578,23 @@ build_component_ref (location_t loc, tre /* Special-case the error message for "ptr.field" for the case where the user has confused "." vs "->". */ rich_location richloc (line_table, loc); - /* "loc" should be the "." token. */ - richloc.add_fixit_replace ("->"); - error_at (&richloc, - "%qE is a pointer; did you mean to use %<->%>?", - datum); + if (TREE_CODE (datum) == INDIRECT_REF && arrow_loc != UNKNOWN_LOCATION) + { + richloc.add_fixit_insert_before (arrow_loc, "(*"); + richloc.add_fixit_insert_after (arrow_loc, ")"); + error_at (&richloc, + "%qE is a pointer to pointer; did you mean to dereference " + "it before applying %<->%> to it?", + TREE_OPERAND (datum, 0)); + } + else + { + /* "loc" should be the "." token. */ + richloc.add_fixit_replace ("->"); + error_at (&richloc, + "%qE is a pointer; did you mean to use %<->%>?", + datum); + } return error_mark_node; } else if (code != ERROR_MARK) --- gcc/c/c-parser.cc.jj 2022-05-23 16:16:30.360856580 +0200 +++ gcc/c/c-parser.cc 2022-05-23 20:33:36.683537409 +0200 @@ -9235,8 +9235,9 @@ c_parser_postfix_expression (c_parser *p if (c_parser_next_token_is (parser, CPP_NAME)) { c_token *comp_tok = c_parser_peek_token (parser); - offsetof_ref = build_component_ref - (loc, offsetof_ref, comp_tok->value, comp_tok->location); + offsetof_ref + = build_component_ref (loc, offsetof_ref, comp_tok->value, + comp_tok->location, UNKNOWN_LOCATION); c_parser_consume_token (parser); while (c_parser_next_token_is (parser, CPP_DOT) || c_parser_next_token_is (parser, @@ -9263,9 +9264,11 @@ c_parser_postfix_expression (c_parser *p break; } c_token *comp_tok = c_parser_peek_token (parser); - offsetof_ref = build_component_ref - (loc, offsetof_ref, comp_tok->value, - comp_tok->location); + offsetof_ref + = build_component_ref (loc, offsetof_ref, + comp_tok->value, + comp_tok->location, + UNKNOWN_LOCATION); c_parser_consume_token (parser); } else @@ -10612,7 +10615,7 @@ c_parser_postfix_expression_after_primar finish = c_parser_peek_token (parser)->get_finish (); c_parser_consume_token (parser); expr.value = build_component_ref (op_loc, expr.value, ident, - comp_loc); + comp_loc, UNKNOWN_LOCATION); set_c_expr_source_range (&expr, start, finish); expr.original_code = ERROR_MARK; if (TREE_CODE (expr.value) != COMPONENT_REF) @@ -10652,7 +10655,8 @@ c_parser_postfix_expression_after_primar build_indirect_ref (op_loc, expr.value, RO_ARROW), - ident, comp_loc); + ident, comp_loc, + expr.get_location ()); set_c_expr_source_range (&expr, start, finish); expr.original_code = ERROR_MARK; if (TREE_CODE (expr.value) != COMPONENT_REF) @@ -13171,6 +13175,7 @@ c_parser_omp_variable_list (c_parser *pa && c_parser_next_token_is (parser, CPP_DEREF))) { location_t op_loc = c_parser_peek_token (parser)->location; + location_t arrow_loc = UNKNOWN_LOCATION; if (c_parser_next_token_is (parser, CPP_DEREF)) { c_expr t_expr; @@ -13181,6 +13186,7 @@ c_parser_omp_variable_list (c_parser *pa t_expr = convert_lvalue_to_rvalue (op_loc, t_expr, true, false); t = build_indirect_ref (op_loc, t_expr.value, RO_ARROW); + arrow_loc = t_expr.get_location (); } c_parser_consume_token (parser); if (!c_parser_next_token_is (parser, CPP_NAME)) @@ -13194,7 +13200,8 @@ c_parser_omp_variable_list (c_parser *pa tree ident = comp_tok->value; location_t comp_loc = comp_tok->location; c_parser_consume_token (parser); - t = build_component_ref (op_loc, t, ident, comp_loc); + t = build_component_ref (op_loc, t, ident, comp_loc, + arrow_loc); } /* FALLTHROUGH */ case OMP_CLAUSE_AFFINITY: --- gcc/c/gimple-parser.cc.jj 2022-02-24 15:27:14.718744040 +0100 +++ gcc/c/gimple-parser.cc 2022-05-23 20:27:34.538195175 +0200 @@ -1800,7 +1800,7 @@ c_parser_gimple_postfix_expression_after finish = c_parser_peek_token (parser)->get_finish (); c_parser_consume_token (parser); expr.value = build_component_ref (op_loc, expr.value, ident, - comp_loc); + comp_loc, UNKNOWN_LOCATION); set_c_expr_source_range (&expr, start, finish); expr.original_code = ERROR_MARK; if (TREE_CODE (expr.value) != COMPONENT_REF) @@ -1848,7 +1848,8 @@ c_parser_gimple_postfix_expression_after expr.value = build_component_ref (op_loc, build_simple_mem_ref_loc (op_loc, expr.value), - ident, comp_loc); + ident, comp_loc, + expr.get_location ()); set_c_expr_source_range (&expr, start, finish); expr.original_code = ERROR_MARK; if (TREE_CODE (expr.value) != COMPONENT_REF) --- gcc/objc/objc-act.cc.jj 2022-01-18 00:18:02.827743339 +0100 +++ gcc/objc/objc-act.cc 2022-05-23 23:59:48.134923529 +0200 @@ -2812,7 +2812,7 @@ objc_build_component_ref (tree datum, tr tf_warning_or_error); #else return build_component_ref (input_location, datum, component, - UNKNOWN_LOCATION); + UNKNOWN_LOCATION, UNKNOWN_LOCATION); #endif } --- gcc/testsuite/gcc.dg/pr91134.c.jj 2022-05-23 20:31:11.751001817 +0200 +++ gcc/testsuite/gcc.dg/pr91134.c 2022-05-23 20:30:45.291268997 +0200 @@ -0,0 +1,13 @@ +/* PR c/91134 */ + +struct X { int member; } x; + +int +foo (void) +{ + struct X *pointer = &x; + struct X **pointerpointer = &pointer; + int i = *pointerpointer->member; /* { dg-error "'pointerpointer' is a pointer to pointer; did you mean to dereference it before applying '->' to it\\\?" } */ + int j = pointer.member; /* { dg-error "'pointer' is a pointer; did you mean to use '->'\\\?" } */ + return i + j; +}