re PR c++/8211 (-Weffc++ warns about copyable classes with func ptr members)

	PR c++/8211
	PR c++/16165
	* class.c (check_field_decls): Improve -Weffc++ warning: do not
	warn for pointers to functions/members, or for classes without
	destructors.

	PR c++/8211
	PR c++/16165
	* g++.dg/warn/effc3.C: New test.

From-SVN: r84338

2 files changed, 37 insertions, 8 deletions

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 5a1a0bc..3d194a4 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2004-07-09  Giovanni Bajo  <giovannibajo@gcc.gnu.org>
+
+	PR c++/8211
+	PR c++/16165
+	* class.c (check_field_decls): Improve -Weffc++ warning: do not
+	warn for pointers to functions/members, or for classes without
+	destructors.
+
 2004-07-08  Mark Mitchell  <mark@codesourcery.com>
 
 	* name-lookup.h (struct cp_binding_level): Update documentation
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 1f3b22a..f9c3b70 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2925,13 +2925,13 @@ check_field_decls (tree t, tree *access_decls,
 {
   tree *field;
   tree *next;
-  int has_pointers;
+  bool has_pointers;
   int any_default_members;
 
   /* Assume there are no access declarations.  */
   *access_decls = NULL_TREE;
   /* Assume this class has no pointer members.  */
-  has_pointers = 0;
+  has_pointers = false;
   /* Assume none of the members of this class have default
      initializations.  */
   any_default_members = 0;
@@ -3072,9 +3072,14 @@ check_field_decls (tree t, tree *access_decls,
 	}
 
       type = strip_array_types (type);
-      
-      if (TYPE_PTR_P (type))
-	has_pointers = 1;
+
+      /* This is used by -Weffc++ (see below). Warn only for pointers
+	 to members which might hold dynamic memory. So do not warn
+	 for pointers to functions or pointers to members.  */
+      if (TYPE_PTR_P (type)
+	  && !TYPE_PTRFN_P (type)
+	  && !TYPE_PTR_TO_MEMBER_P (type))
+	has_pointers = true;
 
       if (CLASS_TYPE_P (type))
 	{
@@ -3140,9 +3145,25 @@ check_field_decls (tree t, tree *access_decls,
 			  &any_default_members);
     }
 
-  /* Effective C++ rule 11.  */
-  if (has_pointers && warn_ecpp && TYPE_HAS_CONSTRUCTOR (t)
-      && ! (TYPE_HAS_INIT_REF (t) && TYPE_HAS_ASSIGN_REF (t)))
+  /* Effective C++ rule 11: if a class has dynamic memory held by pointers,
+     it should also define a copy constructor and an assignment operator to
+     implement the correct copy semantic (deep vs shallow, etc.). As it is
+     not feasible to check whether the constructors do allocate dynamic memory
+     and store it within members, we approximate the warning like this:
+
+     -- Warn only if there are members which are pointers
+     -- Warn only if there is a non-trivial constructor (otherwise,
+	there cannot be memory allocated).
+     -- Warn only if there is a non-trivial destructor. We assume that the
+	user at least implemented the cleanup correctly, and a destructor
+	is needed to free dynamic memory.
+	
+     This seems enough for pratical purposes.  */
+    if (warn_ecpp
+	&& has_pointers
+	&& TYPE_HAS_CONSTRUCTOR (t)
+	&& TYPE_HAS_DESTRUCTOR (t)
+	&& !(TYPE_HAS_INIT_REF (t) && TYPE_HAS_ASSIGN_REF (t)))
     {
       warning ("`%#T' has pointer data members", t);