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

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 5a1a0bc8ceac899a0ef22161a9a7e085ebc075ef..3d194a4366802cc8c6207b53438535f531ef63a6 100644 (file)
--- 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 1f3b22a7ddcb8bd213da0c02f64051cbe07dede6..f9c3b706592eca96fff8d3c66607b1d86f38b23b 100644 (file)
--- 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);
       

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 3b94ce020b95197a85c3fcceda2bd60ede3f18f0..bc11b674407223a3d20b8ec56492576345768445 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+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

diff --git a/gcc/testsuite/g++.dg/warn/effc3.C b/gcc/testsuite/g++.dg/warn/effc3.C
new file mode 100644 (file)
index 0000000..ba2cc03
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/effc3.C
@@ -0,0 +1,58 @@
+// { 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&);
+};