[C++ PATCH] Improve -Weffc++ item #11

["Giovanni Bajo" <giovannibajo at libero dot it>]

Hello,

this patch improves -Weffc++ item 11: "define a copy-ctor and an operator= for
classes which allocate dynamic memory". This is currently implemented by
checking if the class has data members of pointer type. The improvement is
two-fold:

- We don't warn anymore if the class contains data members whose type is
pointer to function or pointer to member. These types can never hold dynamic
memory, so it does not make sense to warn.

- We don't warn anymore if the class does not have a non-trivial destructor. We
assume that if the class does not have a destructor, there is no dynamic memory
allocated (which would need some cleanup), so there is no need to define a
copy-ctor or an assignment operator (suggested by David Carlton here:
http://gcc.gnu.org/ml/gcc/2004-06/msg01389.html)

Tested on i686-pc-linux-gnu by building v3 with CXXFLAGS="-Weffc++", OK for
mainline?

Giovanni Bajo


cp/
        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.

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


Index: class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.619
diff -c -3 -p -r1.619 class.c
*** class.c 24 Jun 2004 06:48:41 -0000 1.619
--- class.c 8 Jul 2004 12:29:31 -0000
*************** check_field_decls (tree t, tree *access_
*** 2931,2943 ****
  {
    tree *field;
    tree *next;
!   int 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;
    /* Assume none of the members of this class have default
       initializations.  */
    any_default_members = 0;
--- 2931,2943 ----
  {
    tree *field;
    tree *next;
!   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 = false;
    /* Assume none of the members of this class have default
       initializations.  */
    any_default_members = 0;
*************** check_field_decls (tree t, tree *access_
*** 3078,3086 ****
   }

        type = strip_array_types (type);
!
!       if (TYPE_PTR_P (type))
!  has_pointers = 1;

        if (CLASS_TYPE_P (type))
   {
--- 3078,3091 ----
   }

        type = strip_array_types (type);
!
!       /* 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))
   {
*************** check_field_decls (tree t, tree *access_
*** 3146,3154 ****
       &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)))
      {
        warning ("`%#T' has pointer data members", t);

--- 3151,3175 ----
       &any_default_members);
      }

!   /* 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);




// { 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&);
};