This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Wuninitializable-member (PR7651 Define -Wextra strictly in terms of other warning flags)


"Manuel López-Ibáñez" <lopezibanez@gmail.com> writes:

| On 29 Dec 2006 20:46:11 +0100, Gabriel Dos Reis
| <gdr@integrable-solutions.net> wrote:
| > "Manuel López-Ibáñez" <lopezibanez@gmail.com> writes:
| >
| > | :ADDPATCH c++:
| > |
| > | This patch continues the effort to fix PR7651 [1].
| > |
| > | A new option -Wuninitializable-member [*] takes over the warning for a
| > | non-static reference or non-static const member appearing in a class
| > | without constructor. The new option is enabled by -Wextra, so we keep
| > | the current behaviour but add the ability to enable/disable this
| > | individual warning.
| >
| > Why -Wuninitializable-member?
| >
| > I would think that -Wuninitialized should take over this.
| >
| 
| My primary reason was to not break current behaviour: adding this
| warning to -Wuninitialized will move the warning from Wextra to Wall
| (since Wuninitialized is enabled by Wall).

Yes, but the fact that warning is not enabled by -Wall can be
considered a bug -- there is no conceivable reason why it really
should be outside -Wall, given the generality of -Wall.

I understand you were preserving the current behaviour.  What I'm
saying is that, for this specific warning, the current behaviour is
not actually desirable; it is a good thing your spotted it.

| Now, there are other reasons, which may or may not be relevant:
| 
| 1) -Wuninitialized only works when using optimisation, while this
| warning does not have such requirement;

yes, there is a sentiment (usually expression by Mark, I think) that
we should emit diagnostics reliably.

However, there are some subsets of "uninitialized" warnings that could
be emitted at -O0, and this is one of them.

| 2) the description of Wuninitialized (Warn if an automatic variable is
| used without first being initialized or if a variable may be clobbered
| by a setjmp call) does not clearly match the purpose of this C++
| warning;

yes, but the documentation is expandable.  

| 3) it is well-known that Wuninitialized could be improved [*] and that
| many users specifically disable Wuninitialized because it emits too
| many false positives. However, no false positive seems to be possible
| for this warning, and as you said, it is a warning that many users
| that disable Wuninitialized may like to use.
| 
| Another alternative could be to rename this warning as
| Wmissing-constructor and it could also include another Wextra warning:
| "A base class is not initialized in a derived class' copy
| constructor".

I'm not sure I would go there.

I think we need to be careful in distinguishing members that need
initialization (the one your original patch is trying to address),
from philosophical views such as -Weffc++. 

I still think that you should make this warning go under the
umbrella of -Wuninitialized (and update the documentation).

-- Gaby


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]