<div class="gmail_quote">On Sun, Feb 12, 2012 at 6:17 AM, Vasiliy Korchagin <span dir="ltr"><<a href="mailto:korchagin@ispras.ru">korchagin@ispras.ru</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

  
    
  
  <div bgcolor="#FFFFFF" text="#000000"><div><div class="h5">
    On 12.02.2012 07:39, Chandler Carruth wrote:
    <blockquote type="cite">
      <div class="gmail_quote">On Sat, Feb 11, 2012 at 5:31 AM, Vasiliy
        Korchagin <span dir="ltr"><<a href="mailto:korchagin@ispras.ru" target="_blank">korchagin@ispras.ru</a>></span>
        wrote:<br>
        <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
          I agree, without setting implicit return zero bit changes in
          codegen are not necessary. New version of patch is attached.</blockquote>
      </div>
      <div><br>
      </div>
      <div>The warning you're adding, as David suggested, should be
        under a separate flag.</div>
      <div><br>
      </div>
      <div>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).</div>
      <div><br>
      </div>
      <div>--- a/lib/Sema/SemaDecl.cpp</div>
      <div>+++ b/lib/Sema/SemaDecl.cpp</div>
      <div>@@ -5795,8 +5795,13 @@ void Sema::CheckMain(FunctionDecl* FD,
        const DeclSpec& DS) {</div>
      <div>   const FunctionType* FT =
        T->getAs<FunctionType>();</div>
      <div> </div>
      <div>   if
        (!Context.hasSameUnqualifiedType(FT->getResultType(),
        Context.IntTy)) {</div>
      <div>-    Diag(FD->getTypeSpecStartLoc(),
        diag::err_main_returns_nonint);</div>
      <div>-    FD->setInvalidDecl(true);</div>
      <div>
        +    if (getLangOptions().C99) {</div>
      <div>+      // In C we allow main() to have non-integer return
        type.</div>
      <div>+      Diag(FD->getTypeSpecStartLoc(),
        diag::warn_main_returns_nonint);</div>
      <div><br>
      </div>
      <div>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.</div>
      <div><br>
      </div>
      <div>
        <div>@@ -7204,7 +7209,8 @@ Decl
          *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,</div>
        <div>     if (FD->isMain()) {</div>
        <div>       // C and C++ allow for main to automagically return
          0.</div>
        <div>       // Implements C++ [basic.start.main]p5 and C99
          5.1.2.2.3.</div>
        <div>-      FD->setHasImplicitReturnZero(true);</div>
        <div>+      if (!getLangOptions().C99)</div>
        <div>+        FD->setHasImplicitReturnZero(true);</div>
      </div>
      <div><br>
      </div>
      <div>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.</div>
    </blockquote></div></div>
    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.</div></blockquote><div><br></div><div>This is super close. Two minor nits, and it should be good-to-go:</div><div><br></div><div>1) The canonical prefix for an ExtWarn diagnostic is "ext_", not "warn_".</div>
<div><br></div><div>2) I would name the test something more generic so we can fold more main tests into this file. 'main.c' would be fine.</div></div>