On Sun, Feb 12, 2012 at 6:17 AM, Vasiliy Korchagin <[email protected]>wrote:
> On 12.02.2012 07:39, Chandler Carruth wrote: > > On Sat, Feb 11, 2012 at 5:31 AM, Vasiliy Korchagin <[email protected]>wrote: > >> I agree, without setting implicit return zero bit changes in codegen are >> not necessary. New version of patch is attached. > > > The warning you're adding, as David suggested, should be under a > separate flag. > > It should also be an extwarn as this is technically a language > extension, and it should be enabled by default (you may already have that, > just want it clarified). > > --- a/lib/Sema/SemaDecl.cpp > +++ b/lib/Sema/SemaDecl.cpp > @@ -5795,8 +5795,13 @@ void Sema::CheckMain(FunctionDecl* FD, const > DeclSpec& DS) { > const FunctionType* FT = T->getAs<FunctionType>(); > > if (!Context.hasSameUnqualifiedType(FT->getResultType(), > Context.IntTy)) { > - Diag(FD->getTypeSpecStartLoc(), diag::err_main_returns_nonint); > - FD->setInvalidDecl(true); > + if (getLangOptions().C99) { > + // In C we allow main() to have non-integer return type. > + Diag(FD->getTypeSpecStartLoc(), diag::warn_main_returns_nonint); > > If you want this to be enabled in all C modes, you should accept more > than just C99: "!getLangOptions().CPlusPlus" is a good candidate here. > > @@ -7204,7 +7209,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, > Stmt *Body, > if (FD->isMain()) { > // C and C++ allow for main to automagically return 0. > // Implements C++ [basic.start.main]p5 and C99 5.1.2.2.3. > - FD->setHasImplicitReturnZero(true); > + if (!getLangOptions().C99) > + FD->setHasImplicitReturnZero(true); > > This isn't correct. You need to be checking for a non-int return type > here, not for C99. If the function has an int return type we want the > implicit return zero. > > Now new warning is under the separate flag ("-Wmain-return-type") and it > is an extwarn and it is enabled by default. Also problems with C modes and > implicit return zero are fixed. New patch is attached. > This is super close. Two minor nits, and it should be good-to-go: 1) The canonical prefix for an ExtWarn diagnostic is "ext_", not "warn_". 2) I would name the test something more generic so we can fold more main tests into this file. 'main.c' would be fine.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
