[PATCH] Warning for main returning a bool.

[Aaron Ballman via cfe-commits]

On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
>> From: "Aaron Ballman" <aaron at aaronballman.com>
>> To: "Hal Finkel" <hfinkel at anl.gov>
>> Cc: "cfe-commits" <cfe-commits at lists.llvm.org>, "Joshua Hurwitz" <hurwitzj at google.com>
>> Sent: Tuesday, November 15, 2016 4:42:05 PM
>> Subject: Re: [PATCH] Warning for main returning a bool.
>>
>> On Tue, Nov 15, 2016 at 5:22 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>> > ----- Original Message -----
>> >> From: "Aaron Ballman via cfe-commits" <cfe-commits at lists.llvm.org>
>> >> To: "Joshua Hurwitz" <hurwitzj at google.com>
>> >> Cc: "cfe-commits" <cfe-commits at lists.llvm.org>
>> >> Sent: Tuesday, November 15, 2016 12:17:28 PM
>> >> Subject: Re: [PATCH] Warning for main returning a bool.
>> >>
>> >> On Fri, Oct 14, 2016 at 1:17 PM, Joshua Hurwitz via cfe-commits
>> >> <cfe-commits at lists.llvm.org> wrote:
>> >> > See attached.
>> >> >
>> >> > Returning a bool from main is a special case of return type
>> >> > mismatch. The
>> >> > common convention when returning a bool is that 'true' (== 1)
>> >> > indicates
>> >> > success and 'false' (== 0) failure. But since main expects a
>> >> > return
>> >> > value of
>> >> > 0 on success, returning a bool is usually unintended.
>> >>
>> >> I am not convinced that this is a high-value diagnostic. Returning
>> >> a
>> >> Boolean from main() may or may not be a bug (the returned value is
>> >> generally a convention more than anything else). Also, why Boolean
>> >> and
>> >> not, say, long long or float?
>> >
>> > I've seen this error often enough, but I think we need to be
>> > careful about false positives here. I recommend that we check only
>> > for explicit uses of boolean immediates (i.e. return true; or
>> > return false;) -- these are often bugs.
>>
>> I could get behind that.
>>
>> > Aaron, I disagree that the return value is just some kind of
>> > convention. It has a clear meaning.
>>
>> For many hosted environments, certainly. Freestanding
>> implementations?
>> Much less so, but I suppose this behavior is still reasonable enough
>> for them (not to mention, there may not even *be* a main() for a
>> freestanding implementation).
>>
>> > Furthermore, the behavior of the system can be quite different for
>> > a non-zero exit code than otherwise, and users who don't
>> > understand what's going on can find it very difficult to
>> > understand what's going wrong.
>>
>> That's a fair point, but my question still stands -- why only Boolean
>> values, and not "return 0x1234567800000000ULL;" or "return 1.2;"?
>>
>> Combining with your idea above, if the check flagged instances where
>> a
>> literal of non-integral type (other than Boolean) is returned from
>> main(), that seems like good value.
>
> Agreed.
>
> FWIW, if we have a function that returns 'int' and the user tries to return '1.2' we should probably warn for any function.

Good point, we already have -Wliteral-conversion (which catches 1.2)
and -Wconstant-conversion (which catches 0x1234567800000000ULL), and
-Wint-conversion for C programs returning awful things like string
literals, all of which are enabled by default. Perhaps Boolean values
really are the only case we don't explicitly warn about.

~Aaron

>
>  -Hal
>
>>
>> ~Aaron
>>
>> >
>> > Thanks again,
>> > Hal
>> >
>> >>
>> >> ~Aaron
>> >>
>> >> >
>> >> > _______________________________________________
>> >> > cfe-commits mailing list
>> >> > cfe-commits at lists.llvm.org
>> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >> >
>> >> _______________________________________________
>> >> cfe-commits mailing list
>> >> cfe-commits at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >>
>> >
>> > --
>> > Hal Finkel
>> > Lead, Compiler Technology and Programming Languages
>> > Leadership Computing Facility
>> > Argonne National Laboratory
>>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory