[PING] Re: [PATCH] C++: suggestions for misspelled private members (PR c++/84993)
Ping:
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00594.html
On Wed, 2018-07-11 at 22:42 -0400, David Malcolm wrote:
> 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");
[PATCH] C++: suggestions for misspelled private members (PR c++/84993)
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_n
