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: [PATCH] avoid uninit value warnings in dead code


On 11/04/2007, Dirk Mueller <dmueller@suse.de> wrote:
>
>  one of the most frequent complains about -Wuninitialized is that it warns
>  about uninitialized values in really obviously dead code. This can happen due
>  to conditional compiles and/or optional features being expanded in the code.
>
>  A more or less trivial patch seems to avoid this. The patch below was
>  bootstrapped with C, C++ and fortran and regression tested with no
>  regressions.
>
>  Comments? Ok?

I would like to comment on this patch in case someone finds it in the
archive and is considering committing it.

First, the patch is consistent with what we do now: We don't warn for
dead code. Check:

int foo (void)
{
  int j;
  if (1 == 0)
    return j;

  return 0;
}

In fact, execute_late_warn_uninitialized is ran after DCE. So we try
to not warn for unreachable things. From this point of view, the patch
is certainly an improvement. If we wanted to warn for dead code, we
would need some front-end warning separated from the current warning
(I called such option Wuninitialized=verbose in my draft proposal
[*]).

Second, I see a potential problem with the patch. Since CCP will
happily merge away uninitialized variables, effectively assuming a
value for the variable and propagating such value, it may happen that
the patch causes us to miss cases that we correctly warn now. However,
I wasn't able to come up with such a case. That probably just means
that I didn't put enough time/imagination.

Third, where is the limit? Clearly, there are cases where CCP won't be
enough and we would need VRP. Should we move early_warn_uninitialized
after VRP also? Perhaps the answer is yes. Or perhaps we shouldn't
warn about conditional blocks in execute_early_warn_uninitialized but
we should actually do it in execute_late_warn_uninitialized. Of
course, that causes other problems (e.g., CCP+DCE kill many correct
warnings)

I don't have a clear opinion about the patch but I am slightly
inclined to support rejection. Mainly because of the second point, we
may be just trading getting right a set of warnings for getting wrong
another. That is moving the problem elsewhere, not fixing it.

Cheers,

Manuel.

[*] http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings


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