On 09/21/2018 04:09 PM, David Malcolm wrote:
This is v2 of the patch; I managed to bit-rot my own patch due to my
fix for r264335, which tightened up the "is this meaningful" threshold
on edit distances when finding spelling correction candidates.
The only change in this version is to rename various things in
the testcase so that they continue to be suggested
("colour" vs "m_color" are no longer near enough, so I renamed
"colour" to "m_colour").
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
Blurb from v1:
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:
<source>: In function 'void test(foo*)':
<source>: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:
<source>: In function 'void test(foo*)':
<source>:12:12: error: 'double foo::m_ratio' is private within this context
if (ptr->m_ratio >= 0.5)
^~~~~~~
<source>:7:10: note: declared private here
double m_ratio;
^~~~~~~
<source>: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:
<source>: In function 'void test(foo*)':
<source>: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()
I wonder if suggesting the inaccessible member is at all helpful
if it cannot be used. Would mentioning only the accessor be
a better approach?
Also, to echo a comment made by someone else in a bug I don't
remember, rather than phrasing the error in the form of
a question ("did you mean x?") it might be better to either
state what we know:
error: 'class foo' has no member named 'ratio'; the nearest
match is 'double foo::m_ratio' (accessible via 'double
foo::get_ratio() const')
or offer a suggestion:
error: 'class foo' has no member named 'ratio'; suggest
using 'double foo::get_ratio() const' instead
A different approach altogether to these spelling messages that
occurs to me but that you may have already considered and rejected
would be to do what GCC does for errors due to ambiguous overloads:
i.e., enumerate the available candidates. This would work well in
cases when multiple members are a close match. It would also make
it possible to explain, for each candidate member, whether it's
accessible.
Martin
