From dd29d26b24928bdbcbdb1e7cbe09284f76f886f7 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Fri, 9 Jul 2004 00:59:05 +0000 Subject: [PATCH] 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 | 8 +++++ gcc/cp/class.c | 37 +++++++++++++++----- gcc/testsuite/ChangeLog | 6 ++++ gcc/testsuite/g++.dg/warn/effc3.C | 58 +++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/effc3.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 5a1a0bc8cea..3d194a43668 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,11 @@ +2004-07-09 Giovanni Bajo + + 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 * name-lookup.h (struct cp_binding_level): Update documentation diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 1f3b22a7ddc..f9c3b706592 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); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 3b94ce020b9..bc11b674407 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2004-07-09 Giovanni Bajo + + 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 index 00000000000..ba2cc039986 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/effc3.C @@ -0,0 +1,58 @@ +// { dg-do compile } +// { dg-options "-Weffc++" } +// Contributed by Benjamin Kosnik +// 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&); +}; -- 2.30.2