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 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. >> >> >> 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) -Wliteral-conversion warning for converting bool literal to another >> type (excluding 1-bit unsigned integral bit-fields) >> 3) and possibly: warning for implicit conversion of bool to >> floating-point (new subgroup of -Wbool-conversion?) >> >> (ordered from most- to least-reasonable to enable by default). >> >> ~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 >> >> >> > _______________________________________________ > 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
