-
Notifications
You must be signed in to change notification settings - Fork 218
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
Partial (simplest possible case) fix for OnError GoTo #999
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GrahamTheCoder
commented
May 3, 2023
catch | ||
{ | ||
} | ||
while (catchIndex != -1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the example output - which is what's discouraging me from further work on this...
One other possible avenue might be to use something like DynamicMethod with IL.Emit to get around the restrictions of scopes and jump to labels with less boilerplate.
TODO: * Deal with non-returning code paths * Ensure loops within the method can still break/continue as expected
GrahamTheCoder
changed the title
OnError GoTo
Partial (simplest possible case) fix for OnError GoTo
May 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partial (simplest possible case) fix for #426
Simplest case covered here: A single top level On Error Go To, everything after it could be placed in a try catch and go to the outer label
May be possible to cover in future
OnErrorGoTo statement and label in same scope, but not top-level (e.g. nested in an if statment/loop)
Extend that system to two statements within a scope (e.g. changing the error handler part way through, it would be possible to have a nested try catch block for the second one, and use an filter clause to avoid hitting the outer one
Still no sensible known solution
General solution: The difficulty is that in C# the labels are scoped to the containing block, so it isn't possible to goto a label straight from a catch block. I've therefore created a loop and switch to allow it to re-enter the scope where it's possible to go to the label. That still won't work for labels inside if blocks and I'm still trying to think of a less verbose/ugly way of handling this in general.
Future
The solution may be to make a best effort at converting to working code, but to always spit out a code comment containing the original VB.
It'd also be possible in a comment to add a suggested starting point for a maintainable solution i.e. Just put it in a simple try catch block.