Move -Wmaybe-uninitialized to -Wextra

[Marc Glisse]

On Mon, 4 Feb 2019, Martin Sebor wrote:

>>> In practice, false positives (and negatives) of both kinds, whether
>>> they fit the formal definition or the informal one, are the nature
>>> of virtually all non-trivial static diagnostics, certainly all those
>>> that depend on control or data flow analysis.  Some are due to bugs
>>> or limitations in the implementation of the warning.  Others are
>>> inherent in the technology.
>> 
>> Yes, and I argue that these warnings belong in a different "level" of 
>> warnings than the trivial warnings.
>
> Introducing more levels sounds fine to me.  I wouldn't want to see
> a more permissive default; my preference would be the opposite,
> leaving it to projects to adjust.

And I'd rather suggest as default something that can be combined with 
-Werror (not that you should do it, just that you could), leaving the 
non-trivial warnings that require motivation to investigate properly to 
people who have at least the motivation to enable extra flags. We can only 
agree to disagree...

>>> Lastly, in the case of uninitialized variables, the usual solution
>>> of initializing them is trivial and always safe (some coding styles
>>> even require it).
>> 
>> Here it shows that we don't work with the same type of code at all. If I am 
>> using a boost::optional, i.e. a class with a buffer and a boolean that says 
>> if the buffer is initialized, how do I initialize the (private) buffer? Or 
>> should boost itself zero out the buffer whenever the boolean is set to 
>> false? The variables can easily be hidden behind dozens of levels of 
>> abstraction that make them hard to initialize, and there may be nothing 
>> meaningful to initialize them with. Uninitialized also includes clobbered 
>> (out-of-scope for instance) in gcc, where it isn't clear what you are 
>> supposed to initialize to quiet the warning.
>
> You're right that this is hard to imagine without first hand experience
> with the problem.  If this is a common pattern with the warning in C++
> class templates in general, a representative test case would help get
> a better appreciation of the problem and might also give us an idea
> of a better solution.  (If there is one in Bugzilla please point me
> at it.)

Looking for "optional" and "-Wmaybe-uninitialized" shows
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Google also gives
https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
etc

And that's just for using a type called 'optional' (3 implementations of 
it).

> It's common to view as a false positive (or "bad" as you write below)
> every instance of a warning designed to detect or prevent bugs that doesn't 
> indicate one (as opposed to warnings designed to help write
> better/clearer code like -Wparentheses).

Well, warnings can't be completely arbitrary, they have to serve a 
purpose. Preventing some control flows where the use is not dominated by 
the initialization clearly enough could be one, but in my experience, on 
average, this does not lead to better programs for any metric besides the 
number of warnings. Again, experience can vary.

> Ideally each instance of every warning designed to find bugs would
> point out one.  But 100% accuracy or a zero rate of the undesirable
> instances is not attainable in general, and reducing their rate at
> the expense of the helpful ones also isn't the best tradeoff either.
> The challenge is striking the right balance between their ratios.

Yes.

> (Only if that can't done then it might be time to consider
> disabling/demoting the warning.  I don't have the impression
> we are at that point with -Wmaybe-uninitialized but I haven't
> done any research.)

It seems to depend on the type of code base, unsurprisingly.

> So since mindless quick fixes aren't the way to go and assuming we
> agree that warnings have value even with some noise, is demoting
> them to lower levels because they're not always used properly
> the best solution?  No predetermined system of warning levels is
> going to make everyone happy.  Different projects have different
> constraints and tolerances for noise, or even resources to fix
> real bugs -- GCC with over 400 wrong-code bugs in Open status
> being a case in point, so a level that works for one, like
> a library, may be overly pedantic for an application.

True. The main point is that there is a feeling that "gcc recommends using 
-Wall and fixing **all** those warnings **easily**" that does not exist 
(to the same level) with -Wextra. I guess that's not such a strong 
argument (you can probably feel that I've already given up).


Coincidentally, yesterday, I had to debug a crash in one of my programs. 
No warning, whatever I tried. At first, -fsanitize=address didn't give 
anything either. Then I found out about 
ASAN_OPTIONS=detect_stack_use_after_return=1, which pointed out super 
precisely which local variable I was accessing from where, which was then 
trivial to understand. So there are cases where I wish we were more 
verbose by default. Unless I misunderstand, this is disabled because it 
slows things down a bit. However the slowdown does not seem that big, 
people who care could always tweak it and are unlikely to use the defaults 
anyway. But I have almost no experience with it yet.

-- 
Marc Glisse