Patchwork fix Objective-C, further improve error recovery for unknown type names

login
register
mail settings
Submitter Paolo Bonzini
Date Nov. 13, 2010, 4:34 p.m.
Message ID <4CDEBE2F.8080406@gnu.org>
Download mbox | patch
Permalink /patch/71057/
State New
Headers show

Comments

Paolo Bonzini - Nov. 13, 2010, 4:34 p.m.
On 11/13/2010 03:15 PM, H.J. Lu wrote:
> On Sat, Oct 30, 2010 at 6:24 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>> This patch improves GCC error detection so that some cases of
>> declarations with unknown type names are detected.  This also
>> allows GCC to do better on cascading errors, because the variables
>> that are declared enter the symbol table.
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46462

Here is the fix.

Bootstrapped (together with the semicolon patch), regtest in progress 
and fixes all remaining failures (except the one caused by PR45062, ok 
for mainline assuming it passes?

Paolo
2010-11-13  Paolo Bonzini  <bonzini@gnu.org>

        PR c/46462
        * c-decl.c (declspecs_add_type): Make variables with error types
        integers.
        * c-parser.c (c_parser_next_tokens_start_declaration): Two IDs
        do not start a declaration before an Objective-C foreach.
        (c_parser_declaration_or_fndef): Improve recovery after unknown
        type name.
        (c_parser_for_statement): Hoist entrance of "foreach context"
        before ifs, add corresponding reset where it was missing.  Do
        not set objc_could_be_foreach_context for C.
Joseph S. Myers - Nov. 13, 2010, 5:26 p.m.
On Sat, 13 Nov 2010, Paolo Bonzini wrote:

> On 11/13/2010 03:15 PM, H.J. Lu wrote:
> > On Sat, Oct 30, 2010 at 6:24 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
> > > This patch improves GCC error detection so that some cases of
> > > declarations with unknown type names are detected.  This also
> > > allows GCC to do better on cascading errors, because the variables
> > > that are declared enter the symbol table.
> > 
> > This caused:
> > 
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46462
> 
> Here is the fix.
> 
> Bootstrapped (together with the semicolon patch), regtest in progress and
> fixes all remaining failures (except the one caused by PR45062, ok for
> mainline assuming it passes?

OK in the absence of any objections from ObjC maintainers within 24 hours.
IainS - Nov. 13, 2010, 6:02 p.m.
On 13 Nov 2010, at 17:26, Joseph S. Myers wrote:

> On Sat, 13 Nov 2010, Paolo Bonzini wrote:
>
>> On 11/13/2010 03:15 PM, H.J. Lu wrote:
>>> On Sat, Oct 30, 2010 at 6:24 AM, Paolo Bonzini<bonzini@gnu.org>   
>>> wrote:
>>>> This patch improves GCC error detection so that some cases of
>>>> declarations with unknown type names are detected.  This also
>>>> allows GCC to do better on cascading errors, because the variables
>>>> that are declared enter the symbol table.
>>>
>>> This caused:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46462
>>
>> Here is the fix.
>>
>> Bootstrapped (together with the semicolon patch), regtest in  
>> progress and
>> fixes all remaining failures (except the one caused by PR45062, ok  
>> for
>> mainline assuming it passes?
>
> OK in the absence of any objections from ObjC maintainers within 24  
> hours.

works for me (GNU & NeXT runtimes on i686-darwin9) ...
.. although I can neither object nor approve ;)

cheers
Iain
Nicola Pero - Nov. 13, 2010, 6:38 p.m.
>> Bootstrapped (together with the semicolon patch), regtest in progress and
>> fixes all remaining failures (except the one caused by PR45062, ok for
>> mainline assuming it passes?
>
> OK in the absence of any objections from ObjC maintainers within 24 hours.

I'm not an ObjC maintainer, but I wrote the ObjC fast enumeration parsing code 
that is being patched and I had a look at the patch - it looks Ok to me. :-)

I have one suggestion though --

> @@ -4653,14 +4658,15 @@ c_parser_for_statement (c_parser *parser
> +      parser->objc_could_be_foreach_context = c_dialect_objc ();                                                           

I would change that to always set objc_could_be_foreach_context to 'true'; in C,
it has not effect, so it does no matter what you set it to.  But if you always set it to
'true', then the nice assert that you added a few lines later to check that 
objc_could_be_foreach_context is always set back to 'false' --

> + gcc_assert(!parser->objc_could_be_foreach_context);

gets always exercised when you run the C testsuite, which means that we are confident
that objc_could_be_foreach_context is always restored to 'false' after parsing a C "for"
loop (which is also a valid ObjC "for" loop). :-)

Thanks
Paolo Bonzini - Nov. 13, 2010, 6:49 p.m.
On 11/13/2010 07:38 PM, Nicola Pero wrote:
> I would change that to always set objc_could_be_foreach_context to 'true'; in C,
> it has not effect, so it does no matter what you set it to.  But if you always set it to
> 'true', then the nice assert that you added a few lines later to check that
> objc_could_be_foreach_context is always set back to 'false' --
>
>> + gcc_assert(!parser->objc_could_be_foreach_context);
>
> gets always exercised when you run the C testsuite, which means that we are confident
> that objc_could_be_foreach_context is always restored to 'false' after parsing a C "for"
> loop (which is also a valid ObjC "for" loop). :-)

You are right.  However, there would be a subtle change, in that the 
shiny new unknown type name error would be disabled in C 'for' loop 
initial declarations, same as it is disabled for Objective C:

$ cat f.c
int f()
{
	for (intt i = 1; i < 10; i++)
		;
}

$ ./cc1 f.c -std=c99 -quiet
f.c: In function ‘f’:
f.c:3:7: error: unknown type name ‘intt’

$ ./cc1obj f.c -std=c99 -quiet
f.c:3:7: error: ‘intt’ undeclared (first use in this function)
f.c:3:12: error: expected ‘;’ before ‘i’
f.c:3:19: error: ‘i’ undeclared (first use in this function)

This can be fixed by adding a c_dialect_objc check to 
c_parser_next_tokens_start_declaration (after which you can do what you 
suggest).  You can even get the new error to work for ObjC if you check 
for RID_IN.  However, I'll leave it for a separate patch, possibly 
written by somebody with more interest in Objective-C. :)

Paolo
Nicola Pero - Nov. 13, 2010, 7:43 p.m.
> You are right.  However, there would be a subtle change, in that the 
> shiny new unknown type name error would be disabled in C 'for' loop 
> initial declarations, same as it is disabled for Objective C:

Thanks - you are right - I hadn't noticed that.

The C and Objective-C compilers should really behave identically when
parsing non-Objective-C things (such as a standard 'for' loop).  If 
your patch intentionally breaks that, it's a problem. ;-)

> This can be fixed by adding a c_dialect_objc check to 
> c_parser_next_tokens_start_declaration (after which you can do what you 
> suggest).  You can even get the new error to work for ObjC if you check 
> for RID_IN.  However, I'll leave it for a separate patch, possibly 
> written by somebody with more interest in Objective-C. :)

I think you should add the missing checks that you describe to restore
having the same behaviour in C and Objective-C; it probably would have 
taken you less time to fix your patch than to write the changes in an 
email to get me to fix it for you ;-)

But you're actually worrying me as I'm now fearful that many more differences 
between C and ObjC may have been introduced.  Maybe we should run the C 
testsuite with the compiler in ObjC mode ?

Thanks
Paolo Bonzini - Nov. 13, 2010, 8:13 p.m.
On 11/13/2010 08:43 PM, Nicola Pero wrote:
>
>> You are right.  However, there would be a subtle change, in that the
>> shiny new unknown type name error would be disabled in C 'for' loop
>> initial declarations, same as it is disabled for Objective C:
>
> Thanks - you are right - I hadn't noticed that.
>
> The C and Objective-C compilers should really behave identically when
> parsing non-Objective-C things (such as a standard 'for' loop).  If
> your patch intentionally breaks that, it's a problem. ;-)

I accepted to deviate a bit from this good rule for "for" loops, since 
Objective-C is not a superset of C there:

int f()
{
         for (int in = 0; ; )
                 ;
}

gives

f.c:3:11: error: expected identifier or ‘(’ before ‘in’

in Objective-C.  I think foreach syntax is a very bad idea, though not 
your fault of course.  "for (x : a)" would have been much better.

> I think you should add the missing checks that you describe to restore
> having the same behaviour in C and Objective-C; it probably would have
> taken you less time to fix your patch than to write the changes in an
> email to get me to fix it for you ;-)

I can do that.  However, not that it's only a matter of which errors you 
get.  My patch does not affect which valid C code is invalid 
Objective-C.  (Part of the idea of the mail was also to document all 
this for teh Google, otherwise I might as well have written it in 
Italian ;).

> But you're actually worrying me as I'm now fearful that many more differences
> between C and ObjC may have been introduced.

Not by me (this one was intentional, and I didn't touch Objective-C very 
much), but...

> Maybe we should run the C
> testsuite with the compiler in ObjC mode ?

... this is a good idea.

Paolo
IainS - Nov. 13, 2010, 8:31 p.m.
On 13 Nov 2010, at 20:13, Paolo Bonzini wrote:

> On 11/13/2010 08:43 PM, Nicola Pero wrote:
>>
>>> You are right.  However, there would be a subtle change, in that the
>>> shiny new unknown type name error would be disabled in C 'for' loop
>>> initial declarations, same as it is disabled for Objective C:
>>
>> Thanks - you are right - I hadn't noticed that.
>>
>> The C and Objective-C compilers should really behave identically when
>> parsing non-Objective-C things (such as a standard 'for' loop).  If
>> your patch intentionally breaks that, it's a problem. ;-)
>
> I accepted to deviate a bit from this good rule for "for" loops,  
> since Objective-C is not a superset of C there:
>
> int f()
> {
>        for (int in = 0; ; )
>                ;
> }
>
> gives
>
> f.c:3:11: error: expected identifier or ‘(’ before ‘in’
>
> in Objective-C.  I think foreach syntax is a very bad idea, though  
> not your fault of course.  "for (x : a)" would have been much better.

This probably reflects that we've still some bugs to shake out of the  
(new) implementation.
on Darwin, both "gcc-4.2 -x objective-c" and "clang -ObjC  "

warn (a) about the c99 requirement and (b) that the var "in" is unused.
(which seems like the correct behavior)

>> Maybe we should run the C
>> testsuite with the compiler in ObjC mode ?
>
> ... this is a good idea.

This is kinda on my TODO - with the idea to make it an optional add-on  
to the ObjC test-suite...
.. one would not want to adopt the additional burden of running the  
entire 'c' testsuite twice every test-cycle.

cheers
Iain
Nicola Pero - Nov. 13, 2010, 9:03 p.m.
>> I accepted to deviate a bit from this good rule for "for" loops,  
>> since Objective-C is not a superset of C there:
>>
>> int f()
>> {
>>        for (int in = 0; ; )
>>                ;
>> }
>>
>> gives
>>
>> f.c:3:11: error: expected identifier or ‘(’ before ‘in’
>>
>> in Objective-C.  I think foreach syntax is a very bad idea, though  
>> not your fault of course.  "for (x : a)" would have been much better.
>
> This probably reflects that we've still some bugs to shake out of the  
> (new) implementation.
> on Darwin, both "gcc-4.2 -x objective-c" and "clang -ObjC  "
>
> warn (a) about the c99 requirement and (b) that the var "in" is unused.
> (which seems like the correct behavior)

You may want to try a more interesting case --

 int in = 0;

 for (in = in + 1; in < 10; in++)
   printf ("%d\n", in);

try it in C and Objective-C.  Do you get the same result ?

In our implementation, I took the approach of simply not allowing 'in' as a variable 
in a 'for' loop in Objective-C.  That may be simplistic and we may want to be more
sophisticated; but if we follow a more complicated logic, we need to document it clearly.
It is ugly to have some hidden heuristics inside the compiler arbitrarily decide what
'in' means, as users have no way of knowing when 'in' can be used as a variable and 
when it can not. ;-)

Thanks
IainS - Nov. 13, 2010, 9:27 p.m.
On 13 Nov 2010, at 21:03, Nicola Pero wrote:

>
>>> I accepted to deviate a bit from this good rule for "for" loops,
>>> since Objective-C is not a superset of C there:
>>>
>>> int f()
>>> {
>>>       for (int in = 0; ; )
>>>               ;
>>> }
>>>
>>> gives
>>>
>>> f.c:3:11: error: expected identifier or ‘(’ before ‘in’
>>>
>>> in Objective-C.  I think foreach syntax is a very bad idea, though
>>> not your fault of course.  "for (x : a)" would have been much  
>>> better.
>>
>> This probably reflects that we've still some bugs to shake out of the
>> (new) implementation.
>> on Darwin, both "gcc-4.2 -x objective-c" and "clang -ObjC  "
>>
>> warn (a) about the c99 requirement and (b) that the var "in" is  
>> unused.
>> (which seems like the correct behavior)
>
> You may want to try a more interesting case --
>
> int in = 0;
>
> for (in = in + 1; in < 10; in++)
>   printf ("%d\n", in);
>
> try it in C and Objective-C.  Do you get the same result ?

OK Darwin, (OSX 10.6.5 Latest XCode)
clang gets it right (same answer with/without -ObjC)
gcc-4.2(.1) gets it right for 'c' and bails out 'confused by earlier  
errors' for -x objective-c

> In our implementation, I took the approach of simply not allowing  
> 'in' as a variable
> in a 'for' loop in Objective-C.  That may be simplistic and we may  
> want to be more
> sophisticated; but if we follow a more complicated logic, we need to  
> document it clearly.
> It is ugly to have some hidden heuristics inside the compiler  
> arbitrarily decide what
> 'in' means, as users have no way of knowing when 'in' can be used as  
> a variable and
> when it can not. ;-)

indeed, we should not be doing that....
... clang is the de-facto 'standard'  I guess ...

Iain
Mike Stump - Nov. 14, 2010, 1:51 a.m.
On Nov 13, 2010, at 10:02 AM, IainS <developer@sandoe-acoustics.co.uk> wrote:
> On 13 Nov 2010, at 17:26, Joseph S. Myers wrote:
> 
>> On Sat, 13 Nov 2010, Paolo Bonzini wrote:
>> 
>>> On 11/13/2010 03:15 PM, H.J. Lu wrote:
>>>> On Sat, Oct 30, 2010 at 6:24 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>>>>> This patch improves GCC error detection so that some cases of
>>>>> declarations with unknown type names are detected.  This also
>>>>> allows GCC to do better on cascading errors, because the variables
>>>>> that are declared enter the symbol table.
>>>> 
>>>> This caused:
>>>> 
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46462
>>> 
>>> Here is the fix.
>>> 
>>> Bootstrapped (together with the semicolon patch), regtest in progress and
>>> fixes all remaining failures (except the one caused by PR45062, ok for
>>> mainline assuming it passes?
>> 
>> OK in the absence of any objections from ObjC maintainers within 24 hours.
> 
> works for me (GNU & NeXT runtimes on i686-darwin9) ...
> .. although I can neither object nor approve ;)

Ok.
Mike Stump - Nov. 14, 2010, 5:18 a.m.
On Nov 13, 2010, at 8:34 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> Bootstrapped (together with the semicolon patch), regtest in progress and fixes all remaining failures (except the one caused by PR45062, ok for mainline assuming it passes?

Ok.
Mike Stump - Nov. 14, 2010, 5:37 a.m.
On Nov 13, 2010, at 1:27 PM, IainS <developer@sandoe-acoustics.co.uk> wrote:
> indeed, we should not be doing that....
> ... clang is the de-facto 'standard'  I guess ...

Yes, but we don't really want to be bug compatible with them, if we think they got it wrong.  One can always file a bug report against clang, if they want to know for sure.
IainS - Nov. 14, 2010, 9:59 a.m.
Hi Mike,

On 14 Nov 2010, at 05:37, Mike Stump wrote:

> On Nov 13, 2010, at 1:27 PM, IainS <developer@sandoe- 
> acoustics.co.uk> wrote:
>> indeed, we should not be doing that....
>> ... clang is the de-facto 'standard'  I guess ...
>
> Yes, but we don't really want to be bug compatible with them, if we  
> think they got it wrong.  One can always file a bug report against  
> clang, if they want to know for sure.

In this instance, it seems to me that clang is doing it right ...

Nicola's example:

> int in = 0;
>
> for (in = in + 1; in < 10; in++)
>  printf ("%d\n", in);

I would expect that to compile properly for both 'c' and 'objc' --
-- unless somewhere there is a statement that 'in' is a reserved word  
in that context.

(in which case we could file a bug against clang because it allows it,
   OSX's gcc-4.2.1 fails - but _not_ with an error that says 'wrong  
use of keyword' or similar).

what do you think is the proper behavior?
cheers,
Iain
Nicola Pero - Nov. 14, 2010, 11:08 a.m.
>>> indeed, we should not be doing that....
>>> ... clang is the de-facto 'standard'  I guess ...
>
>> Yes, but we don't really want to be bug compatible with them, if we  
>> think they got it wrong.  One can always file a bug report against  
>> clang, if they want to know for sure.
>
> In this instance, it seems to me that clang is doing it right ...

I agree that clang is doing it right in this instance [and our compiler
needs more work ;-)]; I think Mike was not commenting on this particular case, 
but just giving general guidelines on how to approach differences.

And I agree with his guidelines; we obviously want to be very compatible,
but if they get something wrong (not in this case) we don't necessarily need 
to be faithfully reproducing obvious bugs. :-)

Anyway, I created a PR for improving our compiler:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46473

Thanks
IainS - Nov. 14, 2010, 11:22 a.m.
On 14 Nov 2010, at 11:08, Nicola Pero wrote:

>
>>>> indeed, we should not be doing that....
>>>> ... clang is the de-facto 'standard'  I guess ...
>>
>>> Yes, but we don't really want to be bug compatible with them, if we
>>> think they got it wrong.  One can always file a bug report against
>>> clang, if they want to know for sure.
>>
>> In this instance, it seems to me that clang is doing it right ...
>
> I agree that clang is doing it right in this instance [and our  
> compiler
> needs more work ;-)]; I think Mike was not commenting on this  
> particular case,
> but just giving general guidelines on how to approach differences.

I read it that way - but also am interested in his view on this  
specific case, in case we missed something...

> And I agree with his guidelines; we obviously want to be very  
> compatible,
> but if they get something wrong (not in this case) we don't  
> necessarily need
> to be faithfully reproducing obvious bugs. :-)

completely agreed ... (although Darwin might end up needing a '- 
mcompat=apple' :/ )

Where does this leave us with the 'fix' to the current FE?
cheers
Iain
H.J. Lu - Nov. 14, 2010, 3:38 p.m.
On Sat, Nov 13, 2010 at 8:34 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 11/13/2010 03:15 PM, H.J. Lu wrote:
>>
>> On Sat, Oct 30, 2010 at 6:24 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>>>
>>> This patch improves GCC error detection so that some cases of
>>> declarations with unknown type names are detected.  This also
>>> allows GCC to do better on cascading errors, because the variables
>>> that are declared enter the symbol table.
>>
>> This caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46462
>
> Here is the fix.
>
> Bootstrapped (together with the semicolon patch), regtest in progress and
> fixes all remaining failures (except the one caused by PR45062, ok for
> mainline assuming it passes?
>

This may have caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46475
Mike Stump - Nov. 14, 2010, 3:58 p.m.
On Nov 14, 2010, at 1:59 AM, IainS <developer@sandoe-acoustics.co.uk> wrote:
> what do you think is the proper behavior?

When you, Nicola and clang all agree, I'd not even entertain a different answer.  :)
>
Mike Stump - Nov. 14, 2010, 4:06 p.m.
On Nov 14, 2010, at 3:08 AM, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:
> I think Mike was not commenting on this particular case, 
> but just giving general guidelines on how to approach differences.

Yes, just to head off any idea that we blindly follow clang without question.  

> And I agree with his guidelines; we obviously want to be very compatible,
> but if they get something wrong (not in this case) we don't necessarily need 
> to be faithfully reproducing obvious bugs. :-)

Nicely stated.  If some import apple header used a bug, I'm of course fine with implementing a bug....
>
Mike Stump - Nov. 14, 2010, 4:14 p.m.
On Nov 14, 2010, at 3:22 AM, IainS <developer@sandoe-acoustics.co.uk> wrote:
>>> 
>> 
> completely agreed ... (although Darwin might end up needing a '-mcompat=apple' :/ )

Ick, no, please.
>
H.J. Lu - Jan. 24, 2011, 2:45 p.m.
On Sat, Nov 13, 2010 at 8:34 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 11/13/2010 03:15 PM, H.J. Lu wrote:
>>
>> On Sat, Oct 30, 2010 at 6:24 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>>>
>>> This patch improves GCC error detection so that some cases of
>>> declarations with unknown type names are detected.  This also
>>> allows GCC to do better on cascading errors, because the variables
>>> that are declared enter the symbol table.
>>
>> This caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46462
>
> Here is the fix.
>
> Bootstrapped (together with the semicolon patch), regtest in progress and
> fixes all remaining failures (except the one caused by PR45062, ok for
> mainline assuming it passes?
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47435

Patch

Index: c-decl.c
===================================================================
--- c-decl.c	(revision 166700)
+++ c-decl.c	(working copy)
@@ -9324,6 +9324,11 @@  declspecs_add_type (location_t loc, stru
 	}
       specs->type = type;
     }
+  else
+    {
+      /* Set a dummy type here to avoid warning about implicit 'int'.  */
+      specs->type = integer_type_node;
+    }
 
   return specs;
 }
Index: c-parser.c
===================================================================
--- c-parser.c	(revision 166700)
+++ c-parser.c	(working copy)
@@ -641,7 +641,10 @@  c_parser_next_tokens_start_declaration (
       && token->id_kind == C_ID_ID
       && (c_parser_peek_2nd_token (parser)->type == CPP_NAME
           || c_parser_peek_2nd_token (parser)->type == CPP_MULT)
-      && !lookup_name (token->value))
+      && !lookup_name (token->value)
+
+      /* Do not try too hard when we could have "object in array".  */
+      && !parser->objc_could_be_foreach_context)
     return true;
 
   return false;
@@ -1373,10 +1376,12 @@  c_parser_declaration_or_fndef (c_parser 
                 c_parser_peek_token (parser)->value);
 
       /* Parse declspecs normally to get a correct pointer type, but avoid
-         a further "fails to be a type name" error.  */
+         a further "fails to be a type name" error.  Refuse nested functions
+         since it is not how the user likely wants us to recover.  */
       c_parser_peek_token (parser)->type = CPP_KEYWORD;
       c_parser_peek_token (parser)->keyword = RID_VOID;
       c_parser_peek_token (parser)->value = error_mark_node;
+      fndef_ok = !nested;
     }
 
   c_parser_declspecs (parser, specs, true, true, start_attr_ok);
@@ -4653,14 +4658,15 @@  c_parser_for_statement (c_parser *parser
     {
       /* Parse the initialization declaration or expression.  */
       object_expression = error_mark_node;
+      parser->objc_could_be_foreach_context = c_dialect_objc ();
       if (c_parser_next_token_is (parser, CPP_SEMICOLON))
 	{
+	  parser->objc_could_be_foreach_context = false;
 	  c_parser_consume_token (parser);
 	  c_finish_expr_stmt (loc, NULL_TREE);
 	}
       else if (c_parser_next_tokens_start_declaration (parser))
 	{
-	  parser->objc_could_be_foreach_context = true;
 	  c_parser_declaration_or_fndef (parser, true, true, true, true, true, 
 					 &object_expression);
 	  parser->objc_could_be_foreach_context = false;
@@ -4690,7 +4696,6 @@  c_parser_for_statement (c_parser *parser
 	      int ext;
 	      ext = disable_extension_diagnostics ();
 	      c_parser_consume_token (parser);
-	      parser->objc_could_be_foreach_context = true;
 	      c_parser_declaration_or_fndef (parser, true, true, true, true,
 					     true, &object_expression);
 	      parser->objc_could_be_foreach_context = false;
@@ -4714,7 +4719,6 @@  c_parser_for_statement (c_parser *parser
 	init_expr:
 	  {
 	    tree init_expression;
-	    parser->objc_could_be_foreach_context = true;
 	    init_expression = c_parser_expression (parser).value;
 	    parser->objc_could_be_foreach_context = false;
 	    if (c_parser_next_token_is_keyword (parser, RID_IN))
@@ -4735,6 +4739,7 @@  c_parser_for_statement (c_parser *parser
 	}
       /* Parse the loop condition.  In the case of a foreach
 	 statement, there is no loop condition.  */
+      gcc_assert (!parser->objc_could_be_foreach_context);
       if (!is_foreach_statement)
 	{
 	  if (c_parser_next_token_is (parser, CPP_SEMICOLON))