Skip to content

Conversation

@abonie
Copy link
Member

@abonie abonie commented Jun 14, 2023

Fixes: #13728

The problem occurred when INTERP_STRING_END token was offside in relation to INTERP_STRING_BEGIN_PART token. Too many contexts would be taken off the offside stack in hwFetchToken in LexFilter which would result in unfinished block error.

It seems to me that in case of interpolated strings, we can completely skip closing surrounding contexts.

@abonie abonie force-pushed the interpolated_string_offside_tokens_fix branch from a6d0177 to 62f3256 Compare June 14, 2023 16:28
@abonie abonie force-pushed the interpolated_string_offside_tokens_fix branch 3 times, most recently from 6b0c47b to cc8071f Compare June 19, 2023 09:14
@abonie
Copy link
Member Author

abonie commented Jun 20, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@abonie abonie force-pushed the interpolated_string_offside_tokens_fix branch from cc8071f to af5d478 Compare June 21, 2023 15:39
abonie added 2 commits June 22, 2023 14:40
We do not really care about interpolated string end token being offside
in relation to the interpolated string beginning. Not sure if not
inserting dummy token is right fix here though.
@abonie abonie force-pushed the interpolated_string_offside_tokens_fix branch from af5d478 to 4252e22 Compare June 22, 2023 12:40
@abonie
Copy link
Member Author

abonie commented Jun 22, 2023

@auduchinok given that you have done a lot of work in the parser - I can't think of any case that this change would break and it seems to fix the original issue as expected. However, I have difficult time fully wrapping my head around how the contexts' stack is used by hwTokenFetch (and debugging this function is pretty much impossible). Do you have any idea/intuition regarding what regression this change could cause?

@abonie abonie changed the title [WIP] Fix for offside errors on interpolated expressions Fix for offside errors on interpolated expressions Jun 26, 2023
@abonie abonie marked this pull request as ready for review June 26, 2023 16:01
@abonie abonie requested a review from a team as a code owner June 26, 2023 16:01
@auduchinok
Copy link
Member

@auduchinok given that you have done a lot of work in the parser - I can't think of any case that this change would break and it seems to fix the original issue as expected. However, I have difficult time fully wrapping my head around how the contexts' stack is used by hwTokenFetch (and debugging this function is pretty much impossible). Do you have any idea/intuition regarding what regression this change could cause?

No, from the top of my head, I don't expect anything to break from these changes.
Just in case, using fsc with --tokenize-debug flag is a tolerable substitute for debugging hwTokenFetch, as it prints a lot of info about the offset stack.

@auduchinok
Copy link
Member

auduchinok commented Jun 26, 2023

@abonie Could you, maybe, add some parser tests, so we have a particular tree structure captured with these changes?

@abonie
Copy link
Member Author

abonie commented Jun 27, 2023

Neat, I didn't know about the --tokenize-debug and I was just manually setting the forceDebug to true.

Added some syntax tree tests.

@abonie abonie enabled auto-merge (squash) June 27, 2023 14:08
@abonie abonie merged commit e33da89 into dotnet:main Jun 27, 2023
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.

Unfinished let error if a line of string interpolation starts with a format instructions inside of a module

4 participants