From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 24409385041B for ; Wed, 25 May 2022 12:24:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 24409385041B Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-553-eUUs_JHEOdekDuhWbLOUwg-1; Wed, 25 May 2022 08:24:26 -0400 X-MC-Unique: eUUs_JHEOdekDuhWbLOUwg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6791D3C0CD4E; Wed, 25 May 2022 12:24:26 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 151E22166B29; Wed, 25 May 2022 12:24:25 +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 24PCONKt039519 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 25 May 2022 14:24:23 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24PCOMi6039518; Wed, 25 May 2022 14:24:22 +0200 Date: Wed, 25 May 2022 14:24:22 +0200 From: Jakub Jelinek To: Marek Polacek Cc: "Joseph S. Myers" , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c: Improve build_component_ref diagnostics [PR91134] Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.7 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-List-Received-Date: Wed, 25 May 2022 12:24:31 -0000 On Tue, May 24, 2022 at 09:43:54AM -0400, Marek Polacek wrote: > Consider extending the test like Done, + added the dg-*-multiline-output stuff to test the fixit hints. Thanks. Here is what I've committed: 2022-05-25 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 diagnostic 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. --- gcc/c/c-tree.h.jj 2022-05-25 11:06:58.381515989 +0200 +++ gcc/c/c-tree.h 2022-05-25 13:57:58.975539087 +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-25 11:06:58.388515915 +0200 +++ gcc/c/c-typeck.cc 2022-05-25 14:19:16.486336943 +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 the 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-25 11:06:58.355516262 +0200 +++ gcc/c/c-parser.cc 2022-05-25 13:57:58.980539036 +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-05-24 08:59:33.375472523 +0200 +++ gcc/c/gimple-parser.cc 2022-05-25 13:57:59.000538828 +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 11:58:59.693980471 +0100 +++ gcc/objc/objc-act.cc 2022-05-25 13:57:59.018538641 +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-25 13:57:59.018538641 +0200 +++ gcc/testsuite/gcc.dg/pr91134.c 2022-05-25 14:17:17.000576752 +0200 @@ -0,0 +1,32 @@ +/* PR c/91134 */ +/* { dg-options "-fdiagnostics-show-caret" } */ + +struct X { int member; } x; +struct Y { struct X **x; } y; + +int +foo (void) +{ + struct X *pointer = &x; + struct Y *yp = &y; + 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\\\?" } */ +/* { dg-begin-multiline-output "" } + int i = *pointerpointer->member; + ^~ + (* ) + { dg-end-multiline-output "" } */ + int j = pointer.member; /* { dg-error "'pointer' is a pointer; did you mean to use '->'\\\?" } */ +/* { dg-begin-multiline-output "" } + int j = pointer.member; + ^ + -> + { dg-end-multiline-output "" } */ + int k = yp->x->member; /* { dg-error "'yp->x' is a pointer to pointer; did you mean to dereference it before applying '->' to it\\\?" } */ +/* { dg-begin-multiline-output "" } + int k = yp->x->member; + ^~ + (* ) + { dg-end-multiline-output "" } */ + return i + j + k; +} Jakub