Bug 3517 - [AST] Incomplete conditional operator in the AST request.
Summary: [AST] Incomplete conditional operator in the AST request.
Status: RESOLVED NOT_ON_ROADMAP
Alias: None
Product: Compilers
Classification: Mono
Component: C# ()
Version: unspecified
Hardware: PC Mac OS
: --- normal
Target Milestone: ---
Assignee: Marek Safar
URL:
: 4179 ()
Depends on:
Blocks:
 
Reported: 2012-02-17 10:40 UTC by Mike Krüger
Modified: 2015-12-03 07:29 UTC (History)
3 users (show)

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


Attachments
parser and tokenizer patch for creating AST nodes for partial conditional expressions (2.34 KB, application/octet-stream)
2012-04-08 14:32 UTC, Miguel de Icaza [MSFT]
Details


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 NOT_ON_ROADMAP

Description Mike Krüger 2012-02-17 10:40:34 UTC
I could need a working AST for these both cases:

---------
class Test
{
  void Foo ()
  {
     a = cond ? expr :
  }

}
--------

class Test
{
  void Foo ()
  {
     a = cond ? expr
  }

}
Comment 1 Miguel de Icaza [MSFT] 2012-02-29 21:38:25 UTC
There seems to be some code that would have produced the error, but still added the symbol to the AST already.

This change was added by Marek on February 21st, can you confirm if that fix works for you?

The commit was for:

26ba1ef537109de4204ff7d1ec4d13621b8042f9
Comment 2 Mike Krüger 2012-03-01 02:39:47 UTC
I merged last tuesday (28) - I synced newresolver with master monodevelop, mcs and nrefactory.

It seems that the behaviour has improved, but it's still not correct.

a = cond ? expr

is parsed to:

ExpressionStatemend -> IdentifierExpression -> expr
Comment 3 Mike Krüger 2012-03-01 02:47:15 UTC
Some more things where it doesn't work:

No statement at all:
----------
class Test
{
  void Foo ()
  {
     a = cond ? invoke ()
  }
}

Empty statement (';'):
----------
class Test
{
  void Foo ()
  {
     a = cond ? expr;
  }
}
Comment 4 Mike Krüger 2012-03-01 02:55:28 UTC
Playing around with that expression a bit more I found another case:

------
class Test
{
  void Foo ()
  {
     a = true ? expr
  }
}

------
Seems that some conditions cause that the whole expression isn't inserted in the AST.
Comment 5 Mike Krüger 2012-03-07 02:40:21 UTC
any news on this ?
Comment 6 Mike Krüger 2012-04-02 01:45:20 UTC
*** Bug 4179 has been marked as a duplicate of this bug. ***
Comment 7 Miguel de Icaza [MSFT] 2012-04-08 14:32:59 UTC
Created attachment 1633 [details]
parser and tokenizer patch for creating AST nodes for partial conditional expressions

There are two sets of problems to fully fix this.

* Deambiguator Problems *

Part of the problem with this bug is that the micro-deambiguator for the "?" token (ToeknizePossibleNullabelType) is based on heuristics more than being based on a micro-parser, so there are various cases that are not entirely properly handled.

Sadly there does not seem to be much information on the C# language spec about how to deal with this, and our test suite is a little light on which scenarios we should support.    I improved this lightly.

* Tokenizer lacks context *

The case:

a = foo ? b;

Is being parsed as:

IDENTIFIER ASSIGN IDENTIFIER TOKEN_NULLABLE_TYPE IDENTIFIER SEMICOLON

Because the tokenizer when it reaches the '?' can not determine based on the input ahead of it (IDENTIFIER, SEMICOLON) that this could be part of an incomplete conditional expression, and lets the parser sort it out.

The parser can not do much in terms of recovery at this point.

= Possible Paths =

It seems that "foo ? b" is an invalid expression, but my attempts to add support for this in the grammar only introduced more shift/reduce conflicts.   I could not find a way of having the parser generate an AST node for this.

But perhaps this is not a problem for the IDE, so I have added a production for FULL_AST, can you tell me if this works for you?

With this patch, and FULL_AST, all three cases in this bug report produce AST nodes
Comment 8 Marek Safar 2012-04-10 11:04:53 UTC
I don't think we need 

null_coalescing_expression INTERR_NULLABLE expression error

rule at all, we should always just fall back to INTERR.
Comment 9 Mike Krüger 2012-04-10 11:41:10 UTC
some things that don't work:

var a = foo ? b

a = true ? foo

and more problematic - boolean expressions:

a = foo != null ? b
Comment 10 Mike Krüger 2015-12-03 07:29:48 UTC
No longer required because of roslyn.