Skip to content

Conversation

@auduchinok
Copy link
Member

Adds recovery for various cases with unfinished record and anon record expressions:

{ F = }
{| F = |}
{| F |}
{| f().F |}

@auduchinok auduchinok requested a review from a team as a code owner September 8, 2023 16:45
@auduchinok
Copy link
Member Author

auduchinok commented Sep 8, 2023

The tests also show the peculiar way of how some unfinished records are parsed today. Consider these cases:

{ F1 =
  F2 = 2 }
{ F1 = 1
  F2 =
  F3 = 3 }

I'd expect them to be parsed as 2 and 3 fields respectively, with error recovery applied for the fields with missing expressions. What happens instead is fields following the unfinished ones are parsed as = binary expressions (AST: 1, 2) or even sequences of them (AST).

I doubt that there's many actual code written like this. We've seen this tree shape before during typing and it made it much harder to analyze this unexpected structure in the tooling.

It's possible to change the structure into the more expected one, but it'd be a breaking change. To me it fits into the strict indentation changes in F# 8 with the exception that there was no warning before. We could probably try requiring additional indentation in record fields expressions:

// good
{ F =
    1 }

// bad
{ F =
  1 }

Or at least do the rewriting as it's currently done for the first field:

{ F1 = 1 // this is parsed as binary expression and rewritten to a field binding
  F2 = 2 } // subsequent fields are parsed as field bindings

What do you think?

@auduchinok auduchinok closed this Sep 9, 2023
@auduchinok auduchinok reopened this Sep 9, 2023
@vzarytovskii
Copy link
Member

I doubt that there's many actual code written like this.

Well, that's famous last words..."don't think it's going to break anyone".

@psfinaki
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@T-Gro T-Gro enabled auto-merge (squash) September 19, 2023 09:10
@T-Gro T-Gro merged commit 7b1879f into dotnet:main Sep 19, 2023
@auduchinok auduchinok deleted the parser-anonRecord branch September 19, 2023 11:59
Copy link
Contributor

Choose a reason for hiding this comment

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

No error here?

Copy link
Member Author

@auduchinok auduchinok Sep 19, 2023

Choose a reason for hiding this comment

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

Yes, this is what my comment above is about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants