<div dir="ltr"><div><div><div><div><div>Clang now allows implicit int in C mode (-x c) in some cases:<br><br></div>abc = 1;<br></div>static foo() { return 1; }<br><br></div>But in function declaration it is now forbidden unlike to GCC and MSVC:<br>
<br></div>void func(xxx);<br><br></div>--Serge<br><div><div><div><div> <br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2013/4/30 Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">To be clear, MSVC does accept that code so long as you pass in the /TC<br>
flag. So, for instance, the test cases with a .c or .m extension<br>
should allow the implicit int (by default .c files are compiled with<br>
/TC in MSVC), but the .cpp/.mm files should disallow it (unless we<br>
force compilation as C with -x c, which would be the clang version of<br>
/TC).<br>
<br>
~Aaron<br>
<div><div class="h5"><br>
On Mon, Apr 29, 2013 at 1:31 PM, Serge Pavlov <<a href="mailto:sepavloff@gmail.com">sepavloff@gmail.com</a>> wrote:<br>
> Hi Richard,<br>
><br>
><br>
> 2013/4/29 Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> [...]<br>
>><br>
>> The problem is that clang in C++ mode accepts the code:<br>
>>><br>
>>> int foo(xxx);<br>
>>> Clang intentionally accepts this code due to a check in<br>
>>> Parser::ParseImplicitInt, which appeared in r156854.<br>
>>> The comment in the inserted code states that MS supports implicit int in<br>
>>> C++ mode, however it looks like none of VS2005, VS2008, VS2010 or VS2012<br>
>>> does it. So removing the check for MS extension solves the problem.<br>
>><br>
>><br>
>> If it is indeed the case that MSVC does not allow implicit int in C++,<br>
>> then we should absolutely remove that "extension". However, someone<br>
>> presumably added it for a reason, so I'd like to be sure that we've checked<br>
>> this thoroughly before proceeding. Does MSVC allow implicit int in any other<br>
>> contexts? For instance...<br>
><br>
><br>
> MSVC doesn't allow implicit int in any context if in C++ mode, details are<br>
> in bugzilla.<br>
><br>
>><br>
>><br>
>> const n = 0; // ok?<br>
>> static f() { // ok?<br>
>> extern m; // ok?<br>
>> return m;<br>
>> }<br>
><br>
><br>
> None of these cases are accepted by MSVC.<br>
><br>
>><br>
>> If MSVC does allow these, then the fix is different: the<br>
>> decl-specifier-seq (or, in C, the declaration-specifiers) for a parameter<br>
>> cannot be empty, so 'int foo(xxx);' would not have implicit int applied,<br>
>> whereas 'int foo(const xxx);' would, and we should make the parser handle<br>
>> that correctly.<br>
>><br>
>>><br>
>>> Another problem - the same code compiled in C mode produces an error,<br>
>>> while both GCC and MSC accept it. To fix it the message<br>
>>> err_ident_list_in_fn_declaration was converted into warning.<br>
>><br>
>><br>
>> Have you checked whether they treat it as an implicit int, or whether they<br>
>> treat it as an (ignored, presumably) identifier list?<br>
><br>
><br>
> They are ignored. For instance, both MSVC and GCC successfully compile the<br>
> following code:<br>
><br>
> void abc(xxx);<br>
> void abc(int x, char*y) {}<br>
><br>
>><br>
>> Also, do you actually have code which relies upon this extension? If not,<br>
>> let's not add it gratuitously.<br>
><br>
><br>
> I know nothing about such, the intent was to make behavior more compatible.<br>
> Probably it doesn't worth implementation.<br>
><br>
>> Please split this into its two constituent changes (removing implicit int<br>
>> in microsoft mode, and accepting an identifier-list in a non-defining<br>
>> function declaration). They're basically unrelated, and make more sense to<br>
>> review separately.<br>
><br>
><br>
> OK. This patch only removes implicit int in MS-compatibility mode for C++.<br>
> Fix to accepting an identifier-list in a non-defining function declaration<br>
> is dropped.<br>
><br>
>>><br>
>>> Files:<br>
>>> include/clang/Basic/DiagnosticSemaKinds.td<br>
>>> lib/Parse/ParseDecl.cpp<br>
>>> lib/Sema/SemaType.cpp<br>
>>> test/Sema/MicrosoftCompatibility.cpp<br>
>>> test/Sema/alloc_size.c<br>
>>> test/Sema/function.c<br>
>>> test/Sema/invalid-decl.c<br>
>>><br>
>>><br>
>>> -----------------------------------------------------------------------------------------------------<br>
>>> diff --git a/include/clang/Basic/DiagnosticSemaKinds.td<br>
>>> b/include/clang/Basic/DiagnosticSemaKinds.td<br>
>>> index 1461716..166dbab 100644<br>
>>> --- a/include/clang/Basic/DiagnosticSemaKinds.td<br>
>>> +++ b/include/clang/Basic/DiagnosticSemaKinds.td<br>
>>> @@ -2314,8 +2314,9 @@ def err_void_only_param : Error<<br>
>>> "'void' must be the first and only parameter if specified">;<br>
>>> def err_void_param_qualified : Error<<br>
>>> "'void' as parameter must not have type qualifiers">;<br>
>>> -def err_ident_list_in_fn_declaration : Error<<br>
>>> - "a parameter list without types is only allowed in a function<br>
>>> definition">;<br>
>>> +def warn_ident_list_in_fn_declaration : Warning<<br>
>>> + "a parameter list without types is only allowed in a function<br>
>>> definition">,<br>
>>> + InGroup<C99Compat>;<br>
>><br>
>><br>
>> This should be an ExtWarn, not a Warning, since this is a required<br>
>> diagnostic per the various C language standards. Also, C99Compat seems<br>
>> wrong.<br>
><br>
><br>
> Thank you for the explanation.<br>
><br>
>><br>
>>><br>
>>> def ext_param_not_declared : Extension<<br>
>>> "parameter %0 was not declared, defaulting to type 'int'">;<br>
>>> def err_param_typedef_of_void : Error<<br>
>>> diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp<br>
>>> index d786ce2..2f0c1a3 100644<br>
>>> --- a/lib/Parse/ParseDecl.cpp<br>
>>> +++ b/lib/Parse/ParseDecl.cpp<br>
>>> @@ -2038,10 +2038,9 @@ bool Parser::ParseImplicitInt(DeclSpec &DS,<br>
>>> CXXScopeSpec *SS,<br>
>>> // error, do lookahead to try to do better recovery. This never<br>
>>> applies<br>
>>> // within a type specifier. Outside of C++, we allow this even if the<br>
>>> // language doesn't "officially" support implicit int -- we support<br>
>>> - // implicit int as an extension in C99 and C11. Allegedly, MS also<br>
>>> - // supports implicit int in C++ mode.<br>
>>> + // implicit int as an extension in C99 and C11.<br>
>>> if (DSC != DSC_type_specifier && DSC != DSC_trailing &&<br>
>>> - (!getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt) &&<br>
>>> + !getLangOpts().CPlusPlus &&<br>
>><br>
>><br>
>> There is a matching check in lib/Sema/DeclSpec.cpp, and possibly<br>
>> elsewhere. If we're not enabling implicit int in -fms-extensions mode, we<br>
>> need to do that consistently throughout the compiler.<br>
><br>
><br>
> Indeed, SemaType.cpp also contains similar check.<br>
><br>
>>><br>
>>> isValidAfterIdentifierInDeclarator(NextToken())) {<br>
>>> // If this token is valid for implicit int, e.g. "static x = 4",<br>
>>> then<br>
>>> // we just avoid eating the identifier, so it will be parsed as the<br>
>>> diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp<br>
>>> index 8bf5143..243b772 100644<br>
>>> --- a/lib/Sema/SemaType.cpp<br>
>>> +++ b/lib/Sema/SemaType.cpp<br>
>>> @@ -2742,7 +2742,7 @@ static TypeSourceInfo<br>
>>> *GetFullTypeForDeclarator(TypeProcessingState &state,<br>
>>> if (FTI.NumArgs && FTI.ArgInfo[0].Param == 0) {<br>
>>> // C99 6.7.5.3p3: Reject int(x,y,z) when it's not a function<br>
>>> // definition.<br>
>>> - S.Diag(FTI.ArgInfo[0].IdentLoc,<br>
>>> diag::err_ident_list_in_fn_declaration);<br>
>>> + S.Diag(FTI.ArgInfo[0].IdentLoc,<br>
>>> diag::warn_ident_list_in_fn_declaration);<br>
>>> D.setInvalidType(true);<br>
>><br>
>><br>
>> If you're not issuing an error, you must build a correct AST -- you can't<br>
>> set things invalid.<br>
>><br>
><br>
> My fault...<br>
> [...]<br>
><br>
><br>
> Updated patch:<br>
><br>
> Files:<br>
> lib/Parse/ParseDecl.cpp<br>
> lib/Sema/DeclSpec.cpp<br>
> lib/Sema/SemaType.cpp<br>
> test/Rewriter/<a href="http://rewrite-byref-in-nested-blocks.mm" target="_blank">rewrite-byref-in-nested-blocks.mm</a><br>
> test/Sema/MicrosoftCompatibility.cpp<br>
><br>
><br>
> diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp<br>
> index d786ce2..2f0c1a3 100644<br>
> --- a/lib/Parse/ParseDecl.cpp<br>
> +++ b/lib/Parse/ParseDecl.cpp<br>
> @@ -2038,10 +2038,9 @@ bool Parser::ParseImplicitInt(DeclSpec &DS,<br>
> CXXScopeSpec *SS,<br>
> // error, do lookahead to try to do better recovery. This never applies<br>
> // within a type specifier. Outside of C++, we allow this even if the<br>
> // language doesn't "officially" support implicit int -- we support<br>
> - // implicit int as an extension in C99 and C11. Allegedly, MS also<br>
> - // supports implicit int in C++ mode.<br>
> + // implicit int as an extension in C99 and C11.<br>
> if (DSC != DSC_type_specifier && DSC != DSC_trailing &&<br>
> - (!getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt) &&<br>
> + !getLangOpts().CPlusPlus &&<br>
> isValidAfterIdentifierInDeclarator(NextToken())) {<br>
> // If this token is valid for implicit int, e.g. "static x = 4", then<br>
> // we just avoid eating the identifier, so it will be parsed as the<br>
> diff --git a/lib/Sema/DeclSpec.cpp b/lib/Sema/DeclSpec.cpp<br>
> index 124f50c..3b3ab2c 100644<br>
> --- a/lib/Sema/DeclSpec.cpp<br>
> +++ b/lib/Sema/DeclSpec.cpp<br>
> @@ -1003,8 +1003,7 @@ void DeclSpec::Finish(DiagnosticsEngine &D,<br>
> Preprocessor &PP) {<br>
> // the type specifier is not optional, but we got 'auto' as a storage<br>
> // class specifier, then assume this is an attempt to use C++0x's 'auto'<br>
> // type specifier.<br>
> - // FIXME: Does Microsoft really support implicit int in C++?<br>
> - if (PP.getLangOpts().CPlusPlus && !PP.getLangOpts().MicrosoftExt &&<br>
> + if (PP.getLangOpts().CPlusPlus &&<br>
> TypeSpecType == TST_unspecified && StorageClassSpec == SCS_auto) {<br>
> TypeSpecType = TST_auto;<br>
> StorageClassSpec = SCS_unspecified;<br>
> diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp<br>
> index 8bf5143..2038f12 100644<br>
> --- a/lib/Sema/SemaType.cpp<br>
> +++ b/lib/Sema/SemaType.cpp<br>
> @@ -793,9 +793,7 @@ static QualType<br>
> ConvertDeclSpecToType(TypeProcessingState &state) {<br>
> // "At least one type specifier shall be given in the declaration<br>
> // specifiers in each declaration, and in the specifier-qualifier<br>
> list in<br>
> // each struct declaration and type name."<br>
> - // FIXME: Does Microsoft really have the implicit int extension in<br>
> C++?<br>
> - if (S.getLangOpts().CPlusPlus &&<br>
> - !S.getLangOpts().MicrosoftExt) {<br>
> + if (S.getLangOpts().CPlusPlus) {<br>
> S.Diag(DeclLoc, diag::err_missing_type_specifier)<br>
> << DS.getSourceRange();<br>
><br>
> diff --git a/test/Rewriter/<a href="http://rewrite-byref-in-nested-blocks.mm" target="_blank">rewrite-byref-in-nested-blocks.mm</a><br>
> b/test/Rewriter/<a href="http://rewrite-byref-in-nested-blocks.mm" target="_blank">rewrite-byref-in-nested-blocks.mm</a><br>
> index 022bb5f..f416b66 100644<br>
> --- a/test/Rewriter/<a href="http://rewrite-byref-in-nested-blocks.mm" target="_blank">rewrite-byref-in-nested-blocks.mm</a><br>
> +++ b/test/Rewriter/<a href="http://rewrite-byref-in-nested-blocks.mm" target="_blank">rewrite-byref-in-nested-blocks.mm</a><br>
> @@ -19,7 +19,7 @@ void f(void (^block)(void));<br>
> - (void)foo {<br>
> __block int kerfluffle;<br>
> // radar 7692183<br>
> - __block x;<br>
> + __block int x;<br>
> f(^{<br>
> f(^{<br>
> y = 42;<br>
><br>
> diff --git a/test/Sema/MicrosoftCompatibility.cpp<br>
> b/test/Sema/MicrosoftCompatibility.cpp<br>
> new file mode 100644<br>
> index 0000000..15c2558<br>
> --- /dev/null<br>
> +++ b/test/Sema/MicrosoftCompatibility.cpp<br>
> @@ -0,0 +1,4 @@<br>
> +// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify<br>
> -fms-compatibility<br>
> +<br>
> +// PR15845<br>
> +int foo(xxx); // expected-error{{unknown type name}}<br>
><br>
><br>
> --<br>
> Thanks,<br>
> --Serge<br>
><br>
</div></div>> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
</blockquote></div><br>
</div></div></div></div>