[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] Fix to PR15845 - Clang accepts invalid code
From:       Richard Smith <richard () metafoo ! co ! uk>
Date:       2013-04-30 22:45:26
Message-ID: CAOfiQqnm5x_ku_1OMAHpERC+-=t9qchAmPk2Pin-k8hDDoCOmA () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Mon, Apr 29, 2013 at 10:31 AM, Serge Pavlov <sepavloff@gmail.com> wrote:

> Hi Richard,
>
>
> 2013/4/29 Richard Smith <richard@metafoo.co.uk>
> [...]
>
>> 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.
>

Thanks, committed as r180822.


>
>
>> 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
>

[Attachment #5 (text/html)]

<div dir="ltr">On Mon, Apr 29, 2013 at 10:31 AM, Serge Pavlov <span dir="ltr">&lt;<a \
href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a>&gt;</span> \
wrote:<br><div class="gmail_extra"><div class="gmail_quote"> <blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
dir="ltr"><div>Hi Richard,<br><br></div><div class="gmail_extra"> <br><div \
class="gmail_extra">2013/4/29 Richard Smith <span dir="ltr">&lt;<a \
href="mailto:richard@metafoo.co.uk" \
target="_blank">richard@metafoo.co.uk</a>&gt;</span><br> <div>[...] <br></div><div \
class="im"><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div \
dir="ltr">The problem is that clang in C++ mode accepts the code:<br> <div \
class="gmail_extra"> <div class="gmail_quote"><div><blockquote class="gmail_quote" \
style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div \
dir="ltr"><div>    int foo(xxx);<br> Clang intentionally accepts this code due to a \
check in Parser::ParseImplicitInt, which appeared in r156854.<br>



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.</div></div>


</blockquote><div><br></div></div><div>If it is indeed the case that MSVC does not \
allow implicit int in C++, then we should absolutely remove that \
&quot;extension&quot;. However, someone presumably added it for a reason, so I&#39;d \
like to be sure that we&#39;ve checked this thoroughly before proceeding. Does MSVC \
allow implicit int in any other contexts? For instance...</div>

</div></div></div></blockquote><div> <br></div></div><div>MSVC doesn&#39;t allow \
implicit int in any context if in C++ mode, details are in \
bugzilla.<br><br></div><div class="im"><blockquote class="gmail_quote" \
style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">


<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div> </div></div></div></div></blockquote><blockquote class="gmail_quote" \
style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div \
dir="ltr"> <div class="gmail_extra"><div class="gmail_quote">
<div>const n = 0; // ok?</div><div>static f() { // ok?</div><div>  extern m; // \
ok?</div><div>  return m;</div><div>}</div></div></div></div></blockquote><div> \
</div></div><div>None of these cases are accepted by MSVC. <br> </div><div \
class="im"> <div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div \
dir="ltr"><div class="gmail_extra"><div class="gmail_quote"> <div>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 &#39;int foo(xxx);&#39; \
would not have implicit int applied, whereas &#39;int foo(const xxx);&#39; would, and \
we should make the parser handle that correctly.</div>

<div>
<div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div \
dir="ltr"><div>

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.<br></div></div></blockquote><div>



<br></div></div><div>Have you checked whether they treat it as an implicit int, or \
whether they treat it as an (ignored, presumably) identifier \
list?</div></div></div></div></blockquote><div><br></div></div><div>They are ignored. \
For instance, both MSVC and GCC successfully compile the following code:<br>

<br>void abc(xxx);<br>void abc(int x, char*y) {}<br>  <br></div><div \
class="im"><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
 <div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote"><div> Also, do you actually have code which relies upon this \
extension? If not, let&#39;s not add it \
gratuitously.</div></div></div></div></blockquote><div> </div></div><div>I know \
nothing about such, the intent was to make behavior more compatible. Probably it \
doesn&#39;t worth implementation.<br>

<br></div><div class="im"><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div \
dir="ltr"><div class="gmail_extra"> <div class="gmail_quote"><div>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&#39;re \
basically unrelated, and make more sense to review separately.</div>

</div></div></div></blockquote><div> </div></div><div>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.</div> \
</div></div></div></blockquote><div><br></div><div style>Thanks, committed as \
r180822.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px \
0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
 <div dir="ltr"><div class="gmail_extra"><div class="gmail_extra"><div><span \
style="color:rgb(80,0,80)"> </span></div><div class="im"><blockquote \
class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
 <div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><blockquote \
class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">



<div dir="ltr"><div>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>





diff --git a/include/clang/Basic/DiagnosticSemaKinds.td \
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&lt;<br>   &quot;&#39;void&#39; \
must be the first and only parameter if specified&quot;&gt;;<br> def \
err_void_param_qualified : Error&lt;<br>   &quot;&#39;void&#39; as parameter must not \
have type qualifiers&quot;&gt;;<br>




-def err_ident_list_in_fn_declaration : Error&lt;<br>-  &quot;a parameter list \
without types is only allowed in a function definition&quot;&gt;;<br>+def \
warn_ident_list_in_fn_declaration : Warning&lt;<br>+  &quot;a parameter list without \
types is only allowed in a function definition&quot;&gt;,<br>




+  InGroup&lt;C99Compat&gt;;<br></div></div></blockquote><div><br></div></div><div>This \
should be an ExtWarn, not a Warning, since this is a required diagnostic per the \
various C language standards. Also, C99Compat seems wrong.</div>

</div></div></div></blockquote><div><br></div></div><div>Thank you for the \
explanation. <br><br></div><div class="im"><blockquote class="gmail_quote" \
style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
 <div dir="ltr">
<div class="gmail_extra"><div class="gmail_quote"><div>
<div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
 <div dir="ltr"><div> def ext_param_not_declared : Extension&lt;<br>   \
&quot;parameter %0 was not declared, defaulting to type &#39;int&#39;&quot;&gt;;<br> \
def err_param_typedef_of_void : Error&lt;<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 &amp;DS, 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&#39;t &quot;officially&quot; 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 &amp;&amp; DSC != DSC_trailing \
&amp;&amp;<br>-      (!getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt) \
&amp;&amp;<br>




+      !getLangOpts().CPlusPlus \
&amp;&amp;<br></div></div></blockquote><div><br></div></div><div>There is a matching \
check in lib/Sema/DeclSpec.cpp, and possibly elsewhere. If we&#39;re not enabling \
implicit int in -fms-extensions mode, we need to do that consistently throughout the \
compiler.</div>

</div></div></div></blockquote><div><br></div></div><div>Indeed, SemaType.cpp also \
contains similar check. <br></div><div class="im"><div> </div><blockquote \
class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">


<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><blockquote \
class="gmail_quote" style="margin:0pt 0pt 0pt \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
 <div dir="ltr"><div>
       isValidAfterIdentifierInDeclarator(NextToken())) {<br>     // If this token is \
                valid for implicit int, e.g. &quot;static x = 4&quot;, 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 *GetFullTypeForDeclarator(TypeProcessingState \
&amp;state,<br>




         if (FTI.NumArgs &amp;&amp; FTI.ArgInfo[0].Param == 0) {<br>           // C99 \
6.7.5.3p3: Reject int(x,y,z) when it&#39;s not a function<br>           // \
definition.<br>-          S.Diag(FTI.ArgInfo[0].IdentLoc, \
diag::err_ident_list_in_fn_declaration);<br>




+          S.Diag(FTI.ArgInfo[0].IdentLoc, \
diag::warn_ident_list_in_fn_declaration);<br>           \
D.setInvalidType(true);<br></div></div></blockquote><div><br></div></div><div>If \
you&#39;re not issuing an error, you must build a correct AST -- you can&#39;t set \
things invalid.</div>


<div> </div></div></div></div></blockquote></div><div>My fault... \
<br>[...]<br><br><br></div>Updated patch:<br><br></div></div><div \
class="gmail_extra">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<div class="im"><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 &amp;DS, 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&#39;t &quot;officially&quot; 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 &amp;&amp; DSC != DSC_trailing \
&amp;&amp;<br>-      (!getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt) \
&amp;&amp;<br>

+      !getLangOpts().CPlusPlus &amp;&amp;<br></div><div class="im">       \
isValidAfterIdentifierInDeclarator(NextToken())) {<br>     // If this token is valid \
for implicit int, e.g. &quot;static x = 4&quot;, then<br>     // we just avoid eating \
the identifier, so it will be parsed as the<br> </div>
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 &amp;D, Preprocessor &amp;PP) \
{<br>

   // the type specifier is not optional, but we got &#39;auto&#39; as a storage<br>  \
// class specifier, then assume this is an attempt to use C++0x&#39;s \
&#39;auto&#39;<br>   // type specifier.<br>-  // FIXME: Does Microsoft really support \
implicit int in C++?<br>

-  if (PP.getLangOpts().CPlusPlus &amp;&amp; !PP.getLangOpts().MicrosoftExt \
&amp;&amp;<br>+  if (PP.getLangOpts().CPlusPlus &amp;&amp;<br>       TypeSpecType == \
TST_unspecified &amp;&amp; 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 ConvertDeclSpecToType(TypeProcessingState \
&amp;state) {<br>       // &quot;At least one type specifier shall be given in the \
declaration<br>       // specifiers in each declaration, and in the \
specifier-qualifier list in<br>

       // each struct declaration and type name.&quot;<br>-      // FIXME: Does \
Microsoft really have the implicit int extension in C++?<br>-      if \
(S.getLangOpts().CPlusPlus &amp;&amp;<br>-          !S.getLangOpts().MicrosoftExt) \
{<br>

+      if (S.getLangOpts().CPlusPlus) {<br>         S.Diag(DeclLoc, \
diag::err_missing_type_specifier)<br>           &lt;&lt; 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> 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;<div class="im"><br>diff --git \
a/test/Sema/MicrosoftCompatibility.cpp 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 -fms-compatibility<br>+<br>+// PR15845<br>+int foo(xxx); // \
expected-error{{unknown type name}}<br><br clear="all"></div></div> <span \
class=""><font color="#888888"><div class="gmail_extra"> <br>-- \
<br>Thanks,<br>--Serge<br> </div></font></span></div>
</blockquote></div><br></div></div>



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic