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

Clean lexer #1263

Merged
merged 4 commits into from
Jun 20, 2016
Merged

Clean lexer #1263

merged 4 commits into from
Jun 20, 2016

Conversation

forki
Copy link
Contributor

@forki forki commented Jun 15, 2016

  • Removed unused permitFsharpKeywords switch
  • cleaned up pattern matching

IdentifierToken args lexbuf s
else v
else
match keywordTable.TryGetValue s with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please double check this return-a-tuple-and-match-on-it formulation produces code at least as good as before. I still tend to write the mutable by hand in performance-critical code but I know there is an optimization that should give us the right code here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  internal static Parser.token KeywordOrIdentifierToken(Lexhelp.lexargs args, LexBuffer<char> lexbuf, string s)
  {
    Parser.token token1 = (Parser.token) null;
    bool flag = Lexhelp.Keywords.keywordTable.TryGetValue(s, out token1);
    Parser.token token2 = token1;
    if (flag)
    {
         ...
    }
  }

@forki
Copy link
Contributor Author

forki commented Jun 17, 2016

Somehow the build is broken (unrelated to this)

Installed:
    93 package(s) to C:\projects\visualfsharp-3dtit\lkg\project.json
Error: dotnet restore failed

#sigh

@dsyme
Copy link
Contributor

dsyme commented Jun 17, 2016

Thanks for checking the generated code!

@dsyme
Copy link
Contributor

dsyme commented Jun 17, 2016

@KevinRansom We can merge this once CI is fixed

@dsyme dsyme closed this Jun 20, 2016
@dsyme dsyme reopened this Jun 20, 2016
@KevinRansom KevinRansom merged commit cad6ec2 into dotnet:master Jun 20, 2016
@KevinRansom
Copy link
Member

@forki Thank you for taking care of this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants