C++: fix-it hints suggesting accessors for private fields

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.

From-SVN: r248128

6 files changed, 351 insertions, 11 deletions

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 228f0ad..d659fb9 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,33 @@
+2017-05-16  David Malcolm  <dmalcolm@redhat.com>
+
+	* 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.
+
 2017-05-16  Marek Polacek  <polacek@redhat.com>
 
 	PR sanitizer/80536
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 1367344..cc64202 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6396,7 +6396,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);
 
@@ -6422,17 +6422,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 739ff18..0efc5db 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5758,8 +5758,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,
@@ -6412,8 +6434,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);
@@ -6468,7 +6492,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 fa72c47..61bdb93 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 ce2899b..d3249fe 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 46a6bf8..8c94d20 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2647,6 +2647,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,
@@ -2829,8 +2869,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))