On Tue, 2017-04-25 at 07:49 -0400, Nathan Sidwell wrote:
> On 04/25/2017 07:46 AM, Nathan Sidwell wrote:
> > On 04/24/2017 04:06 PM, David Malcolm wrote:
> >
> > > test.cc:12:13: note: field ‘int foo::m_field’ can be accessed via
> > > ‘int
> > > foo::get_field() const’
> > > return f->m_field;
> > > ^~~~~~~
> > > get_field()
> > >
> > > Assuming that an IDE can offer to apply fix-it hints, this should
> > > make it easier to handle refactorings where one makes a field
> > > private and adds a getter.
> > >
> > > It also helps by letting the user know that a getter exists, and
> > > the name of the getter ("is it "field", "get_field", etc?").
> >
> > Neat!
> >
> > >
> > > OK for trunk?
> > >
> > > gcc/cp/ChangeLog:
> > > * call.c (maybe_suggest_accessor): New function.
> > > (enforce_access): Call maybe_suggest_accessor for
> > > inaccessible
> > > decls.
> > > * cp-tree.h (locate_field_accessor): New decl.
> > > * search.c (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.
> > > (dfs_locate_field_accessor_pre): New function.
> > > (locate_field_accessor): New function.
> >
> > ok.
>
> Oh, what if the field is being accessed for modification or
> lvalueness?
> Must the accessor return T, or can it return 'T cv &'? I.e. does it
> need to look for setters too?
There's an example of doing so in the patch in:
g++.dg/other/accessor-fixits-2.C
(see direct_accessor_p vs reference_accessor_p in the patch for how
this is handled).
The patch doesn't take account the context of how the returned field is
used, e.g. whether as an rvalue vs in a modification/lvalue way; it
just looks for a method of the form
{ return FIELD; }
for the correct field, favoring returning T to returning T&.
Is there a way to get at the "style" of access from
call.c:enforce_access? (to determine if T vs T& would be a better
suggestion)
Otherwise, is the patch OK as-is?
A case the patch doesn't provide a hint for yet is for static data; e.g. for:
return foo::s_singleton;
(as in g++.dg/other/accessor-fixits-3.C in the patch); would that be OK
to leave as a follow-up?
Thanks
Dave