Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved parser recovery during typing #11817

Closed
gafter opened this issue Jun 7, 2016 · 4 comments
Closed

Improved parser recovery during typing #11817

gafter opened this issue Jun 7, 2016 · 4 comments
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. Feature Request help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@gafter
Copy link
Member

gafter commented Jun 7, 2016

See #11807 (comment) and #11807 (comment)

In code such as

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

class Program
{
    static void Main(string[] args)
    {
        var z = args.Select(a => a. // here
        var foo = 
    }
}

It would be helpful to the IDE if the parser would consider var foo = as the start of a separate statement.

@gafter gafter added Area-Compilers Feature Request Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. labels Jun 7, 2016
@gafter
Copy link
Member Author

gafter commented Jun 7, 2016

The quoted snippets from @CyrusNajmabadi are

This can be fixed though with a bit of lookahead (and i thought we already did this int he compiler in one place). Basically, instead of blindly consuming 'var' as the name of a., we instead peek ahead. If we see that we have something that would not be valid in the expression context we're in, we ask the following.

  1. Is 'var' on the next line?
  2. Is it followed by whitespace and then an identifier?

If both are true, then we should not consume it as a name. Once we've done that, then parser error recovery comes into play. I would not expect argument parsing to succeed here (absent a comma or other clear indicator that this is an arg, and especially given that it looks like the start of a statement), and i would expect statement parsing to say: "yup, this looks good". We'd then bail out of argument parsing and go into statement parsing.

We did this with ParseVariableDeclarator for example. This is how we make sure htat we parse the following properly:

                // C                    //<-- here
                // Console.WriteLine();

and

Note: i made this specific parsing fix for typescript which you can see here:

https://github.com/Microsoft/TypeScript/blob/master/src/compiler/parser.ts#L1850

There we look specifically for the pattern:

            // So, we check for the following specific case:
            //
            //      name.
            //      identifierOrKeyword identifierNameOrKeyword

If we see that we do not consume the first 'identifierOrKeyword' as the name of the member access.

Now, in TS this was Especially important as all keywords can be consumed on the right side of a 'dot'. However, it's still a really good heuristic that would improve thing for the 'var' (and other identifier) case for C#.

@CyrusNajmabadi
Copy link
Member

Can you please construct an example that shows how the suggestion would improve the IDE experience

Well.. the example is the code you've provided :)

THe improved parsing would have made is unnecessary to do the particular bit of error tolerance work in the binder that you're currently incurring :)

The overall idea is basically that all features flow from the quality of the parse tree. The more the parse tree matches the user's expectations, the better all other features (classification, formatting, completion, etc. etc. etc.) will work. The more horked the parse tree becomes, the more flakey and odd features will behave around the problem point.

So, in general, when we can cheaply see that something has gone wrong, and we have a reasonable heuristic to addressing it and producing a good tree, the better the entire rest of the experience is.

@gafter
Copy link
Member Author

gafter commented Jun 7, 2016

The improved parsing would have made is unnecessary to do the particular bit of error tolerance work in the binder that you're currently incurring

Not so. See #557, where the "problem" occurs so much further in the input that it is not obvious how to "correct" it locally. There is a comma in the source in #557 (though unbeknownst to the parser intended for another enclosing construct). The improved error tolerance in the binder in #11807 is needed to handle those cases.

@gafter gafter added this to the 2.1 milestone Jul 21, 2016
@jaredpar jaredpar added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Mar 9, 2017
@jaredpar jaredpar modified the milestones: Unknown, 15.1 Mar 9, 2017
@CyrusNajmabadi
Copy link
Member

Fixed with #15885

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. Feature Request help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
None yet
Development

No branches or pull requests

3 participants