Re: [PATCH] Warning for main returning a bool.

[Richard Smith via cfe-commits]

Committed as r288097.

On 28 November 2016 at 05:27, Joshua Hurwitz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Thanks Richard for looking at the revised patch.
>
> On Mon, Nov 21, 2016 at 1:50 PM Richard Smith <rich...@metafoo.co.uk>
> wrote:
>
>> This looks good to me. (While we could generalize this further, this
>> patch is a strict improvement, and we'll probably want to treat the 'main'
>> case specially regardless of whether we add a more general conversion
>> warning.)
>>
>> On 21 November 2016 at 07:18, Joshua Hurwitz <hurwi...@google.com> wrote:
>>
>> I modified the patch to warn only when a bool literal is returned from
>> main. See attached. -Josh
>>
>>
>> On Tue, Nov 15, 2016 at 7:50 PM Richard Smith <rich...@metafoo.co.uk>
>> wrote:
>>
>> On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfin...@anl.gov> wrote:
>> > - Original Message -
>> >> From: "Aaron Ballman" <aa...@aaronballman.com>
>> >> To: "Hal Finkel" <hfin...@anl.gov>
>> >> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" <
>> hurwi...@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 <hfin...@anl.gov> wrote:
>> >> > - Original Message -
>> >> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
>> >> >> To: "Joshua Hurwitz" <hurwi...@google.com>
>> >> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" or "return 1.2;"?
>> >>
>> >> Combining with your idea above, if the check flagged instances where
>> >> a
>> >&

Re: [PATCH] Warning for main returning a bool.

[Joshua Hurwitz via cfe-commits]

Thanks Richard for looking at the revised patch.

On Mon, Nov 21, 2016 at 1:50 PM Richard Smith <rich...@metafoo.co.uk> wrote:

> This looks good to me. (While we could generalize this further, this patch
> is a strict improvement, and we'll probably want to treat the 'main' case
> specially regardless of whether we add a more general conversion warning.)
>
> On 21 November 2016 at 07:18, Joshua Hurwitz <hurwi...@google.com> wrote:
>
> I modified the patch to warn only when a bool literal is returned from
> main. See attached. -Josh
>
>
> On Tue, Nov 15, 2016 at 7:50 PM Richard Smith <rich...@metafoo.co.uk>
> wrote:
>
> On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfin...@anl.gov> wrote:
> > - Original Message -
> >> From: "Aaron Ballman" <aa...@aaronballman.com>
> >> To: "Hal Finkel" <hfin...@anl.gov>
> >> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" <
> hurwi...@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 <hfin...@anl.gov> wrote:
> >> > - Original Message -
> >> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
> >> >> To: "Joshua Hurwitz" <hurwi...@google.com>
> >> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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 0x12345678ULL), and
> -Wint-

Re: [PATCH] Warning for main returning a bool.

[Richard Smith via cfe-commits]

This looks good to me. (While we could generalize this further, this patch
is a strict improvement, and we'll probably want to treat the 'main' case
specially regardless of whether we add a more general conversion warning.)

On 21 November 2016 at 07:18, Joshua Hurwitz <hurwi...@google.com> wrote:

> I modified the patch to warn only when a bool literal is returned from
> main. See attached. -Josh
>
>
> On Tue, Nov 15, 2016 at 7:50 PM Richard Smith <rich...@metafoo.co.uk>
> wrote:
>
>> On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfin...@anl.gov> wrote:
>> > - Original Message -
>> >> From: "Aaron Ballman" <aa...@aaronballman.com>
>> >> To: "Hal Finkel" <hfin...@anl.gov>
>> >> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" <
>> hurwi...@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 <hfin...@anl.gov> wrote:
>> >> > - Original Message -
>> >> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
>> >> >> To: "Joshua Hurwitz" <hurwi...@google.com>
>> >> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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 -Wlite

Re: [PATCH] Warning for main returning a bool.

[David Blaikie via cfe-commits]

On Tue, Nov 15, 2016 at 4:50 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfin...@anl.gov> wrote:
> > - Original Message -
> >> From: "Aaron Ballman" <aa...@aaronballman.com>
> >> To: "Hal Finkel" <hfin...@anl.gov>
> >> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" <
> hurwi...@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 <hfin...@anl.gov> wrote:
> >> > - Original Message -
> >> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
> >> >> To: "Joshua Hurwitz" <hurwi...@google.com>
> >> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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 0x12345678ULL), 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.
>
>
> Wow, I'm amazed that we have no warning at all for converting, say, 'true'
> to int (or indeed to float).
>

Yeah, it's somewhat conspicuously missing/explicitly avoided in the
literal-conversion warning code. I think I tried to prototype fixing/adding
this many years ago - forget where/how far I got.

It'd be great to have in the general form for things like this: void
f(bo

Re: [PATCH] Warning for main returning a bool.

[Joshua Hurwitz via cfe-commits]

I modified the patch to warn only when a bool literal is returned from
main. See attached. -Josh

On Tue, Nov 15, 2016 at 7:50 PM Richard Smith <rich...@metafoo.co.uk> wrote:

> On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfin...@anl.gov> wrote:
> > - Original Message -
> >> From: "Aaron Ballman" <aa...@aaronballman.com>
> >> To: "Hal Finkel" <hfin...@anl.gov>
> >> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" <
> hurwi...@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 <hfin...@anl.gov> wrote:
> >> > - Original Message -
> >> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
> >> >> To: "Joshua Hurwitz" <hurwi...@google.com>
> >> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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 0x12345678ULL), 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.
>
>
> Wow, I'm amazed that we have no warning at all for converting, say, 'true'
> to int (or indeed to float).
>
> Even with a warning for bool literal -> non-bool conversions in place, it
> would still seem valuable to factor out a check for the corresponding case
> in a return stat

Re: [PATCH] Warning for main returning a bool.

[Richard Smith via cfe-commits]

On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfin...@anl.gov> wrote:
> > - Original Message -
> >> From: "Aaron Ballman" <aa...@aaronballman.com>
> >> To: "Hal Finkel" <hfin...@anl.gov>
> >> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" <
> hurwi...@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 <hfin...@anl.gov> wrote:
> >> > - Original Message -
> >> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
> >> >> To: "Joshua Hurwitz" <hurwi...@google.com>
> >> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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 0x12345678ULL), 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.
>

Wow, I'm amazed that we have no warning at all for converting, say, 'true'
to int (or indeed to float).

Even with a warning for bool literal -> non-bool conversions in place, it
would still seem valuable to factor out a check for the corresponding case
in a return statement in main, since in that case we actually know what the
return value means, and the chance of a false positive goes way down.

So I think that means we want two or possibly three checks here:

1) main returns bool literal
2) -W

Re: [PATCH] Warning for main returning a bool.

[Aaron Ballman via cfe-commits]

On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <hfin...@anl.gov> wrote:
> - Original Message -
>> From: "Aaron Ballman" <aa...@aaronballman.com>
>> To: "Hal Finkel" <hfin...@anl.gov>
>> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" 
>> <hurwi...@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 <hfin...@anl.gov> wrote:
>> > - Original Message -
>> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
>> >> To: "Joshua Hurwitz" <hurwi...@google.com>
>> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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 0x12345678ULL), 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@lists.llvm.org
>> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >> >
>> >> ___
>> >> cfe-commits mailing list
>> >> cfe-commits@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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Hal Finkel via cfe-commits]

- Original Message -
> From: "Aaron Ballman" <aa...@aaronballman.com>
> To: "Hal Finkel" <hfin...@anl.gov>
> Cc: "cfe-commits" <cfe-commits@lists.llvm.org>, "Joshua Hurwitz" 
> <hurwi...@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 <hfin...@anl.gov> wrote:
> > - Original Message -
> >> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
> >> To: "Joshua Hurwitz" <hurwi...@google.com>
> >> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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.

 -Hal

> 
> ~Aaron
> 
> >
> > Thanks again,
> > Hal
> >
> >>
> >> ~Aaron
> >>
> >> >
> >> > ___
> >> > cfe-commits mailing list
> >> > cfe-commits@lists.llvm.org
> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >> >
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Aaron Ballman via cfe-commits]

On Tue, Nov 15, 2016 at 5:22 PM, Hal Finkel <hfin...@anl.gov> wrote:
> - Original Message -
>> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
>> To: "Joshua Hurwitz" <hurwi...@google.com>
>> Cc: "cfe-commits" <cfe-commits@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@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 0x12345678ULL;" 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.

~Aaron

>
> Thanks again,
> Hal
>
>>
>> ~Aaron
>>
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>> ___
>> cfe-commits mailing list
>> cfe-commits@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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Hal Finkel via cfe-commits]

- Original Message -
> From: "Aaron Ballman via cfe-commits" <cfe-commits@lists.llvm.org>
> To: "Joshua Hurwitz" <hurwi...@google.com>
> Cc: "cfe-commits" <cfe-commits@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@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.

Aaron, I disagree that the return value is just some kind of convention. It has 
a clear meaning. 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.

Thanks again,
Hal

> 
> ~Aaron
> 
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> ___
> cfe-commits mailing list
> cfe-commits@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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Aaron Ballman via cfe-commits]

On Fri, Oct 14, 2016 at 1:17 PM, Joshua Hurwitz via cfe-commits
 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?

~Aaron

>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Joshua Hurwitz via cfe-commits]

Friendly ping. Any further thoughts on this suggested warning?

On Sat, Nov 5, 2016 at 1:48 PM Manuel Klimek  wrote:

> +richard
>
> On Fri, Oct 14, 2016 at 10:18 AM Joshua Hurwitz via cfe-commits <
> cfe-commits@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.
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Manuel Klimek via cfe-commits]

On Sun, Nov 6, 2016 at 1:18 AM Michał Górny via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Fri, 14 Oct 2016 17:17:59 +
> Joshua Hurwitz via cfe-commits  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.
>
> This triggers a false positive if you use a boolean expression like:
>
>   return !foo;
>
> i.e. whenever user intentionally inverts a 'non-zero success' into 'zero
> success'.
>

I would imagine that not everybody's going to switch on that warning, but I
wouldn't consider this a false positive on !foo, mainly because it still
looks unclear whether the author thought about the semantics of the return
value.


>
> --
> Best regards,
> Michał Górny
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Michał Górny via cfe-commits]

On Fri, 14 Oct 2016 17:17:59 +
Joshua Hurwitz via cfe-commits  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.

This triggers a false positive if you use a boolean expression like:

  return !foo;

i.e. whenever user intentionally inverts a 'non-zero success' into 'zero
success'.

-- 
Best regards,
Michał Górny



pgpCBaeN4Nmc1.pgp
Description: OpenPGP digital signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Warning for main returning a bool.

[Manuel Klimek via cfe-commits]

+richard

On Fri, Oct 14, 2016 at 10:18 AM Joshua Hurwitz via cfe-commits <
cfe-commits@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.
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits