On Sep 4, 2011, at 2:48 PM, David Blaikie wrote:

> Here's a patch to add FixIt diagnostics to the "main cannot/should not be 
> static" and "main cannot be inline" warnings/errors.
> 
> Some context from a previous email conversation with Douglas Gregor:
>  
> No, main cannot be a template.
> 
> Seems the error for templated main was coming from the 
> ActOnFunctionDeclarator use case anyway - by the time we got to template 
> instantiation we'd already diagnosed this problem so it was just redundant to 
> check there.
>  
> 
>> So should we just push the isMain/checkMain checking up into 
>> ActOnFunctionDeclarator instead? Then it wouldn't add any overhead to 
>> irrelevant contexts (methods & templates) & the checking could be more 
>> descriptive.
> 
> Works for me!
> 
> So I've done that - pulling out the first few checks from 
> CheckFunctionDeclaration into the two call sites in ActOnFunctionDeclarator & 
> then passing the DeclSpec to the CheckMain calls. I could refactor that 
> common "IsMain/CheckMain/CheckFunctionDeclarator" into one function to use 
> from these two (non-C++ and C++) call sites in ActOnFunctionDeclarator if you 
> think that'd be better.
> 
> I also split up the static/inline warnings so they could warn against the 
> keyword rather than against the 'main' (when warning against the 'main' and 
> issuing a fixit for 'static' on a different line (or inside a macro), the 
> diagnostic just printed out without the fixit) - this seemed consistent with 
> other cases relating to spurious static in SemaDecl.cpp (some other cases 
> include the fixit, some don't - I'll see about making some patches for those 
> other missing fixits too).

+    Diag(DS.getStorageClassSpecLoc(), getLangOptions().CPlusPlus ? 
diag::err_static_main : diag::warn_static_main) << 
FixItHint::CreateRemoval(DS.getStorageClassSpecLoc());
+  if (FD->isInlineSpecified())
+    Diag(DS.getInlineSpecLoc(), diag::err_inline_main) << 
FixItHint::CreateRemoval(DS.getInlineSpecLoc());

Please limit code to 80 columns. Otherwise, looks good! If you don't have 
commit access already, please follow the directions at

        http://llvm.org/docs/DeveloperPolicy.html#commitaccess

so you can commit this yourself.


> After this I'll just keep working my way down through CheckMain to add fixits 
> to each of the suggestions where they're applicable.

Okay!

        - Doug

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to