Re: [PATCH 1/3] Revert "make error()'s constant return value more visible"

From: Jeff King <peff@peff.net>
Date: May 5, 2014, 21:29 UTC
Message-ID: <20140505212938.GA16715@sigill.intra.peff.net>
In-Reply-To: <53673e0499ebe_78fd70f2ecb5@nysa.notmuch>

On Mon, May 05, 2014 at 02:30:12AM -0500, Felipe Contreras wrote:

> If we have a) code that fixes a couple warnings with -O3 but introduces
> hundreds with -O2, vs. b) code that has only a comple warnings with -O3,
> I'd go for b) any day.

I agree. But my point was to ask whether we can fix both.

> Yes, I see the problem now with -O3. And yes, I looked at the patch you
> sent. I haven't tested it but I bet it would sove both problems.

Unfortunately, it does not work in all cases (and I obviously did something wrong when testing it last night). I should have taken my own advice and re-read the commit message for e208f9c more carefully, which says:

    If we can make the compiler aware that error() will always
    return -1, it can do a better job of analysis. The simplest
    method would be to inline the error() function. However,
    this doesn't work, because gcc will not inline a variadc
    function. We can work around this by defining a macro.[...]

I cannot think of any other way to make the compiler aware of the constant value, but perhaps somebody else is more clever than I am. Another alternative is to write out "return error(...)" as "error(...); return -1". In some ways that is more readable, though it is more verbose (and would cause quite a bit of code churn).

So applying your patches may be the least-bad solution.

-Peff