13.02.2012 12:28, Vasiliy Korchagin пишет:
13.02.2012 05:39, Chandler Carruth пишет:
On Sun, Feb 12, 2012 at 6:17 AM, Vasiliy Korchagin <[email protected] <mailto:[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] <mailto:[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.
Warning prefix is changed and test is renamed.

Vasiliy Korchagin,
The Institute for System Programming of the Russian Academy of Sciences


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Sorry, last time patch was incorrect. Now it is ok.

- Vasiliy
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 8c821c9..0343b8a 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -104,6 +104,7 @@ def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
                                       [CXX98CompatLocalTypeTemplateArgs]>;
 def MalformedWarningCheck : DiagGroup<"malformed-warning-check">;
 def Main : DiagGroup<"main">;
+def MainReturnType : DiagGroup<"main-return-type">;
 def MissingBraces : DiagGroup<"missing-braces">;
 def MissingDeclarations: DiagGroup<"missing-declarations">;
 def : DiagGroup<"missing-format-attribute">;
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 3e6c190..4aed633 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -348,6 +348,8 @@ def err_constexpr_main : Error<
   "'main' is not allowed to be declared constexpr">;
 def err_main_template_decl : Error<"'main' cannot be a template">;
 def err_main_returns_nonint : Error<"'main' must return 'int'">;
+def ext_main_returns_nonint : ExtWarn<"return type of 'main' is not 'int'">,
+    InGroup<MainReturnType>;
 def err_main_surplus_args : Error<"too many parameters (%0) for 'main': "
     "must be 0, 2, or 3">;
 def warn_main_one_arg : Warning<"only one parameter on 'main' declaration">,
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 3c73f68..06a6fc7 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -5797,8 +5797,14 @@ 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().CPlusPlus) {
+      Diag(FD->getTypeSpecStartLoc(), diag::err_main_returns_nonint);
+      FD->setInvalidDecl(true);
+    } else {
+      // In C we allow main() to have non-integer return type, but we should
+      // warn about it.
+      Diag(FD->getTypeSpecStartLoc(), diag::ext_main_returns_nonint);
+    }
   }
 
   // Treat protoless main() as nullary.
@@ -7262,7 +7268,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 (FD->getResultType()->isIntegerType())
+        FD->setHasImplicitReturnZero(true);
       WP.disableCheckFallThrough();
     } else if (FD->hasAttr<NakedAttr>()) {
       // If the function is marked 'naked', don't complain about missing return
diff --git a/test/Sema/main.c b/test/Sema/main.c
new file mode 100644
index 0000000..e1dc114
--- /dev/null
+++ b/test/Sema/main.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void main() {} // expected-warning {{return type of 'main' is not 'int'}}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to