<div dir="ltr">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.)</div><div class="gmail_extra"><br><div class="gmail_quote">On 21 November 2016 at 07:18, Joshua Hurwitz <span dir="ltr"><<a href="mailto:hurwitzj@google.com" target="_blank">hurwitzj@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I modified the patch to warn only when a bool literal is returned from main. See attached. -Josh<div><div class="h5"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Nov 15, 2016 at 7:50 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="m_-6961844705446483138gmail_msg"><div class="gmail_extra m_-6961844705446483138gmail_msg"><div class="gmail_quote m_-6961844705446483138gmail_msg">On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <span dir="ltr" class="m_-6961844705446483138gmail_msg"><<a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br class="m_-6961844705446483138gmail_msg"><blockquote class="gmail_quote m_-6961844705446483138gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_-6961844705446483138m_2672333832697950803HOEnZb m_-6961844705446483138gmail_msg"><div class="m_-6961844705446483138m_2672333832697950803h5 m_-6961844705446483138gmail_msg">On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="m_-6961844705446483138gmail_msg" target="_blank">hfinkel@anl.gov</a>> wrote:<br class="m_-6961844705446483138gmail_msg">
> ----- Original Message -----<br class="m_-6961844705446483138gmail_msg">
>> From: "Aaron Ballman" <<a href="mailto:aaron@aaronballman.com" class="m_-6961844705446483138gmail_msg" target="_blank">aaron@aaronballman.com</a>><br class="m_-6961844705446483138gmail_msg">
>> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" class="m_-6961844705446483138gmail_msg" target="_blank">hfinkel@anl.gov</a>><br class="m_-6961844705446483138gmail_msg">
>> Cc: "cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a>>, "Joshua Hurwitz" <<a href="mailto:hurwitzj@google.com" class="m_-6961844705446483138gmail_msg" target="_blank">hurwitzj@google.com</a>><br class="m_-6961844705446483138gmail_msg">
>> Sent: Tuesday, November 15, 2016 4:42:05 PM<br class="m_-6961844705446483138gmail_msg">
>> Subject: Re: [PATCH] Warning for main returning a bool.<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> On Tue, Nov 15, 2016 at 5:22 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="m_-6961844705446483138gmail_msg" target="_blank">hfinkel@anl.gov</a>> wrote:<br class="m_-6961844705446483138gmail_msg">
>> > ----- Original Message -----<br class="m_-6961844705446483138gmail_msg">
>> >> From: "Aaron Ballman via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a>><br class="m_-6961844705446483138gmail_msg">
>> >> To: "Joshua Hurwitz" <<a href="mailto:hurwitzj@google.com" class="m_-6961844705446483138gmail_msg" target="_blank">hurwitzj@google.com</a>><br class="m_-6961844705446483138gmail_msg">
>> >> Cc: "cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a>><br class="m_-6961844705446483138gmail_msg">
>> >> Sent: Tuesday, November 15, 2016 12:17:28 PM<br class="m_-6961844705446483138gmail_msg">
>> >> Subject: Re: [PATCH] Warning for main returning a bool.<br class="m_-6961844705446483138gmail_msg">
>> >><br class="m_-6961844705446483138gmail_msg">
>> >> On Fri, Oct 14, 2016 at 1:17 PM, Joshua Hurwitz via cfe-commits<br class="m_-6961844705446483138gmail_msg">
>> >> <<a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br class="m_-6961844705446483138gmail_msg">
>> >> > See attached.<br class="m_-6961844705446483138gmail_msg">
>> >> ><br class="m_-6961844705446483138gmail_msg">
>> >> > Returning a bool from main is a special case of return type<br class="m_-6961844705446483138gmail_msg">
>> >> > mismatch. The<br class="m_-6961844705446483138gmail_msg">
>> >> > common convention when returning a bool is that 'true' (== 1)<br class="m_-6961844705446483138gmail_msg">
>> >> > indicates<br class="m_-6961844705446483138gmail_msg">
>> >> > success and 'false' (== 0) failure. But since main expects a<br class="m_-6961844705446483138gmail_msg">
>> >> > return<br class="m_-6961844705446483138gmail_msg">
>> >> > value of<br class="m_-6961844705446483138gmail_msg">
>> >> > 0 on success, returning a bool is usually unintended.<br class="m_-6961844705446483138gmail_msg">
>> >><br class="m_-6961844705446483138gmail_msg">
>> >> I am not convinced that this is a high-value diagnostic. Returning<br class="m_-6961844705446483138gmail_msg">
>> >> a<br class="m_-6961844705446483138gmail_msg">
>> >> Boolean from main() may or may not be a bug (the returned value is<br class="m_-6961844705446483138gmail_msg">
>> >> generally a convention more than anything else). Also, why Boolean<br class="m_-6961844705446483138gmail_msg">
>> >> and<br class="m_-6961844705446483138gmail_msg">
>> >> not, say, long long or float?<br class="m_-6961844705446483138gmail_msg">
>> ><br class="m_-6961844705446483138gmail_msg">
>> > I've seen this error often enough, but I think we need to be<br class="m_-6961844705446483138gmail_msg">
>> > careful about false positives here. I recommend that we check only<br class="m_-6961844705446483138gmail_msg">
>> > for explicit uses of boolean immediates (i.e. return true; or<br class="m_-6961844705446483138gmail_msg">
>> > return false;) -- these are often bugs.<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> I could get behind that.<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> > Aaron, I disagree that the return value is just some kind of<br class="m_-6961844705446483138gmail_msg">
>> > convention. It has a clear meaning.<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> For many hosted environments, certainly. Freestanding<br class="m_-6961844705446483138gmail_msg">
>> implementations?<br class="m_-6961844705446483138gmail_msg">
>> Much less so, but I suppose this behavior is still reasonable enough<br class="m_-6961844705446483138gmail_msg">
>> for them (not to mention, there may not even *be* a main() for a<br class="m_-6961844705446483138gmail_msg">
>> freestanding implementation).<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> > Furthermore, the behavior of the system can be quite different for<br class="m_-6961844705446483138gmail_msg">
>> > a non-zero exit code than otherwise, and users who don't<br class="m_-6961844705446483138gmail_msg">
>> > understand what's going on can find it very difficult to<br class="m_-6961844705446483138gmail_msg">
>> > understand what's going wrong.<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> That's a fair point, but my question still stands -- why only Boolean<br class="m_-6961844705446483138gmail_msg">
>> values, and not "return 0x1234567800000000ULL;" or "return 1.2;"?<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> Combining with your idea above, if the check flagged instances where<br class="m_-6961844705446483138gmail_msg">
>> a<br class="m_-6961844705446483138gmail_msg">
>> literal of non-integral type (other than Boolean) is returned from<br class="m_-6961844705446483138gmail_msg">
>> main(), that seems like good value.<br class="m_-6961844705446483138gmail_msg">
><br class="m_-6961844705446483138gmail_msg">
> Agreed.<br class="m_-6961844705446483138gmail_msg">
><br class="m_-6961844705446483138gmail_msg">
> FWIW, if we have a function that returns 'int' and the user tries to return '1.2' we should probably warn for any function.<br class="m_-6961844705446483138gmail_msg">
<br class="m_-6961844705446483138gmail_msg">
</div></div>Good point, we already have -Wliteral-conversion (which catches 1.2)<br class="m_-6961844705446483138gmail_msg">
and -Wconstant-conversion (which catches 0x1234567800000000ULL), and<br class="m_-6961844705446483138gmail_msg">
-Wint-conversion for C programs returning awful things like string<br class="m_-6961844705446483138gmail_msg">
literals, all of which are enabled by default. Perhaps Boolean values<br class="m_-6961844705446483138gmail_msg">
really are the only case we don't explicitly warn about.<br class="m_-6961844705446483138gmail_msg"></blockquote><div class="m_-6961844705446483138gmail_msg"><br class="m_-6961844705446483138gmail_msg"></div></div></div></div><div dir="ltr" class="m_-6961844705446483138gmail_msg"><div class="gmail_extra m_-6961844705446483138gmail_msg"><div class="gmail_quote m_-6961844705446483138gmail_msg"><div class="m_-6961844705446483138gmail_msg">Wow, I'm amazed that we have no warning at all for converting, say, 'true' to int (or indeed to float).</div><div class="m_-6961844705446483138gmail_msg"><br class="m_-6961844705446483138gmail_msg"></div><div class="m_-6961844705446483138gmail_msg">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.</div><div class="m_-6961844705446483138gmail_msg"><br class="m_-6961844705446483138gmail_msg"></div><div class="m_-6961844705446483138gmail_msg">So I think that means we want two or possibly three checks here:</div><div class="m_-6961844705446483138gmail_msg"><br class="m_-6961844705446483138gmail_msg"></div><div class="m_-6961844705446483138gmail_msg">1) main returns bool literal</div><div class="m_-6961844705446483138gmail_msg">2) -Wliteral-conversion warning for converting bool literal to another type (excluding 1-bit unsigned integral bit-fields)</div><div class="m_-6961844705446483138gmail_msg">3) and possibly: warning for implicit conversion of bool to floating-point (new subgroup of -Wbool-conversion?)</div><div class="m_-6961844705446483138gmail_msg"><br class="m_-6961844705446483138gmail_msg"></div><div class="m_-6961844705446483138gmail_msg">(ordered from most- to least-reasonable to enable by default).</div></div></div></div><div dir="ltr" class="m_-6961844705446483138gmail_msg"><div class="gmail_extra m_-6961844705446483138gmail_msg"><div class="gmail_quote m_-6961844705446483138gmail_msg"><div class="m_-6961844705446483138gmail_msg"><br class="m_-6961844705446483138gmail_msg"></div><blockquote class="gmail_quote m_-6961844705446483138gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
~Aaron<br class="m_-6961844705446483138gmail_msg">
<div class="m_-6961844705446483138m_2672333832697950803HOEnZb m_-6961844705446483138gmail_msg"><div class="m_-6961844705446483138m_2672333832697950803h5 m_-6961844705446483138gmail_msg"><br class="m_-6961844705446483138gmail_msg">
><br class="m_-6961844705446483138gmail_msg">
>  -Hal<br class="m_-6961844705446483138gmail_msg">
><br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> ~Aaron<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
>> ><br class="m_-6961844705446483138gmail_msg">
>> > Thanks again,<br class="m_-6961844705446483138gmail_msg">
>> > Hal<br class="m_-6961844705446483138gmail_msg">
>> ><br class="m_-6961844705446483138gmail_msg">
>> >><br class="m_-6961844705446483138gmail_msg">
>> >> ~Aaron<br class="m_-6961844705446483138gmail_msg">
>> >><br class="m_-6961844705446483138gmail_msg">
>> >> ><br class="m_-6961844705446483138gmail_msg">
>> >> > ______________________________<wbr>_________________<br class="m_-6961844705446483138gmail_msg">
>> >> > cfe-commits mailing list<br class="m_-6961844705446483138gmail_msg">
>> >> > <a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a><br class="m_-6961844705446483138gmail_msg">
>> >> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" class="m_-6961844705446483138gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br class="m_-6961844705446483138gmail_msg">
>> >> ><br class="m_-6961844705446483138gmail_msg">
>> >> ______________________________<wbr>_________________<br class="m_-6961844705446483138gmail_msg">
>> >> cfe-commits mailing list<br class="m_-6961844705446483138gmail_msg">
>> >> <a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a><br class="m_-6961844705446483138gmail_msg">
>> >> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" class="m_-6961844705446483138gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br class="m_-6961844705446483138gmail_msg">
>> >><br class="m_-6961844705446483138gmail_msg">
>> ><br class="m_-6961844705446483138gmail_msg">
>> > --<br class="m_-6961844705446483138gmail_msg">
>> > Hal Finkel<br class="m_-6961844705446483138gmail_msg">
>> > Lead, Compiler Technology and Programming Languages<br class="m_-6961844705446483138gmail_msg">
>> > Leadership Computing Facility<br class="m_-6961844705446483138gmail_msg">
>> > Argonne National Laboratory<br class="m_-6961844705446483138gmail_msg">
>><br class="m_-6961844705446483138gmail_msg">
><br class="m_-6961844705446483138gmail_msg">
> --<br class="m_-6961844705446483138gmail_msg">
> Hal Finkel<br class="m_-6961844705446483138gmail_msg">
> Lead, Compiler Technology and Programming Languages<br class="m_-6961844705446483138gmail_msg">
> Leadership Computing Facility<br class="m_-6961844705446483138gmail_msg">
> Argonne National Laboratory<br class="m_-6961844705446483138gmail_msg">
______________________________<wbr>_________________<br class="m_-6961844705446483138gmail_msg">
cfe-commits mailing list<br class="m_-6961844705446483138gmail_msg">
<a href="mailto:cfe-commits@lists.llvm.org" class="m_-6961844705446483138gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a><br class="m_-6961844705446483138gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" class="m_-6961844705446483138gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br class="m_-6961844705446483138gmail_msg">
</div></div></blockquote></div></div></div></blockquote></div></div></div></div>
</blockquote></div><br></div>