From patchwork Thu Jul 12 02:42:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 942750 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-481397-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="VCy8WSae"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41QzcS30Btz9s19 for ; Thu, 12 Jul 2018 11:58:29 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id; q=dns; s=default; b=YM1kMtpWyVEg q0d5/6smaQNoihRCr7q2N7AOmtXwtUXhKej49nJxkBMTY9tMI3tvguFrZlzc2T4U zf9qbvhod+TG8cPHzWUpSqVSshNlg2H+/2ya+6FXUWLoa6Lb1mYAlyjYirU17qCC 1CnPO5cQCyp7teL17MVasjggE+vczf0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id; s=default; bh=f01g3/WxTxa2YI6Utv Zr9iJaEKk=; b=VCy8WSae/uwi4ryh3kJbf5ecZHH/rwLDX468LdgMqopcF1/Q9v yuGy2L+DpZ2zBG8g2CgBMwZeX+PEL9DQIyr1oW7Pbdt2wflcUjYAGEvBXsBqTvea QlJqd8RY3R6klHdZLejc6gVqRp3KPvBUvwPirAxvSKB5gliv+1vWP5o1Q= Received: (qmail 52829 invoked by alias); 12 Jul 2018 01:58:20 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 52797 invoked by uid 89); 12 Jul 2018 01:58:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=colour, sk:accesso, ratio, 1119 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; Thu, 12 Jul 2018 01:58:15 +0000 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7372B87621 for ; Thu, 12 Jul 2018 01:58:14 +0000 (UTC) Received: from c64.redhat.com (ovpn-112-31.phx2.redhat.com [10.3.112.31]) by smtp.corp.redhat.com (Postfix) with ESMTP id 472A5308BDB0; Thu, 12 Jul 2018 01:58:13 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [PATCH] C++: suggestions for misspelled private members (PR c++/84993) Date: Wed, 11 Jul 2018 22:42:25 -0400 Message-Id: <1531363345-50981-1-git-send-email-dmalcolm@redhat.com> X-IsSubscribed: yes PR c++/84993 identifies a problem with our suggestions for misspelled member names in the C++ FE for the case where the member is private. For example, given: class foo { public: double get_ratio() const { return m_ratio; } private: double m_ratio; }; void test(foo *ptr) { if (ptr->ratio >= 0.5) ;// etc } ...we currently emit this suggestion: : In function 'void test(foo*)': :12:12: error: 'class foo' has no member named 'ratio'; did you mean 'm_ratio'? if (ptr->ratio >= 0.5) ^~~~~ m_ratio ...but if the user follows this suggestion, they get: : In function 'void test(foo*)': :12:12: error: 'double foo::m_ratio' is private within this context if (ptr->m_ratio >= 0.5) ^~~~~~~ :7:10: note: declared private here double m_ratio; ^~~~~~~ :12:12: note: field 'double foo::m_ratio' can be accessed via 'double foo::get_ratio() const' if (ptr->m_ratio >= 0.5) ^~~~~~~ get_ratio() It feels wrong to be emitting a fix-it hint that doesn't compile, so this patch adds the accessor fix-it hint logic to this case, so that we directly offer a valid suggestion: : In function 'void test(foo*)': :12:12: error: 'class foo' has no member named 'ratio'; did you mean 'double foo::m_ratio'? (accessible via 'double foo::get_ratio() const') if (ptr->ratio >= 0.5) ^~~~~ get_ratio() Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; adds 105 PASS results to g++.sum. OK for trunk? gcc/cp/ChangeLog: PR c++/84993 * call.c (enforce_access): Move diagnostics to... (complain_about_access): ...this new function. * cp-tree.h (class access_failure_info): Rename split out field "m_field_decl" into "m_decl" and "m_diag_decl". (access_failure_info::record_access_failure): Add tree param. (access_failure_info::was_inaccessible_p): New accessor. (access_failure_info::get_decl): New accessor. (access_failure_info::get_diag_decl): New accessor. (access_failure_info::get_any_accessor): New member function. (access_failure_info::add_fixit_hint): New static member function. (complain_about_access): New decl. * typeck.c (access_failure_info::record_access_failure): Update for change to fields. (access_failure_info::maybe_suggest_accessor): Split out into... (access_failure_info::get_any_accessor): ...this new function... (access_failure_info::add_fixit_hint): ...and this new function. (finish_class_member_access_expr): Split out "has no member named" error-handling into... (complain_about_unrecognized_member): ...this new function, and check that the guessed name is accessible along the access path. Only provide a spell-correction fix-it hint if it is; otherwise, attempt to issue an accessor fix-it hint. gcc/testsuite/ChangeLog: PR c++/84993 * g++.dg/torture/accessor-fixits-9.C: New test. --- gcc/cp/call.c | 64 ++++++---- gcc/cp/cp-tree.h | 17 ++- gcc/cp/typeck.c | 150 +++++++++++++++++------ gcc/testsuite/g++.dg/torture/accessor-fixits-9.C | 119 ++++++++++++++++++ 4 files changed, 282 insertions(+), 68 deletions(-) create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-9.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 209c1fd..121f0ca 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -6456,6 +6456,38 @@ build_op_delete_call (enum tree_code code, tree addr, tree size, return error_mark_node; } +/* Issue diagnostics about a disallowed access of DECL, using DIAG_DECL + in the diagnostics. + + If ISSUE_ERROR is true, then issue an error about the + access, followed by a note showing the declaration. + Otherwise, just show the note. */ + +void +complain_about_access (tree decl, tree diag_decl, bool issue_error) +{ + if (TREE_PRIVATE (decl)) + { + if (issue_error) + error ("%q#D is private within this context", diag_decl); + inform (DECL_SOURCE_LOCATION (diag_decl), + "declared private here"); + } + else if (TREE_PROTECTED (decl)) + { + if (issue_error) + error ("%q#D is protected within this context", diag_decl); + inform (DECL_SOURCE_LOCATION (diag_decl), + "declared protected here"); + } + else + { + if (issue_error) + error ("%q#D is inaccessible within this context", diag_decl); + inform (DECL_SOURCE_LOCATION (diag_decl), "declared here"); + } +} + /* If the current scope isn't allowed to access DECL along BASETYPE_PATH, give an error. The most derived class in BASETYPE_PATH is the one used to qualify DECL. DIAG_DECL is @@ -6480,34 +6512,12 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl, if (!accessible_p (basetype_path, decl, true)) { + if (flag_new_inheriting_ctors) + diag_decl = strip_inheriting_ctors (diag_decl); if (complain & tf_error) - { - if (flag_new_inheriting_ctors) - diag_decl = strip_inheriting_ctors (diag_decl); - if (TREE_PRIVATE (decl)) - { - error ("%q#D is private within this context", diag_decl); - inform (DECL_SOURCE_LOCATION (diag_decl), - "declared private here"); - if (afi) - afi->record_access_failure (basetype_path, diag_decl); - } - else if (TREE_PROTECTED (decl)) - { - error ("%q#D is protected within this context", diag_decl); - inform (DECL_SOURCE_LOCATION (diag_decl), - "declared protected here"); - if (afi) - afi->record_access_failure (basetype_path, diag_decl); - } - else - { - error ("%q#D is inaccessible within this context", diag_decl); - inform (DECL_SOURCE_LOCATION (diag_decl), "declared here"); - if (afi) - afi->record_access_failure (basetype_path, diag_decl); - } - } + complain_about_access (decl, diag_decl, true); + if (afi) + afi->record_access_failure (basetype_path, decl, diag_decl); return false; } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 0fac7e9..9b7a497 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6054,18 +6054,27 @@ extern location_t get_fndecl_argument_location (tree, int); class access_failure_info { public: - access_failure_info () : m_was_inaccessible (false), m_basetype_path (NULL_TREE), - m_field_decl (NULL_TREE) {} + access_failure_info () : m_was_inaccessible (false), + m_basetype_path (NULL_TREE), + m_decl (NULL_TREE), m_diag_decl (NULL_TREE) {} - void record_access_failure (tree basetype_path, tree field_decl); + void record_access_failure (tree basetype_path, tree decl, tree diag_decl); + + bool was_inaccessible_p () const { return m_was_inaccessible; } + tree get_decl () const { return m_decl; } + tree get_diag_decl () const { return m_diag_decl; } + tree get_any_accessor (bool const_p) const; void maybe_suggest_accessor (bool const_p) const; + static void add_fixit_hint (rich_location *richloc, tree accessor); private: bool m_was_inaccessible; tree m_basetype_path; - tree m_field_decl; + tree m_decl; + tree m_diag_decl; }; +extern void complain_about_access (tree, tree, bool); extern bool enforce_access (tree, tree, tree, tsubst_flags_t, access_failure_info *afi = NULL); diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index ea4ce96..d03e797 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -2703,43 +2703,138 @@ check_template_keyword (tree decl) } /* Record that an access failure occurred on BASETYPE_PATH attempting - to access FIELD_DECL. */ + to access DECL, where DIAG_DECL should be used for diagnostics. */ void access_failure_info::record_access_failure (tree basetype_path, - tree field_decl) + tree decl, tree diag_decl) { m_was_inaccessible = true; m_basetype_path = basetype_path; - m_field_decl = field_decl; + m_decl = decl; + m_diag_decl = diag_decl; } /* If an access failure was recorded, then attempt to locate an - accessor function for the pertinent field, and if one is - available, add a note and fix-it hint suggesting using it. */ + accessor function for the pertinent field. + Otherwise, return NULL_TREE. */ -void -access_failure_info::maybe_suggest_accessor (bool const_p) const +tree +access_failure_info::get_any_accessor (bool const_p) const { - if (!m_was_inaccessible) - return; + if (!was_inaccessible_p ()) + return NULL_TREE; tree accessor - = locate_field_accessor (m_basetype_path, m_field_decl, const_p); + = locate_field_accessor (m_basetype_path, m_diag_decl, const_p); if (!accessor) - return; + return NULL_TREE; /* The accessor must itself be accessible for it to be a reasonable suggestion. */ if (!accessible_p (m_basetype_path, accessor, true)) - return; + return NULL_TREE; - rich_location richloc (line_table, input_location); + return accessor; +} + +/* Add a fix-it hint to RICHLOC suggesting the use of ACCESSOR_DECL, by + replacing the primary location in RICHLOC with "accessor()". */ + +void +access_failure_info::add_fixit_hint (rich_location *richloc, + tree accessor_decl) +{ pretty_printer pp; - pp_printf (&pp, "%s()", IDENTIFIER_POINTER (DECL_NAME (accessor))); - richloc.add_fixit_replace (pp_formatted_text (&pp)); + pp_printf (&pp, "%s()", IDENTIFIER_POINTER (DECL_NAME (accessor_decl))); + richloc->add_fixit_replace (pp_formatted_text (&pp)); +} + +/* If an access failure was recorded, then attempt to locate an + accessor function for the pertinent field, and if one is + available, add a note and fix-it hint suggesting using it. */ + +void +access_failure_info::maybe_suggest_accessor (bool const_p) const +{ + tree accessor = get_any_accessor (const_p); + if (accessor == NULL_TREE) + return; + rich_location richloc (line_table, input_location); + add_fixit_hint (&richloc, accessor); inform (&richloc, "field %q#D can be accessed via %q#D", - m_field_decl, accessor); + m_diag_decl, accessor); +} + +/* Subroutine of finish_class_member_access_expr. + Issue an error about NAME not being a member of ACCESS_PATH (or + OBJECT_TYPE), potentially providing a fix-it hint for misspelled + names. */ + +static void +complain_about_unrecognized_member (tree access_path, tree name, + tree object_type) +{ + /* Attempt to provide a hint about misspelled names. */ + tree guessed_id = lookup_member_fuzzy (access_path, name, + /*want_type=*/false); + if (guessed_id == NULL_TREE) + { + /* No hint. */ + error ("%q#T has no member named %qE", + TREE_CODE (access_path) == TREE_BINFO + ? TREE_TYPE (access_path) : object_type, name); + return; + } + + location_t bogus_component_loc = input_location; + gcc_rich_location rich_loc (bogus_component_loc); + + /* Check that the guessed name is accessible along access_path. */ + access_failure_info afi; + lookup_member (access_path, guessed_id, /*protect=*/1, + /*want_type=*/false, /*complain=*/false, + &afi); + if (afi.was_inaccessible_p ()) + { + tree accessor = afi.get_any_accessor (TYPE_READONLY (object_type)); + if (accessor) + { + /* The guessed name isn't directly accessible, but can be accessed + via an accessor member function. */ + afi.add_fixit_hint (&rich_loc, accessor); + error_at (&rich_loc, + "%q#T has no member named %qE;" + " did you mean %q#D? (accessible via %q#D)", + TREE_CODE (access_path) == TREE_BINFO + ? TREE_TYPE (access_path) : object_type, + name, afi.get_diag_decl (), accessor); + } + else + { + /* The guessed name isn't directly accessible, and no accessor + member function could be found. */ + error_at (&rich_loc, + "%q#T has no member named %qE;" + " did you mean %q#D? (not accessible from this context)", + TREE_CODE (access_path) == TREE_BINFO + ? TREE_TYPE (access_path) : object_type, + name, afi.get_diag_decl ()); + complain_about_access (afi.get_decl (), afi.get_diag_decl (), false); + } + } + else + { + /* The guessed name is directly accessible; suggest it. */ + rich_loc.add_fixit_misspelled_id (bogus_component_loc, + guessed_id); + error_at (&rich_loc, + "%q#T has no member named %qE;" + " did you mean %qE?", + TREE_CODE (access_path) == TREE_BINFO + ? TREE_TYPE (access_path) : object_type, + name, guessed_id); + } } /* This function is called by the parser to process a class member @@ -2935,27 +3030,8 @@ finish_class_member_access_expr (cp_expr object, tree name, bool template_p, /* Try again at instantiation time. */ goto dependent; if (complain & tf_error) - { - tree guessed_id = lookup_member_fuzzy (access_path, name, - /*want_type=*/false); - if (guessed_id) - { - location_t bogus_component_loc = input_location; - gcc_rich_location rich_loc (bogus_component_loc); - rich_loc.add_fixit_misspelled_id (bogus_component_loc, - guessed_id); - error_at (&rich_loc, - "%q#T has no member named %qE;" - " did you mean %qE?", - TREE_CODE (access_path) == TREE_BINFO - ? TREE_TYPE (access_path) : object_type, - name, guessed_id); - } - else - error ("%q#T has no member named %qE", - TREE_CODE (access_path) == TREE_BINFO - ? TREE_TYPE (access_path) : object_type, name); - } + complain_about_unrecognized_member (access_path, name, + object_type); return error_mark_node; } if (member == error_mark_node) diff --git a/gcc/testsuite/g++.dg/torture/accessor-fixits-9.C b/gcc/testsuite/g++.dg/torture/accessor-fixits-9.C new file mode 100644 index 0000000..276c5b8 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/accessor-fixits-9.C @@ -0,0 +1,119 @@ +// PR c++/84993 +// { dg-options "-fdiagnostics-show-caret" } + +/* Misspelling (by omitting a leading "m_") of a private member for which + there's a public accessor. + + We expect a fix-it hint suggesting the accessor. */ + +class t1 +{ +public: + int get_ratio () const { return m_ratio; } + +private: + int m_ratio; +}; + +int test (t1 *ptr_1) +{ + return ptr_1->ratio; // { dg-error "'class t1' has no member named 'ratio'; did you mean 'int t1::m_ratio'\\? \\(accessible via 'int t1::get_ratio\\(\\) const'\\)" } + /* { dg-begin-multiline-output "" } + return ptr_1->ratio; + ^~~~~ + get_ratio() + { dg-end-multiline-output "" } */ +} + + +/* Misspelling of a private member for which there's a public accessor. + + We expect a fix-it hint suggesting the accessor. */ + +class t2 +{ +public: + int get_color () const { return m_color; } + +private: + int m_color; +}; + +int test (t2 *ptr_2) +{ + return ptr_2->m_colour; // { dg-error "'class t2' has no member named 'm_colour'; did you mean 'int t2::m_color'\\? \\(accessible via 'int t2::get_color\\(\\) const'\\)" } + /* { dg-begin-multiline-output "" } + return ptr_2->m_colour; + ^~~~~~~~ + get_color() + { dg-end-multiline-output "" } */ +} + + +/* Misspelling of a private member via a subclass pointer, for which there's + a public accessor in the base class. + + We expect a fix-it hint suggesting the accessor. */ + +class t3 : public t2 {}; + +int test (t3 *ptr_3) +{ + return ptr_3->colour; // { dg-error "'class t3' has no member named 'colour'; did you mean 'int t2::m_color'\\? \\(accessible via 'int t2::get_color\\(\\) const'\\)" } + /* { dg-begin-multiline-output "" } + return ptr_3->colour; + ^~~~~~ + get_color() + { dg-end-multiline-output "" } */ +} + + +/* Misspelling of a protected member, for which there's isn't a public + accessor. + + We expect no fix-it hint; instead a message identifying where the + data member was declared. */ + +class t4 +{ +protected: + int m_color; // { dg-message "declared protected here" } +}; + +int test (t4 *ptr_4) +{ + return ptr_4->colour; // { dg-error "'class t4' has no member named 'colour'; did you mean 'int t4::m_color'\\? \\(not accessible from this context\\)" } + /* { dg-begin-multiline-output "" } + return ptr_4->colour; + ^~~~~~ + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ +} + + +/* Misspelling of a private member, for which the accessor is also private. + + We expect no fix-it hint; instead a message identifying where the + data member was declared. */ + +class t5 +{ + int get_color () const { return m_color; } + int m_color; // { dg-message "declared private here" } +}; + +int test (t5 *ptr_5) +{ + return ptr_5->colour; // { dg-error "'class t5' has no member named 'colour'; did you mean 'int t5::m_color'\\? \\(not accessible from this context\\)" } + /* { dg-begin-multiline-output "" } + return ptr_5->colour; + ^~~~~~ + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ +}