This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C++ PATCH] Improve -Weffc++ item #11
Kai Henningsen wrote:
>> * class.c (check_field_decls): Improve -Weffc++ warning: do
>> not warn for pointers to functions/members, or for classes
>> without destructors.
>
> This does not match the actual patch:
>
>> ! -- 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);
>
> This also tests for constructors.
The ChangeLog lists only the changes introduced by my patch. The test for
non-trivial constructors was already there.
> That seems wrong: a class can easily hold dynamic memory
> that is only allocated on demand, which means it
> doesn't necessarily need a non-trivial constructor.
Sure, there are thousands of usage patterns for pointers within classes, and we
can't possibly catch all of them. If you go your way, even the destructor test
is probably useless, because there could be a member function to free the
memory, and some external cleanup machinery that makes sure that it is called
when needed.
We simply can't win here. -Weffc++ is used to catch simple mistakes in simple,
straightforward usage patterns. Usually, it's better to remove false positives
and miss more opportunities to warn, and this is what my patch does. Next in
the queue there is probably a patch to enable/disable single warnings in
the -Weffc++ suite, so that you can finetune it to your application.
--
Giovanni Bajo