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

gcc/cp/ChangeLog

+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

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);
       

gcc/testsuite/ChangeLog

+2004-07-09  Giovanni Bajo  <giovannibajo@gcc.gnu.org>
+
+	PR c++/8211
+	PR c++/16165
+	* g++.dg/warn/effc3.C: New test.
+
 2004-07-09  David Billinghurst (David.Billinghurst@riotinto.com)
 
 	* gfortran.dg/g77/f77-edit-i-in.f: Copy from g77.dg and

gcc/testsuite/g++.dg/warn/effc3.C

+// { dg-do compile }
+// { dg-options "-Weffc++" }
+// Contributed by Benjamin Kosnik <bkoz at redhat dot com>
+// PR c++/16165 and PR c++/8211: Improve item 11 of -Weffc++
+
+
+// We should not warn for this class since this kind of pointers can
+//  never hold dynamic memory.
+struct A {
+  void (*func1)(void);
+  void (A::*func2)(void);
+  int A::*func3;
+
+  int a;
+  void b(void);
+
+  A();
+  ~A();
+};
+
+// We do not warn for this class because there is no destructor, so we
+//  assume there is no dynamic memory allocated (it could point to a
+//  global variable).
+struct B {
+  int *ptr;
+  B();
+};
+
+
+// We should emit a warning for these
+struct C1 {		// { dg-warning "" "" }
+  int *ptr;
+  C1();
+  ~C1();
+};
+
+struct C2 {		// { dg-warning "" "" }
+  int *ptr;
+  C2();
+  C2(const C2&);
+  ~C2();
+};
+
+struct C3 {		// { dg-warning "" "" }
+  int *ptr;
+  C3();
+  ~C3();
+  C3& operator=(const C3&);
+};
+
+// But not for this
+struct C4 {
+  int *ptr;
+  C4();
+  C4(const C4&);
+  ~C4();
+  C4& operator=(const C4&);
+};