Bug 3578 - [AST] For condition not in the AST.
Summary: [AST] For condition not in the AST.
Status: RESOLVED FIXED
Alias: None
Product: Compilers
Classification: Mono
Component: C# ()
Version: unspecified
Hardware: PC Mac OS
: --- major
Target Milestone: ---
Assignee: Marek Safar
URL:
: 3570 3973 4286 ()
Depends on:
Blocks:
 
Reported: 2012-02-22 02:53 UTC by Mike Krüger
Modified: 2013-03-22 12:17 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Mike Krüger 2012-02-22 02:53:20 UTC
When there is a ; missing the condition is not put into the AST.

Testcase:
--------------
class Klass {
	void Test()
	{
		for (int i = 0; i < foo.bar)
	}
}
--------------
Comment 1 Mike Krüger 2012-02-22 02:54:06 UTC
*** Bug 3570 has been marked as a duplicate of this bug. ***
Comment 2 Miguel de Icaza [MSFT] 2012-02-29 21:30:36 UTC
Can you try this patch?


diff --git a/mcs/mcs/cs-parser.jay b/mcs/mcs/cs-parser.jay
index 2d204a7..cdf621c 100644
--- a/mcs/mcs/cs-parser.jay
+++ b/mcs/mcs/cs-parser.jay
@@ -5197,6 +5206,56 @@ for_statement_cont
 	  }
 	;
 
+iterator_part
+	:  opt_for_condition SEMICOLON
+	  {
+		For f = (For) oob_stack.Peek ();
+		f.Condition = (BooleanExpression) $1;
+	  }
+	  opt_for_iterator CLOSE_PARENS
+	  {
+		For f = (For) oob_stack.Peek ();
+		f.Iterator = (Statement) $4;
+		$$ = new Tuple<Location,Location> (GetLocation ($2), GetLocation ($5));
+	  }
+
+	  // Handle errors in the case of opt_for_condition being followed by
+	  // a close parenthesis
+	| opt_for_condition CLOSE_PARENS {
+	      report.Error (1525, GetLocation ($2), "Unexpected symbol ')', expected ';'");
+	      For f = (For) oob_stack.Peek ();
+	      f.Condition = (BooleanExpression) $1;
+	      $$ = new Tuple<Location,Location> (GetLocation ($2), GetLocation ($2));
+	  }
+	;
+
+// Has to use be extra rule to recover started block
+for_statement_cont
+	: opt_for_initializer SEMICOLON
+	  {
+		((For) $0).Initializer = (Statement) $1;
+		oob_stack.Push ($0);
+	  }
+	  iterator_part
+	  embedded_statement
+	  {
+		oob_stack.Pop ();
+		if ($5 is EmptyStatement && lexer.peek_token () == Token.OPEN_BRACE)
+			Warning_EmptyStatement (GetLocation ($5));
+	  
+		For f = ((For) $0);
+		f.Statement = (Statement) $5;
+		lbag.AddStatement (f, current_block.StartLocation, GetLocation ($2), ((Tuple<Location,Location>) $5).Item1, ((Tuple<Location,Location>) $5).Item2);
+
+		$$ = end_block (GetLocation ($2));
+	  }
+	| error
+	  {
+		Error_SyntaxError (yyToken);
+		$$ = end_block (current_block.StartLocation);
+	  }
+	;
+
 opt_for_initializer
 	: /* empty */		{ $$ = new EmptyStatement (lexer.Location); }
 	| for_initializer
Comment 3 Mike Krüger 2012-03-01 03:43:05 UTC
Doesn't work for me. 

What do you say to this rule change :
----------------------------------------
for_statement
	: FOR open_parens_any
	  {
		start_block (GetLocation ($2));
		current_block.IsCompilerGenerated = true;
		For f = new For (GetLocation ($1));
		current_block.AddStatement (f);
		lbag.AddStatement (f, current_block.StartLocation, GetLocation ($2));
		$$ = f;
	  }
	  for_statement_cont
	  {
		$$ = $4;
	  }
	;
	
// Has to use be extra rule to recover started block
for_statement_cont
	: opt_for_initializer SEMICOLON
	  {
		For f =  (For) $0;
		f.Initializer = (Statement) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = f;
	  }
	  for_statement_condition
	  {
		$$ = $4;
	  }
	| opt_for_initializer CLOSE_PARENS {
		report.Error (1525, GetLocation ($2), "Unexpected symbol ')', expected ';'");
		For f =  (For) $0;
		f.Initializer = (Statement) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = end_block (GetLocation ($2));
	}
	;

for_statement_condition
	: opt_for_condition SEMICOLON
	  {
		For f =  (For) $0;
		f.Condition = (BooleanExpression) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = f;
	  }
	  for_statement_end
	  {
		$$ = $4;
	  }
	| opt_for_condition CLOSE_PARENS {
		report.Error (1525, GetLocation ($2), "Unexpected symbol ')', expected ';'");
		For f =  (For) $0;
		f.Condition = (BooleanExpression) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = end_block (GetLocation ($2));
	}
	;

for_statement_end
	: opt_for_iterator CLOSE_PARENS
	  embedded_statement
	  {
		For f =  (For) $0;
		f.Iterator = (Statement) $1;
		
		if ($3 is EmptyStatement && lexer.peek_token () == Token.OPEN_BRACE)
			Warning_EmptyStatement (GetLocation ($3));
	  
		f.Statement = (Statement) $3;
		lbag.AppendTo (f, GetLocation ($2));

		$$ = end_block (GetLocation ($2));
	  }
	| error
	  {
		Error_SyntaxError (yyToken);
		$$ = end_block (current_block.StartLocation);
	  }
	;

----------------------------------------
Comment 4 Mike Krüger 2012-03-07 02:40:09 UTC
The patch doesn't work, the version I proposed works -> something needs to be done here.
Comment 5 Mike Krüger 2012-04-05 13:02:30 UTC
Ok my patch is junk that breaks it:

class TestClass
{
  static void Main(string[] args)
  {
     string[] foo = new string[] { "hello" };
     for(int i = 0; i != foo
  }
}
Comment 6 Mike Krüger 2012-04-05 13:03:18 UTC
*** Bug 3973 has been marked as a duplicate of this bug. ***
Comment 7 Mike Krüger 2012-04-06 06:23:00 UTC
*** Bug 4286 has been marked as a duplicate of this bug. ***
Comment 8 Miguel de Icaza [MSFT] 2012-04-06 17:05:00 UTC
The proper patch is on Git/master, can you try it out?
Comment 9 Mike Krüger 2012-04-07 05:05:16 UTC
tried - but I wasn't able to parse things with conditions.

I've changed my first grammar fix - and it now seems to work. At least completion in the condition works with that. 

Full patch:
--------------------------------------------------------------
for_statement
	: FOR open_parens_any
	  {
		start_block (GetLocation ($2));
		current_block.IsCompilerGenerated = true;
		For f = new For (GetLocation ($1));
		current_block.AddStatement (f);
		lbag.AddStatement (f, current_block.StartLocation);
		$$ = f;
	  }
	  for_statement_cont
	  {
		$$ = $4;
	  }
	;
	
// Has to use be extra rule to recover started block
for_statement_cont
	: opt_for_initializer SEMICOLON
	  {
		For f =  (For) $0;
		f.Initializer = (Statement) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = f;
	  }
	  for_statement_condition
	  {
		$$ = $4;
	  }
	| opt_for_initializer CLOSE_PARENS {
		report.Error (1525, GetLocation ($2), "Unexpected symbol ')', expected ';'");
		For f =  (For) $0;
		f.Initializer = (Statement) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = end_block (GetLocation ($2));
	}
	;

for_statement_condition
	: opt_for_condition SEMICOLON
	  {
		For f =  (For) $0;
		f.Condition = (BooleanExpression) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = f;
	  }
	  for_statement_end
	  {
		$$ = $4;
	  }
	| boolean_expression CLOSE_PARENS {
		report.Error (1525, GetLocation ($2), "Unexpected symbol ')', expected ';'");
		For f =  (For) $0;
		f.Condition = (BooleanExpression) $1;
		lbag.AppendTo (f, GetLocation ($2));
		$$ = end_block (GetLocation ($2));
	}
	;

for_statement_end
	: opt_for_iterator CLOSE_PARENS
	  embedded_statement
	  {
		For f =  (For) $0;
		f.Iterator = (Statement) $1;
		
		if ($3 is EmptyStatement && lexer.peek_token () == Token.OPEN_BRACE)
			Warning_EmptyStatement (GetLocation ($3));
	  
		f.Statement = (Statement) $3;
		lbag.AppendTo (f, GetLocation ($2));

		$$ = end_block (GetLocation ($2));
	  }
	| error
	  {
		Error_SyntaxError (yyToken);
		$$ = end_block (current_block.StartLocation);
	  }
	;
--------------------------------------------------------------

But even if that special case is fixed - there is a deeper problem.

The real problem for that is:
--------------------------------------------------------------
class TestClass
{
  static void Main(string[] args)
  {
     string[] foo = new string[] { "hello" };
     for(int i = 0; i != foo
  }
}
--------------------------------------------------------------

Leads to method.Body == null. That can't be really fixed in the for statement, this may apply to other statements as well. A bug in a statement should not cause the upper block to vanish from the AST.


For code completion I add brackets and end up with:
--------------------------------------------------------------
class TestClass
{
  static void Main(string[] args)
  {
     string[] foo = new string[] { "hello" };
     for(int i = 0; i != foo)
  }
}
--------------------------------------------------------------
that's now working, the for tests are green - but letting the file parse at this state:
--------------------------------------------------------------
class TestClass
{
  static void Main(string[] args)
  {
     string[] foo = new string[] { "hello" };
     for(int i = 0; i != foo|
  }
}
--------------------------------------------------------------
Cases this AST:

--------------------------------------------------------------
class TestClass
{
  static void Main(string[] args)


                            |
}

--------------------------------------------------------------
And then the completion engine can't find local variables or parameters at all.
Comment 10 Marek Safar 2013-03-22 12:17:38 UTC
I tried all enclosed cases and it seems to work with master