OK.
On Fri, May 5, 2017 at 6:50 PM, David Malcolm <dmalc...@redhat.com> wrote:
> On Mon, 2017-05-01 at 14:43 -0400, Jason Merrill wrote:
>> On Thu, Apr 27, 2017 at 7:23 AM, Nathan Sidwell <nat...@acm.org>
>> 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<deferred_access_check, va_gc> *,
> 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:
> + <component_ref:
> + <indirect_ref:S>
> + <nop_expr:P*
> + <parm_decl (this)>
> + <field_decl (FIELD_DECL)>>>. */
> +
> +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:
> + <return_expr
> + <init_expr:T
> + <result_decl:T
> + <nop_expr:T
> + <component_ref:
> + <indirect_ref:S>
> + <nop_expr:P*
> + <parm_decl (this)>
> + <field_decl (FIELD_DECL)>>>. */
> +
> +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:
> + <return_expr
> + <init_expr:T&
> + <result_decl:T&
> + <nop_expr: T&
> + <addr_expr: T*
> + <component_ref:T
> + <indirect_ref:S
> + <nop_expr
> + <parm_decl (this)>>
> + <field (FIELD_DECL)>>>>>>. */
> +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<tree, va_gc> *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
>
