OpenE2K/gcc
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

C++: suggestions for misspelled private members (PR c++/84993)

Browse Source 
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()

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.

From-SVN: r265056
This commit is contained in:
David Malcolm 2018-10-11 19:03:33 +00:00 committed by David Malcolm
parent c7f45560c7
commit 03f6d32edb
6 changed files with 316 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
gcc/cp/ChangeLog
View File

@ -1,3 +1,29 @@
2018-10-11 David Malcolm <dmalcolm@redhat.com>
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.
2018-10-11 Nathan Sidwell <nathan@acm.org>
* parser.c (cp_parser_translation_unit): Return void. Don't fail

64
gcc/cp/call.c
View File

@ -6514,6 +6514,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
@ -6538,34 +6570,12 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl,
if (!accessible_p (basetype_path, decl, true))
{
if (flag_new_inheriting_ctors)
diag_decl = strip_inheriting_ctors (diag_decl);
if (complain & tf_error)
{
if (flag_new_inheriting_ctors)
diag_decl = strip_inheriting_ctors (diag_decl);
if (TREE_PRIVATE (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);
}
}
complain_about_access (decl, diag_decl, true);
if (afi)
afi->record_access_failure (basetype_path, decl, diag_decl);
return false;
}

17
gcc/cp/cp-tree.h
View File

@ -6103,18 +6103,27 @@ extern void complain_about_bad_argument (location_t arg_loc,
class access_failure_info
{
public:
access_failure_info () : m_was_inaccessible (false), m_basetype_path (NULL_TREE),
m_field_decl (NULL_TREE) {}
access_failure_info () : m_was_inaccessible (false),
m_basetype_path (NULL_TREE),
m_decl (NULL_TREE), m_diag_decl (NULL_TREE) {}
void record_access_failure (tree basetype_path, tree field_decl);
void record_access_failure (tree basetype_path, tree decl, tree diag_decl);
bool was_inaccessible_p () const { return m_was_inaccessible; }
tree get_decl () const { return m_decl; }
tree get_diag_decl () const { return m_diag_decl; }
tree get_any_accessor (bool const_p) const;
void maybe_suggest_accessor (bool const_p) const;
static void add_fixit_hint (rich_location *richloc, tree accessor);
private:
bool m_was_inaccessible;
tree m_basetype_path;
tree m_field_decl;
tree m_decl;
tree m_diag_decl;
};
extern void complain_about_access (tree, tree, bool);
extern bool enforce_access (tree, tree, tree,
tsubst_flags_t,
access_failure_info *afi = NULL);

156
gcc/cp/typeck.c
View File

@ -2708,15 +2708,51 @@ check_template_keyword (tree decl)
}
/* Record that an access failure occurred on BASETYPE_PATH attempting
to access FIELD_DECL. */
to access DECL, where DIAG_DECL should be used for diagnostics. */
void
access_failure_info::record_access_failure (tree basetype_path,
tree field_decl)
tree decl, tree diag_decl)
{
m_was_inaccessible = true;
m_basetype_path = basetype_path;
m_field_decl = field_decl;
m_decl = decl;
m_diag_decl = diag_decl;
}
/* If an access failure was recorded, then attempt to locate an
accessor function for the pertinent field.
Otherwise, return NULL_TREE. */
tree
access_failure_info::get_any_accessor (bool const_p) const
{
if (!was_inaccessible_p ())
return NULL_TREE;
tree accessor
= locate_field_accessor (m_basetype_path, m_diag_decl, const_p);
if (!accessor)
return NULL_TREE;
/* The accessor must itself be accessible for it to be a reasonable
suggestion. */
if (!accessible_p (m_basetype_path, accessor, true))
return NULL_TREE;
return accessor;
}
/* Add a fix-it hint to RICHLOC suggesting the use of ACCESSOR_DECL, by
replacing the primary location in RICHLOC with "accessor()". */
void
access_failure_info::add_fixit_hint (rich_location *richloc,
tree accessor_decl)
{
pretty_printer pp;
pp_printf (&pp, "%s()", IDENTIFIER_POINTER (DECL_NAME (accessor_decl)));
richloc->add_fixit_replace (pp_formatted_text (&pp));
}
/* If an access failure was recorded, then attempt to locate an
@ -2726,25 +2762,84 @@ access_failure_info::record_access_failure (tree basetype_path,
void
access_failure_info::maybe_suggest_accessor (bool const_p) const
{
if (!m_was_inaccessible)
tree accessor = get_any_accessor (const_p);
if (accessor == NULL_TREE)
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));
add_fixit_hint (&richloc, accessor);
inform (&richloc, "field %q#D can be accessed via %q#D",
m_field_decl, accessor);
m_diag_decl, accessor);
}
/* Subroutine of finish_class_member_access_expr.
Issue an error about NAME not being a member of ACCESS_PATH (or
OBJECT_TYPE), potentially providing a fix-it hint for misspelled
names. */
static void
complain_about_unrecognized_member (tree access_path, tree name,
tree object_type)
{
/* Attempt to provide a hint about misspelled names. */
tree guessed_id = lookup_member_fuzzy (access_path, name,
/*want_type=*/false);
if (guessed_id == NULL_TREE)
{
/* No hint. */
error ("%q#T has no member named %qE",
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type, name);
return;
}
location_t bogus_component_loc = input_location;
gcc_rich_location rich_loc (bogus_component_loc);
/* Check that the guessed name is accessible along access_path. */
access_failure_info afi;
lookup_member (access_path, guessed_id, /*protect=*/1,
/*want_type=*/false, /*complain=*/false,
&afi);
if (afi.was_inaccessible_p ())
{
tree accessor = afi.get_any_accessor (TYPE_READONLY (object_type));
if (accessor)
{
/* The guessed name isn't directly accessible, but can be accessed
via an accessor member function. */
afi.add_fixit_hint (&rich_loc, accessor);
error_at (&rich_loc,
"%q#T has no member named %qE;"
" did you mean %q#D? (accessible via %q#D)",
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type,
name, afi.get_diag_decl (), accessor);
}
else
{
/* The guessed name isn't directly accessible, and no accessor
member function could be found. */
error_at (&rich_loc,
"%q#T has no member named %qE;"
" did you mean %q#D? (not accessible from this context)",
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type,
name, afi.get_diag_decl ());
complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
}
}
else
{
/* The guessed name is directly accessible; suggest it. */
rich_loc.add_fixit_misspelled_id (bogus_component_loc,
guessed_id);
error_at (&rich_loc,
"%q#T has no member named %qE;"
" did you mean %qE?",
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type,
name, guessed_id);
}
}
/* This function is called by the parser to process a class member
@ -2940,27 +3035,8 @@ finish_class_member_access_expr (cp_expr object, tree name, bool template_p,
/* Try again at instantiation time. */
goto dependent;
if (complain & tf_error)
{
tree guessed_id = lookup_member_fuzzy (access_path, name,
/*want_type=*/false);
if (guessed_id)
{
location_t bogus_component_loc = input_location;
gcc_rich_location rich_loc (bogus_component_loc);
rich_loc.add_fixit_misspelled_id (bogus_component_loc,
guessed_id);
error_at (&rich_loc,
"%q#T has no member named %qE;"
" did you mean %qE?",
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type,
name, guessed_id);
}
else
error ("%q#T has no member named %qE",
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type, name);
}
complain_about_unrecognized_member (access_path, name,
object_type);
return error_mark_node;
}
if (member == error_mark_node)

5
gcc/testsuite/ChangeLog
View File

@ -1,3 +1,8 @@
2018-10-11 David Malcolm <dmalcolm@redhat.com>
PR c++/84993
* g++.dg/torture/accessor-fixits-9.C: New test.
2018-10-11 Nathan Sidwell <nathan@acm.org>
* g++.dg/parse/close-brace.C: New.

119
gcc/testsuite/g++.dg/torture/accessor-fixits-9.C Normal file
View File

@ -0,0 +1,119 @@
// PR c++/84993
// { dg-options "-fdiagnostics-show-caret" }
/* Misspelling (by omitting a leading "m_") of a private member for which
there's a public accessor.
We expect a fix-it hint suggesting the accessor. */
class t1
{
public:
int get_ratio () const { return m_ratio; }
private:
int m_ratio;
};
int test (t1 *ptr_1)
{
return ptr_1->ratio; // { dg-error "'class t1' has no member named 'ratio'; did you mean 'int t1::m_ratio'\\? \\(accessible via 'int t1::get_ratio\\(\\) const'\\)" }
/* { dg-begin-multiline-output "" }
return ptr_1->ratio;
^~~~~
get_ratio()
{ dg-end-multiline-output "" } */
}
/* Misspelling of a private member for which there's a public accessor.
We expect a fix-it hint suggesting the accessor. */
class t2
{
public:
int get_color () const { return m_color; }
private:
int m_color;
};
int test (t2 *ptr_2)
{
return ptr_2->m_colour; // { dg-error "'class t2' has no member named 'm_colour'; did you mean 'int t2::m_color'\\? \\(accessible via 'int t2::get_color\\(\\) const'\\)" }
/* { dg-begin-multiline-output "" }
return ptr_2->m_colour;
^~~~~~~~
get_color()
{ dg-end-multiline-output "" } */
}
/* Misspelling of a private member via a subclass pointer, for which there's
a public accessor in the base class.
We expect a fix-it hint suggesting the accessor. */
class t3 : public t2 {};
int test (t3 *ptr_3)
{
return ptr_3->m_colour; // { dg-error "'class t3' has no member named 'm_colour'; did you mean 'int t2::m_color'\\? \\(accessible via 'int t2::get_color\\(\\) const'\\)" }
/* { dg-begin-multiline-output "" }
return ptr_3->m_colour;
^~~~~~~~
get_color()
{ dg-end-multiline-output "" } */
}
/* Misspelling of a protected member, for which there's isn't a public
accessor.
We expect no fix-it hint; instead a message identifying where the
data member was declared. */
class t4
{
protected:
int m_color; // { dg-message "declared protected here" }
};
int test (t4 *ptr_4)
{
return ptr_4->m_colour; // { dg-error "'class t4' has no member named 'm_colour'; did you mean 'int t4::m_color'\\? \\(not accessible from this context\\)" }
/* { dg-begin-multiline-output "" }
return ptr_4->m_colour;
^~~~~~~~
{ dg-end-multiline-output "" } */
/* { dg-begin-multiline-output "" }
int m_color;
^~~~~~~
{ dg-end-multiline-output "" } */
}
/* Misspelling of a private member, for which the accessor is also private.
We expect no fix-it hint; instead a message identifying where the
data member was declared. */
class t5
{
int get_color () const { return m_color; }
int m_color; // { dg-message "declared private here" }
};
int test (t5 *ptr_5)
{
return ptr_5->m_colour; // { dg-error "'class t5' has no member named 'm_colour'; did you mean 'int t5::m_color'\\? \\(not accessible from this context\\)" }
/* { dg-begin-multiline-output "" }
return ptr_5->m_colour;
^~~~~~~~
{ dg-end-multiline-output "" } */
/* { dg-begin-multiline-output "" }
int m_color;
^~~~~~~
{ dg-end-multiline-output "" } */
}