From f1f14f1f6d55862ff4de38fb3d84df7749b2b2c0 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 23 Dec 2016 18:47:12 -0500
Subject: [PATCH 24/25] FIXME: WIP on fixits for suggesting accessors for
 private fields

---
 gcc/cp/call.c                               |  26 ++++++
 gcc/cp/search.c                             | 120 ++++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/test-accessor-fixits.C | 101 +++++++++++++++++++++++
 3 files changed, 247 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/test-accessor-fixits.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 86c7647..a1b649b 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6408,6 +6408,29 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
   return error_mark_node;
 }
 
+/* FIXME: make this a decl in a header.  */
+
+extern tree
+locate_field_accessor (tree basetype_path, tree field_decl);
+
+/* FIXME.  */
+
+static void
+maybe_suggest_accessor (tree basetype_path, tree field_decl)
+{
+  tree accessor = locate_field_accessor (basetype_path, field_decl);
+  if (!accessor)
+    return;
+
+  rich_location richloc (line_table, input_location);
+  // FIXME: do we want to have some kind of formatted printing for fixits?
+  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",
+                      field_decl, accessor);
+}
+
 /* 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
@@ -6441,17 +6464,20 @@ 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");
+	      maybe_suggest_accessor (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");
+	      maybe_suggest_accessor (basetype_path, diag_decl);
 	    }
 	  else
 	    {
 	      error ("%q#D is inaccessible within this context", diag_decl);
 	      inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
+	      maybe_suggest_accessor (basetype_path, diag_decl);
 	    }
 	}
       return false;
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index 09c1b4e..e114ac8 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -1993,6 +1993,126 @@ 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
+fixme_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;
+}
+
+/* Return true if FN is an accessor method for FIELD_DECL.
+   i.e. a method of the form { return FIELD; }, with no
+   conversions. */
+
+static bool
+field_accessor_p (tree fn, tree field_decl)
+{
+  if (TREE_CODE (fn) != FUNCTION_DECL)
+    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;
+
+  // FIXME: need lots of checks here!
+  // the method must be a simple "return FIELD" for the correct field...
+  // FIXME: but what about lvalues vs rvalues
+
+  tree field_type = TREE_TYPE (field_decl);
+
+  tree init_expr = TREE_OPERAND (saved_tree, 0);
+  if (!fixme_p (init_expr, INIT_EXPR, field_type))
+    return false;
+
+  tree result_decl = TREE_OPERAND (init_expr, 0);
+  if (!fixme_p (result_decl, RESULT_DECL, field_type))
+    return false;
+
+  tree component_ref = STRIP_NOPS (TREE_OPERAND (init_expr, 1));
+  if (!fixme_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;
+  // FIXME: verify that it's "this"
+
+  // FIXME: verify all the types
+
+  /* Must access the correct field.  */
+  if (TREE_OPERAND (component_ref, 1) != field_decl)
+    return false;
+
+  /* We expect an:
+       <init_expr:T
+         <result_decl:T
+	   <nop_expr:T
+	     <component_ref:
+               <indirect_ref:S>
+	       <nop_expr:P*
+	         <parm_decl (this)>
+	       <field_decl (FIELD_DECL)>>>. */
+
+  return true;
+}
+
+/* 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
+my_dfs_accessible_pre (tree binfo, void *data)
+{
+  tree field_decl = (tree)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, field_decl))
+	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)
+{
+  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.  */
+  return dfs_walk_once_accessible (basetype_path, /*friends=*/true,
+				   my_dfs_accessible_pre,
+				   NULL, field_decl);
+}
+
 /* Check that virtual overrider OVERRIDER is acceptable for base function
    BASEFN. Issue diagnostic, and return zero, if unacceptable.  */
 
diff --git a/gcc/testsuite/g++.dg/test-accessor-fixits.C b/gcc/testsuite/g++.dg/test-accessor-fixits.C
new file mode 100644
index 0000000..c05c98b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/test-accessor-fixits.C
@@ -0,0 +1,101 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+class foo
+{
+public:
+  int get_color () const { return m_color; }
+  int get_shape () const { return m_shape; }
+
+private:
+  int m_color;
+  int m_shape;
+};
+
+int test_access_color (foo &f)
+{
+  return f.m_color; // { dg-error ".int foo::m_color. is private within this context" }
+  /* { dg-begin-multiline-output "" }
+   return f.m_color;
+            ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "declared private here" "" { target *-*-* } 10 }
+  /* { dg-begin-multiline-output "" }
+   int m_color;
+       ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "field .int foo::m_color. can be accessed via .int foo::get_color\\(\\) const." "" { target *-*-* } .-12 }
+  /* { dg-begin-multiline-output "" }
+   return f.m_color;
+            ^~~~~~~
+            get_color()
+     { dg-end-multiline-output "" } */
+}
+
+
+int test_deref_color (foo *f)
+{
+  return f->m_color; // { dg-error ".int foo::m_color. is private within this context" }
+  /* { dg-begin-multiline-output "" }
+   return f->m_color;
+             ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "declared private here" "" { target *-*-* } 10 }
+  /* { dg-begin-multiline-output "" }
+   int m_color;
+       ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "field .int foo::m_color. can be accessed via .int foo::get_color\\(\\) const." "" { target *-*-* } .-12 }
+  /* { dg-begin-multiline-output "" }
+   return f->m_color;
+             ^~~~~~~
+             get_color()
+     { dg-end-multiline-output "" } */
+}
+
+int test_access_shape (foo &f)
+{
+  return f.m_shape; // { dg-error ".int foo::m_shape. is private within this context" }
+  /* { dg-begin-multiline-output "" }
+   return f.m_shape;
+            ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "declared private here" "" { target *-*-* } 11 }
+  /* { dg-begin-multiline-output "" }
+   int m_shape;
+       ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "field .int foo::m_shape. can be accessed via .int foo::get_shape\\(\\) const." "" { target *-*-* } .-12 }
+  /* { dg-begin-multiline-output "" }
+   return f.m_shape;
+            ^~~~~~~
+            get_shape()
+     { dg-end-multiline-output "" } */
+}
+
+int test_deref_shape (foo *f)
+{
+  return f->m_shape; // { dg-error ".int foo::m_shape. is private within this context" }
+  /* { dg-begin-multiline-output "" }
+   return f->m_shape;
+             ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "declared private here" "" { target *-*-* } 11 }
+  /* { dg-begin-multiline-output "" }
+   int m_shape;
+       ^~~~~~~
+     { dg-end-multiline-output "" } */
+
+  // { dg-message "field .int foo::m_shape. can be accessed via .int foo::get_shape\\(\\) const." "" { target *-*-* } .-12 }
+  /* { dg-begin-multiline-output "" }
+   return f->m_shape;
+             ^~~~~~~
+             get_shape()
+     { dg-end-multiline-output "" } */
+}
-- 
1.8.5.3

