[Bug c++/16166] -Weffc++ finer granularity
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #14 from Eric Gallager --- This came up on the gcc-help mailing list here: https://gcc.gnu.org/ml/gcc-help/2018-11/msg3.html
[Bug c++/16166] -Weffc++ finer granularity
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 Eric Gallager changed: What|Removed |Added CC||egallager at gcc dot gnu.org --- Comment #13 from Eric Gallager --- (In reply to Jonathan Wakely from comment #12) > Created attachment 43443 [details] > Patch to split -Weffc++ into separate options. > > This adds several new warning options to replace -Weffc++, and makes > -Weffc++ enable them all. Right I've been meaning to try this patch but couldn't, due to having been blocked from bootstrapping for a while by bug 82092... it looks like that one might be getting fixed around now though, so once I've verified that, I'll try this...
[Bug c++/16166] -Weffc++ finer granularity
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #12 from Jonathan Wakely --- Created attachment 43443 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43443=edit Patch to split -Weffc++ into separate options. This adds several new warning options to replace -Weffc++, and makes -Weffc++ enable them all.
[Bug c++/16166] -Weffc++ finer granularity
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 Eric Gallager changed: What|Removed |Added CC||tromey at gcc dot gnu.org --- Comment #11 from Eric Gallager --- *** Bug 81431 has been marked as a duplicate of this bug. ***
[Bug c++/16166] -Weffc++ finer granularity
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #10 from Jonathan Wakely redi at gcc dot gnu.org --- *** Bug 63871 has been marked as a duplicate of this bug. ***
[Bug c++/16166] -Weffc++ finer granularity
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #9 from Jonathan Wakely redi at gcc dot gnu.org --- (In reply to Jonathan Wakely from comment #3) # Item 11: Define a copy constructor and an assignment operator for classes with dynamically allocated memory. Replaced by Item 14: Think carefully about copying behavior in resource-managing classes - the advice is less specific, but more useful. I'm not sure how to turn it into a warning though! I think this could be replaced by a new warning for PR 58407
[Bug c++/16166] -Weffc++ finer granularity
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #5 from Jonathan Wakely redi at gcc dot gnu.org 2012-05-29 08:34:32 UTC --- (In reply to comment #4) * Item 11: Define a copy constructor and an assignment operator for classes with dynamically allocated memory. -Wcopy-resource-class IMHO this warning should just go. With deleted copy ctor/assign and move ctor/assign there are even more places where a hard and fast rule isn't useful. * Item 12: Prefer initialization to assignment in constructors. -Wassignment-in-constructor If I ever get my -Wmeminit patch working properly it would provide this. * Item 14: Make destructors virtual in base classes. Already covered by -Wnon-virtual-dtor And the more useful -Wdelete-non-virtual-dtor In summary, you could simulate exactly the behavior of -Weffc++ by turning on each of these warnings individually, or you could turn on -Weffc++ and selectively turn off a few warnings that you don't want. Yep, that would be much better
[Bug c++/16166] -Weffc++ finer granularity
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 Manuel López-Ibáñez manu at gcc dot gnu.org changed: What|Removed |Added CC||manu at gcc dot gnu.org --- Comment #6 from Manuel López-Ibáñez manu at gcc dot gnu.org 2012-05-29 09:58:08 UTC --- (In reply to comment #4) I would recommend against naming each warning -Weffc++[n], but rather, give a more descriptive name. My suggestion is to create a few warnings, so that -Weffc++ would map to the following set of warnings (with their current description for reference and my suggested name): David, if you wish to implement such a patch (or, even better, series of patches, one for each new option), the only changes needed are: * In the particular warning () calls, replace OPT_Weffc__ with the appropriate OPT_W option. * In c-family/c.opt, add a new entry for the new Wfoo option and use EnabledBy(Weffc++) * In doc/invoke.texi, document the new options. * Bootstrap+regression test and submit to gcc-patches. Profit!
[Bug c++/16166] -Weffc++ finer granularity
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #7 from David Stone david at doublewise dot net 2012-05-29 20:57:22 UTC --- (In reply to comment #5) (In reply to comment #4) * Item 11: Define a copy constructor and an assignment operator for classes with dynamically allocated memory. -Wcopy-resource-class IMHO this warning should just go. With deleted copy ctor/assign and move ctor/assign there are even more places where a hard and fast rule isn't useful. And in general, classes should just use a smart pointer from boost / the standard library, rather than managing their own memory directly. I agree that this warning probably isn't that useful, and I wouldn't be sad to see it go. * Item 12: Prefer initialization to assignment in constructors. -Wassignment-in-constructor If I ever get my -Wmeminit patch working properly it would provide this. Is the issue just finding the time to do it, or are there subtle issues involved? * Item 14: Make destructors virtual in base classes. Already covered by -Wnon-virtual-dtor And the more useful -Wdelete-non-virtual-dtor Yeah, my only thought for the usefulness of -Wnon-virtual-dtor over -Wdelete-non-virtual-dtor is that a library author may wish to have a class that users are supposed to derive from, but doesn't actually call delete anywhere in the library code. They may want to ensure that users of the library (who may not have any warnings turned on at all) do not run into any surprising bugs. (In reply to comment #6) David, if you wish to implement such a patch (or, even better, series of patches, one for each new option), the only changes needed are: I am currently writing up a patch to update doc/invoke.texi for warnings, to try and make it easier to see what is turned on by -Wall and -Wextra and what isn't. Do you think it would be best for me to submit a patch based on the current documentation, my updated documentation (as a separate patch, of course), or some combination of the two?
[Bug c++/16166] -Weffc++ finer granularity
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #8 from Jonathan Wakely redi at gcc dot gnu.org 2012-05-29 23:21:23 UTC --- I would keep the patches separate.
[Bug c++/16166] -Weffc++ finer granularity
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #4 from David Stone david at doublewise dot net 2012-05-29 02:13:53 UTC --- I would recommend against naming each warning -Weffc++[n], but rather, give a more descriptive name. My suggestion is to create a few warnings, so that -Weffc++ would map to the following set of warnings (with their current description for reference and my suggested name): Effective C++: * Item 11: Define a copy constructor and an assignment operator for classes with dynamically allocated memory. -Wcopy-resource-class * Item 12: Prefer initialization to assignment in constructors. -Wassignment-in-constructor * Item 14: Make destructors virtual in base classes. Already covered by -Wnon-virtual-dtor * Item 15: Have operator= return a reference to *this. -Wassignment-operator-return-value * Item 23: Don't try to return a reference when you must return an object. -Wsuspicious-reference-return More Effective C++: * Item 6: Distinguish between prefix and postfix forms of increment and decrement operators. -Wprefix-postfix * Item 7: Never overload , ||, or ,. Perhaps this should be split into two warnings of its own: -Woverloaded-short-circuit -Woverloaded-comma In summary, you could simulate exactly the behavior of -Weffc++ by turning on each of these warnings individually, or you could turn on -Weffc++ and selectively turn off a few warnings that you don't want.
[Bug c++/16166] -Weffc++ finer granularity
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166 --- Comment #3 from Jonathan Wakely redi at gcc dot gnu.org 2011-11-07 16:10:10 UTC --- Reviewing these warnings w.r.t the much improved third edition... (In reply to comment #1) # Item 11: Define a copy constructor and an assignment operator for classes with dynamically allocated memory. Replaced by Item 14: Think carefully about copying behavior in resource-managing classes - the advice is less specific, but more useful. I'm not sure how to turn it into a warning though! # Item 12: Prefer initialization to assignment in constructors. Replaced by Item 4: Make sure that objects are initialized before they're used, and G++ misinterprets the original item anyway and warns about *any* member without a mem-initializer, which is very annoying: there's no point initializing a std::string, it has a perfectly safe default constructor. My -Wmeminit patch for PR 2972 should replace the current warning for this item, as it only warns about members left uninitialized by the constructor. # Item 14: Make destructors virtual in base classes. Adjusted to Item 7: Declare destructors virtual in polymorphic base classes, the warning is still relevant (but -Wdelete-non-virtual-dtor is more useful IMHO) # Item 15: Have operator= return a reference to *this. Renumbered to Item 10. Still relevant. # Item 23: Don't try to return a reference when you must return an object. Renumbered to Item 21. Still relevant. # Item 6: Distinguish between prefix and postfix forms of increment and decrement operators. # Item 7: Never overload , ||, or ,. These are from More Effective C++ which only has one edition.
[Bug c++/16166] -Weffc++ finer granularity
-- What|Removed |Added Keywords||diagnostic Last reconfirmed|2004-10-30 17:49:38 |2005-01-29 03:19:30 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16166
