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