Robert Haas <robertmhaas@gmail.com> writes:
> On Sat, Jul 14, 2012 at 6:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hm. I seem to recall that at least some of these lines were themselves
>> put in to suppress compiler warnings.
> You mean things like this?
> -
> - /* keep compiler happy */
> - return NULL;
That particular case appears to have been in the aboriginal commit of
the GIN code. It's possible that Teodor's compiler complained without
it, but it seems at least as likely that (a) he was just guessing that
some compilers would complain, or (b) it was leftover from an earlier
version of the function in which the loop wasn't so obviously
non-exitable.
> I admit that compiler warnings are horribly annoying and we probably
> ought to favor new compilers over old ones, at least in master, but
> I'm kinda skeptical about the contention that no compiler anywhere
> will complain if we remove hunks like that one. The comment seems
> pretty clear. I feel like we're missing something here.
I don't have a whole lot of sympathy for compilers that think a
"for (;;) { ... }" loop with no break statement might terminate.
If you're going to emit warnings on the basis of reachability
analysis, I think you should be doing reachability analysis that's
not completely brain-dead. Or else not be surprised when people
ignore your warnings.
Now, I'm prepared to believe that some of these functions might
contain cases that are not quite as black-and-white as that one.
That's why I suggested we might end up doing further code tweaking.
But at least for this example, I don't have a problem with applying
the patch and seeing what happens.
The bigger picture here is that we're threading a line between
getting warnings from compilers that are too smart, versus warnings
from compilers that are not smart enough (which could actually be
the same compiler with different settings :-(). We're not going
to be able to satisfy all cases --- but I think it's probably wise
to tilt towards satisfying the smarter compilers, when we have to
compromise.
regards, tom lane
