From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126427 invoked by alias); 5 May 2017 22:18:57 -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 126412 invoked by uid 89); 5 May 2017 22:18:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No 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,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; Fri, 05 May 2017 22:18:51 +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 E259E80F94; Fri, 5 May 2017 22:18:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E259E80F94 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=dmalcolm@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E259E80F94 Received: from c64.redhat.com (ovpn-112-30.phx2.redhat.com [10.3.112.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5B1AD7D644; Fri, 5 May 2017 22:18:51 +0000 (UTC) From: David Malcolm To: Jason Merrill , Nathan Sidwell Cc: gcc-patches List , David Malcolm Subject: [PATCH v2] C++: fix-it hints suggesting accessors for private fields Date: Fri, 05 May 2017 23:26:00 -0000 Message-Id: <1494024614-18181-1-git-send-email-dmalcolm@redhat.com> In-Reply-To: References: X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00463.txt.bz2 On Mon, 2017-05-01 at 14:43 -0400, Jason Merrill wrote: > On Thu, Apr 27, 2017 at 7:23 AM, Nathan Sidwell > wrote: > > On 04/26/2017 12:34 PM, David Malcolm wrote: > > > > > Thanks - yes; that gives information on the const vs non-const of > > > the > > > "this" parameter, but doesn't say whether the argument was const > > > vs non > > > -const. > > > > > > > However, within: > > > > > > int test_const_ptr (const t1 *ptr) > > > { > > > return ptr->m_color; > > > } > > > from which we can see the const-ness of the t1: > > > > > > correct. > > > > > but the call to lookup_member from within > > > finish_class_member_access_expr discards this information, giving > > > just > > > "access_path": a BINFO that wraps the RECORD_TYPE for t1 > > > directly. > > > > > > Correct. > > > > lookup_member just looks for a matching name. the BINFO represents > > the > > class hierarchy - it's not modified depending on the cvquals of > > where you > > came from. > > > > > A somewhat invasive solution would be for lookup_member to grow > > > an extra: > > > tree object > > > parameter, and to pass this information down through the access > > > -enforcement code, so that locate_field_accessor can look at the > > > const > > > -ness of the lookup, and avoid suggesting const methods when the > > > object > > > is const. The code would probably need to support the new param > > > being > > > NULL_TREE for cases where we're looking up a static member. Or > > > maybe > > > an enum of access style for const vs non-const vs static. > > > Maybe name the param "access_hint" to signify that it's merely > > > there > > > for the purpose of hints for the user, and not to affect the > > > parsing > > > itself? > > > > Hm, that does seem rather unfortunate. > > > > > > Another solution would be to not bother offering non-const > > > methods as > > > accessors. > > > > > > I think that would be very unfortunate. > > > > How about adding a tsubst_flag value? > > > > tf_const_obj = 1 << 11, /* For alternative accessor suggestion > > help. */ > > > > and pass that in? the tsubst flags have grown in meaning somewhat > > since > > they first appeared -- their name is no longer so appropriate. > > > > (of course we have the same problem with volatile, but that's > > probably > > overkill for first attempt.) > > > > Jason, WDYT? > > I'd suggest handling this diagnostic in > finish_class_member_access_expr, rather than try to push down context > information into lookup_member. Perhaps by adding another parameter > to lookup_member for passing back the inaccessible or ambiguous > lookup > result? > > Jason Thanks. Here's an updated version of the patch which adds an optional struct ptr, for writing back the info, which then gets emitted (if set) in finish_class_member_access_expr (and thus has access to the constness of the object). Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. OK for trunk? gcc/cp/ChangeLog: * call.c (enforce_access): Add access_failure_info * param and use it to record access failures. * cp-tree.h (class access_failure_info): New class. (enforce_access): Add access_failure_info * param, defaulting to NULL. (lookup_member): Likewise. (locate_field_accessor): New function decl. (perform_or_defer_access_check): Add access_failure_info * param, defaulting to NULL. * search.c (lookup_member): Add access_failure_info * param and pass it on to call to perform_or_defer_access_check. (matches_code_and_type_p): New function. (field_access_p): New function. (direct_accessor_p): New function. (reference_accessor_p): New function. (field_accessor_p): New function. (struct locate_field_data): New struct. (dfs_locate_field_accessor_pre): New function. (locate_field_accessor): New function. * semantics.c (perform_or_defer_access_check): Add access_failure_info * param, and pass it on to call to enforce_access. * typeck.c (access_failure_info::record_access_failure): New method. (access_failure_info::maybe_suggest_accessor): New method. (finish_class_member_access_expr): Pass an access_failure_info instance to the lookup_member call, and call its maybe_suggest_accessor method afterwards. gcc/testsuite/ChangeLog: * g++.dg/other/accessor-fixits-1.C: New test case. * g++.dg/other/accessor-fixits-2.C: New test case. * g++.dg/other/accessor-fixits-3.C: New test case. * g++.dg/other/accessor-fixits-4.C: New test case. --- gcc/cp/call.c | 8 +- gcc/cp/cp-tree.h | 31 +++- gcc/cp/search.c | 240 ++++++++++++++++++++++++- gcc/cp/semantics.c | 8 +- gcc/cp/typeck.c | 45 ++++- gcc/testsuite/g++.dg/other/accessor-fixits-1.C | 178 ++++++++++++++++++ gcc/testsuite/g++.dg/other/accessor-fixits-2.C | 104 +++++++++++ gcc/testsuite/g++.dg/other/accessor-fixits-3.C | 15 ++ gcc/testsuite/g++.dg/other/accessor-fixits-4.C | 48 +++++ 9 files changed, 666 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-1.C create mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-2.C create mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-3.C create mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-4.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index c15b8e4..9ed4ad0 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -6415,7 +6415,7 @@ build_op_delete_call (enum tree_code code, tree addr, tree size, bool enforce_access (tree basetype_path, tree decl, tree diag_decl, - tsubst_flags_t complain) + tsubst_flags_t complain, access_failure_info *afi) { gcc_assert (TREE_CODE (basetype_path) == TREE_BINFO); @@ -6441,17 +6441,23 @@ enforce_access (tree basetype_path, tree decl, tree diag_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); } } return false; diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 8721ed4..c2b9df8 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5659,8 +5659,30 @@ extern bool can_convert_arg (tree, tree, tree, int, tsubst_flags_t); extern bool can_convert_arg_bad (tree, tree, tree, int, tsubst_flags_t); + +/* A class for recording information about access failures (e.g. private + fields), so that we can potentially supply a fix-it hint about + an accessor (from a context in which the constness of the object + is known). */ + +class access_failure_info +{ + public: + access_failure_info () : m_was_inaccessible (false), m_basetype_path (NULL_TREE), + m_field_decl (NULL_TREE) {} + + void record_access_failure (tree basetype_path, tree field_decl); + void maybe_suggest_accessor (bool const_p) const; + + private: + bool m_was_inaccessible; + tree m_basetype_path; + tree m_field_decl; +}; + extern bool enforce_access (tree, tree, tree, - tsubst_flags_t); + tsubst_flags_t, + access_failure_info *afi = NULL); extern void push_defarg_context (tree); extern void pop_defarg_context (void); extern tree convert_default_arg (tree, tree, tree, int, @@ -6322,8 +6344,10 @@ extern tree lookup_fnfields_slot_nolazy (tree, tree); extern int class_method_index_for_fn (tree, tree); extern tree lookup_fnfields (tree, tree, int); extern tree lookup_member (tree, tree, int, bool, - tsubst_flags_t); + tsubst_flags_t, + access_failure_info *afi = NULL); extern tree lookup_member_fuzzy (tree, tree, bool); +extern tree locate_field_accessor (tree, tree, bool); extern int look_for_overrides (tree, tree); extern void get_pure_virtuals (tree); extern void maybe_suppress_debug_info (tree); @@ -6378,7 +6402,8 @@ extern bool perform_access_checks (vec *, tsubst_flags_t); extern bool perform_deferred_access_checks (tsubst_flags_t); extern bool perform_or_defer_access_check (tree, tree, tree, - tsubst_flags_t); + tsubst_flags_t, + access_failure_info *afi = NULL); /* RAII sentinel to ensures that deferred access checks are popped before a function returns. */ diff --git a/gcc/cp/search.c b/gcc/cp/search.c index 09c1b4e..2cd6ea5 100644 --- a/gcc/cp/search.c +++ b/gcc/cp/search.c @@ -1232,11 +1232,13 @@ build_baselink (tree binfo, tree access_binfo, tree functions, tree optype) WANT_TYPE is 1 when we should only return TYPE_DECLs. - If nothing can be found return NULL_TREE and do not issue an error. */ + If nothing can be found return NULL_TREE and do not issue an error. + + If non-NULL, failure information is written back to AFI. */ tree lookup_member (tree xbasetype, tree name, int protect, bool want_type, - tsubst_flags_t complain) + tsubst_flags_t complain, access_failure_info *afi) { tree rval, rval_binfo = NULL_TREE; tree type = NULL_TREE, basetype_path = NULL_TREE; @@ -1337,7 +1339,7 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type, tree decl = is_overloaded_fn (rval) ? get_first_fn (rval) : rval; if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl) && !perform_or_defer_access_check (basetype_path, decl, decl, - complain)) + complain, afi)) rval = error_mark_node; } @@ -1993,6 +1995,238 @@ dfs_walk_once_accessible (tree binfo, bool friends_p, return rval; } +/* Return true iff the code of T is CODE, and it has compatible + type with TYPE. */ + +static bool +matches_code_and_type_p (tree t, enum tree_code code, tree type) +{ + if (TREE_CODE (t) != code) + return false; + if (!cxx_types_compatible_p (TREE_TYPE (t), type)) + return false; + return true; +} + +/* Subroutine of direct_accessor_p and reference_accessor_p. + Determine if COMPONENT_REF is a simple field lookup of this->FIELD_DECL. + We expect a tree of the form: + + + >>. */ + +static bool +field_access_p (tree component_ref, tree field_decl, tree field_type) +{ + if (!matches_code_and_type_p (component_ref, COMPONENT_REF, field_type)) + return false; + + tree indirect_ref = TREE_OPERAND (component_ref, 0); + if (TREE_CODE (indirect_ref) != INDIRECT_REF) + return false; + + tree ptr = STRIP_NOPS (TREE_OPERAND (indirect_ref, 0)); + if (!is_this_parameter (ptr)) + return false; + + /* Must access the correct field. */ + if (TREE_OPERAND (component_ref, 1) != field_decl) + return false; + return true; +} + +/* Subroutine of field_accessor_p. + + Assuming that INIT_EXPR has already had its code and type checked, + determine if it is a simple accessor for FIELD_DECL + (of type FIELD_TYPE). + + Specifically, a simple accessor within struct S of the form: + T get_field () { return m_field; } + should have a DECL_SAVED_TREE of the form: + + + >>. */ + +static bool +direct_accessor_p (tree init_expr, tree field_decl, tree field_type) +{ + tree result_decl = TREE_OPERAND (init_expr, 0); + if (!matches_code_and_type_p (result_decl, RESULT_DECL, field_type)) + return false; + + tree component_ref = STRIP_NOPS (TREE_OPERAND (init_expr, 1)); + if (!field_access_p (component_ref, field_decl, field_type)) + return false; + + return true; +} + +/* Subroutine of field_accessor_p. + + Assuming that INIT_EXPR has already had its code and type checked, + determine if it is a "reference" accessor for FIELD_DECL + (of type FIELD_REFERENCE_TYPE). + + Specifically, a simple accessor within struct S of the form: + T& get_field () { return m_field; } + should have a DECL_SAVED_TREE of the form: + > + >>>>>. */ +static bool +reference_accessor_p (tree init_expr, tree field_decl, tree field_type, + tree field_reference_type) +{ + tree result_decl = TREE_OPERAND (init_expr, 0); + if (!matches_code_and_type_p (result_decl, RESULT_DECL, field_reference_type)) + return false; + + tree field_pointer_type = build_pointer_type (field_type); + tree addr_expr = STRIP_NOPS (TREE_OPERAND (init_expr, 1)); + if (!matches_code_and_type_p (addr_expr, ADDR_EXPR, field_pointer_type)) + return false; + + tree component_ref = STRIP_NOPS (TREE_OPERAND (addr_expr, 0)); + + if (!field_access_p (component_ref, field_decl, field_type)) + return false; + + return true; +} + +/* Return true if FN is an accessor method for FIELD_DECL. + i.e. a method of the form { return FIELD; }, with no + conversions. + + If CONST_P, then additionally require that FN be a const + method. */ + +static bool +field_accessor_p (tree fn, tree field_decl, bool const_p) +{ + if (TREE_CODE (fn) != FUNCTION_DECL) + return false; + + /* We don't yet support looking up static data, just fields. */ + if (TREE_CODE (field_decl) != FIELD_DECL) + return false; + + tree fntype = TREE_TYPE (fn); + if (TREE_CODE (fntype) != METHOD_TYPE) + return false; + + /* If the field is accessed via a const "this" argument, verify + that the "this" parameter is const. */ + if (const_p) + { + tree this_type = type_of_this_parm (fntype); + if (!TYPE_READONLY (this_type)) + return false; + } + + tree saved_tree = DECL_SAVED_TREE (fn); + + if (saved_tree == NULL_TREE) + return false; + + if (TREE_CODE (saved_tree) != RETURN_EXPR) + return false; + + tree init_expr = TREE_OPERAND (saved_tree, 0); + if (TREE_CODE (init_expr) != INIT_EXPR) + return false; + + /* Determine if this is a simple accessor within struct S of the form: + T get_field () { return m_field; }. */ + tree field_type = TREE_TYPE (field_decl); + if (cxx_types_compatible_p (TREE_TYPE (init_expr), field_type)) + return direct_accessor_p (init_expr, field_decl, field_type); + + /* Failing that, determine if it is an accessor of the form: + T& get_field () { return m_field; }. */ + tree field_reference_type = cp_build_reference_type (field_type, false); + if (cxx_types_compatible_p (TREE_TYPE (init_expr), field_reference_type)) + return reference_accessor_p (init_expr, field_decl, field_type, + field_reference_type); + + return false; +} + +/* Callback data for dfs_locate_field_accessor_pre. */ + +struct locate_field_data +{ + locate_field_data (tree field_decl_, bool const_p_) + : field_decl (field_decl_), const_p (const_p_) {} + + tree field_decl; + bool const_p; +}; + +/* Return a FUNCTION_DECL that is an "accessor" method for DATA, a FIELD_DECL, + callable via binfo, if one exists, otherwise return NULL_TREE. + + Callback for dfs_walk_once_accessible for use within + locate_field_accessor. */ + +static tree +dfs_locate_field_accessor_pre (tree binfo, void *data) +{ + locate_field_data *lfd = (locate_field_data *)data; + tree type = BINFO_TYPE (binfo); + + vec *method_vec; + tree fn; + size_t i; + + if (!CLASS_TYPE_P (type)) + return NULL_TREE; + + method_vec = CLASSTYPE_METHOD_VEC (type); + if (!method_vec) + return NULL_TREE; + + for (i = 0; vec_safe_iterate (method_vec, i, &fn); ++i) + if (fn) + if (field_accessor_p (fn, lfd->field_decl, lfd->const_p)) + return fn; + + return NULL_TREE; +} + +/* Return a FUNCTION_DECL that is an "accessor" method for FIELD_DECL, + callable via BASETYPE_PATH, if one exists, otherwise return NULL_TREE. */ + +tree +locate_field_accessor (tree basetype_path, tree field_decl, bool const_p) +{ + if (TREE_CODE (basetype_path) != TREE_BINFO) + return NULL_TREE; + + /* Walk the hierarchy, looking for a method of some base class that allows + access to the field. */ + locate_field_data lfd (field_decl, const_p); + return dfs_walk_once_accessible (basetype_path, /*friends=*/true, + dfs_locate_field_accessor_pre, + NULL, &lfd); +} + /* Check that virtual overrider OVERRIDER is acceptable for base function BASEFN. Issue diagnostic, and return zero, if unacceptable. */ diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 4db2462..e6c6bd9 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -305,11 +305,13 @@ perform_deferred_access_checks (tsubst_flags_t complain) /* Defer checking the accessibility of DECL, when looked up in BINFO. DIAG_DECL is the declaration to use to print diagnostics. - Return value like perform_access_checks above. */ + Return value like perform_access_checks above. + If non-NULL, report failures to AFI. */ bool perform_or_defer_access_check (tree binfo, tree decl, tree diag_decl, - tsubst_flags_t complain) + tsubst_flags_t complain, + access_failure_info *afi) { int i; deferred_access *ptr; @@ -328,7 +330,7 @@ perform_or_defer_access_check (tree binfo, tree decl, tree diag_decl, /* If we are not supposed to defer access checks, just check now. */ if (ptr->deferring_access_checks_kind == dk_no_deferred) { - bool ok = enforce_access (binfo, decl, diag_decl, complain); + bool ok = enforce_access (binfo, decl, diag_decl, complain, afi); return (complain & tf_error) ? true : ok; } diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 7aee0d6..66fb7ba 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -2650,6 +2650,46 @@ check_template_keyword (tree decl) } } +/* Record that an access failure occurred on BASETYPE_PATH attempting + to access FIELD_DECL. */ + +void +access_failure_info::record_access_failure (tree basetype_path, + tree field_decl) +{ + m_was_inaccessible = true; + m_basetype_path = basetype_path; + m_field_decl = field_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. */ + +void +access_failure_info::maybe_suggest_accessor (bool const_p) const +{ + if (!m_was_inaccessible) + return; + + tree accessor + = locate_field_accessor (m_basetype_path, m_field_decl, const_p); + if (!accessor) + return; + + /* The accessor must itself be accessible for it to be a reasonable + suggestion. */ + if (!accessible_p (m_basetype_path, accessor, true)) + return; + + rich_location richloc (line_table, input_location); + pretty_printer pp; + pp_printf (&pp, "%s()", IDENTIFIER_POINTER (DECL_NAME (accessor))); + richloc.add_fixit_replace (pp_formatted_text (&pp)); + inform_at_rich_loc (&richloc, "field %q#D can be accessed via %q#D", + m_field_decl, accessor); +} + /* This function is called by the parser to process a class member access expression of the form OBJECT.NAME. NAME is a node used by the parser to represent a name; it is not yet a DECL. It may, @@ -2834,8 +2874,11 @@ finish_class_member_access_expr (cp_expr object, tree name, bool template_p, else { /* Look up the member. */ + access_failure_info afi; member = lookup_member (access_path, name, /*protect=*/1, - /*want_type=*/false, complain); + /*want_type=*/false, complain, + &afi); + afi.maybe_suggest_accessor (TYPE_READONLY (object_type)); if (member == NULL_TREE) { if (dependent_type_p (object_type)) diff --git a/gcc/testsuite/g++.dg/other/accessor-fixits-1.C b/gcc/testsuite/g++.dg/other/accessor-fixits-1.C new file mode 100644 index 0000000..cc96b87 --- /dev/null +++ b/gcc/testsuite/g++.dg/other/accessor-fixits-1.C @@ -0,0 +1,178 @@ +// { dg-options "-fdiagnostics-show-caret" } + +class t1 +{ +public: + int get_color () const { return m_color; } + int get_shape () const { return m_shape; } + +private: + int m_color; + +protected: + int m_shape; +}; + +int test_access_t1_color (t1 &ref) +{ + return ref.m_color; // { dg-error ".int t1::m_color. is private within this context" } + /* { dg-begin-multiline-output "" } + return ref.m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "declared private here" "" { target *-*-* } 10 } + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_color. can be accessed via .int t1::get_color\\(\\) const." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ref.m_color; + ^~~~~~~ + get_color() + { dg-end-multiline-output "" } */ +} + +int test_access_t1_shape (t1 &ref) +{ + return ref.m_shape; // { dg-error ".int t1::m_shape. is protected within this context" } + /* { dg-begin-multiline-output "" } + return ref.m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "declared protected here" "" { target *-*-* } 13 } + /* { dg-begin-multiline-output "" } + int m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_shape. can be accessed via .int t1::get_shape\\(\\) const." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ref.m_shape; + ^~~~~~~ + get_shape() + { dg-end-multiline-output "" } */ +} + +int test_deref_t1_color (t1 *ptr) +{ + return ptr->m_color; // { dg-error ".int t1::m_color. is private within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_color. can be accessed via .int t1::get_color\\(\\) const." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + get_color() + { dg-end-multiline-output "" } */ +} + +int test_deref_t1_shape (t1 *ptr) +{ + return ptr->m_shape; // { dg-error ".int t1::m_shape. is protected within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + + /* { dg-begin-multiline-output "" } + int m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_shape. can be accessed via .int t1::get_shape\\(\\) const." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ptr->m_shape; + ^~~~~~~ + get_shape() + { dg-end-multiline-output "" } */ +} + +/* Example of public inheritance. */ + +class t2 : public t1 +{ +}; + +int test_deref_t2_color (t2 *ptr) +{ + return ptr->m_color; // { dg-error ".int t1::m_color. is private within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_color. can be accessed via .int t1::get_color\\(\\) const." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + get_color() + { dg-end-multiline-output "" } */ +} + +/* Example of private inheritance. */ + +class t3 : private t1 +{ +}; + +int test_deref_t3_color (t3 *ptr) +{ + return ptr->m_color; // { dg-error ".int t1::m_color. is private within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + /* We shouldn't provide a fix-it hint for this case due to the + private inheritance. */ +} + +/* Example of non-public "accessor". */ + +class t4 +{ + int m_field; + int get_field () { return m_field; } +}; + +int test_deref_t4_field (t4 *ptr) +{ + return ptr->m_field; // { dg-error ".int t4::m_field. is private within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_field; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + /* { dg-begin-multiline-output "" } + int m_field; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + /* We shouldn't provide a fix-it hint for this case, as the accessor is + itself private. */ +} diff --git a/gcc/testsuite/g++.dg/other/accessor-fixits-2.C b/gcc/testsuite/g++.dg/other/accessor-fixits-2.C new file mode 100644 index 0000000..e1a2b78 --- /dev/null +++ b/gcc/testsuite/g++.dg/other/accessor-fixits-2.C @@ -0,0 +1,104 @@ +// { dg-options "-fdiagnostics-show-caret" } + +/* Test of accessors that return references. */ + +class t1 +{ +public: + int& get_color () { return m_color; } + int& get_shape () { return m_shape; } + +private: + int m_color; + +protected: + int m_shape; +}; + +int test_access_t1_color (t1 &ref) +{ + return ref.m_color; // { dg-error ".int t1::m_color. is private within this context" } + /* { dg-begin-multiline-output "" } + return ref.m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "declared private here" "" { target *-*-* } 12 } + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_color. can be accessed via .int& t1::get_color\\(\\)." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ref.m_color; + ^~~~~~~ + get_color() + { dg-end-multiline-output "" } */ +} + +int test_access_t1_shape (t1 &ref) +{ + return ref.m_shape; // { dg-error ".int t1::m_shape. is protected within this context" } + /* { dg-begin-multiline-output "" } + return ref.m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "declared protected here" "" { target *-*-* } 15 } + /* { dg-begin-multiline-output "" } + int m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_shape. can be accessed via .int& t1::get_shape\\(\\)." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ref.m_shape; + ^~~~~~~ + get_shape() + { dg-end-multiline-output "" } */ +} + +int test_deref_t1_color (t1 *ptr) +{ + return ptr->m_color; // { dg-error ".int t1::m_color. is private within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_color. can be accessed via .int& t1::get_color\\(\\)." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + get_color() + { dg-end-multiline-output "" } */ +} + +int test_deref_t1_shape (t1 *ptr) +{ + return ptr->m_shape; // { dg-error ".int t1::m_shape. is protected within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + + /* { dg-begin-multiline-output "" } + int m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "field .int t1::m_shape. can be accessed via .int& t1::get_shape\\(\\)." "" { target *-*-* } .-12 } + /* { dg-begin-multiline-output "" } + return ptr->m_shape; + ^~~~~~~ + get_shape() + { dg-end-multiline-output "" } */ +} diff --git a/gcc/testsuite/g++.dg/other/accessor-fixits-3.C b/gcc/testsuite/g++.dg/other/accessor-fixits-3.C new file mode 100644 index 0000000..27d2eb4 --- /dev/null +++ b/gcc/testsuite/g++.dg/other/accessor-fixits-3.C @@ -0,0 +1,15 @@ +class foo +{ +public: + static foo& get_singleton () { return s_singleton; } + +private: + static foo s_singleton; +}; + +foo & test_access_singleton () +{ + return foo::s_singleton; // { dg-error ".foo foo::s_singleton. is private within this context" } + // { dg-message "declared private here" "" { target *-*-* } 7 } + // We don't yet support generating a fix-it hint for this case. +} diff --git a/gcc/testsuite/g++.dg/other/accessor-fixits-4.C b/gcc/testsuite/g++.dg/other/accessor-fixits-4.C new file mode 100644 index 0000000..c03dd4e --- /dev/null +++ b/gcc/testsuite/g++.dg/other/accessor-fixits-4.C @@ -0,0 +1,48 @@ +// { dg-options "-fdiagnostics-show-caret" } + +class t1 +{ +public: + int& get_color () { return m_color; } + int& get_shape () { return m_shape; } + +private: + int m_color; // { dg-line color_decl } + int m_shape; // { dg-line shape_decl } +}; + +int test_const_ptr (const t1 *ptr) +{ + return ptr->m_color; // { dg-error ".int t1::m_color. is private within this context" } + /* { dg-begin-multiline-output "" } + return ptr->m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "declared private here" "" { target *-*-* } color_decl } + /* { dg-begin-multiline-output "" } + int m_color; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + /* We shouldn't issue a suggestion: the accessor is non-const, and we + only have a const ptr. */ +} + +int test_const_reference (const t1 &ref) +{ + return ref.m_shape; // { dg-error ".int t1::m_shape. is private within this context" } + /* { dg-begin-multiline-output "" } + return ref.m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + // { dg-message "declared private here" "" { target *-*-* } shape_decl } + /* { dg-begin-multiline-output "" } + int m_shape; + ^~~~~~~ + { dg-end-multiline-output "" } */ + + /* We shouldn't issue a suggestion: the accessor is non-const, and we + only have a const ptr. */ +} -- 1.8.5.3