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

Return should be legal in Subs #38

Open
bclothier opened this issue Oct 21, 2021 · 86 comments
Open

Return should be legal in Subs #38

bclothier opened this issue Oct 21, 2021 · 86 comments

Comments

@bclothier
Copy link
Collaborator

bclothier commented Oct 21, 2021

Describe the bug
Trying to Return from a Sub raises a "internal error"

To Reproduce
Create the code:

    Public Sub Duh()
        Return
    End Sub

It will highlight Return as an error.

Expected behavior
No error. It should be legal to return. Had I put in something to return (e.g. Return 1 or Return "foo"), then it's appropriate to show an error since it's a Sub.

BTW, I prefer this over Exit Sub because this avoids the need to change the code should it change from Sub to Function (or vice versa).

Desktop (please complete the following information):

  • OS: Windows 7
  • twinBASIC compiler version 11.120
@EduardoVB
Copy link

"Return", I understand means to return some value (in a Function or Property Get procedure).

But it could also be understood as "return to the calling procedure", although that meaning seems a bit forced IMO.

@bclothier
Copy link
Collaborator Author

This is prior arts in several other languages. In fact, there's prior art in VBx with its GoSub...Return misfeature.

@DaveInCaz
Copy link

This is prior arts in several other languages. In fact, there's prior art in VBx with its GoSub...Return misfeature.

Total aside, but in VB6 I often use Gosub...Return as a substitute for a finally block.

Something like...

On Error Goto handler
...

'Work is done
Gosub cleanup
Exit Sub

cleanup:
    'release resources or whatever is needed
    Return

handler:
   Gosub cleanup
   DoStandardErorrHandling(...)

End Sub

@EduardoVB
Copy link

This is prior arts in several other languages. In fact, there's prior art in VBx with its GoSub...Return misfeature.

I always understood the Return of Gosub as a "return to the calling line+1" but the Return of procedures that need to return a value as something different, as "return this value and exit".
But may be I misunderstood.

@WaynePhillipsEA
Copy link
Contributor

The problem with this request is indeed the conflict with the legacy GoSub... Return syntax. My original plan was to allow Return inside of Subs, but only if no GoSub had been encountered in the procedure, which would allow for full backwards compatibility when we implement the GoSub...Return syntax.

However, just Return in a procedure (without GoSub) in VBx produces a runtime error, not a compile error, so from a technical perspective we can't guarantee that nobody is relying on that behaviour (although it would be extremely unlikely, and very bad code).

@Kr00l
Copy link
Collaborator

Kr00l commented Oct 23, 2021

My two cents:
Prior art is a good argument. However, like with the Class Finalize there are restrictions.
So here the same with GoSub...Return.
I would disallow Return on a Sub that has no GoSub. Also to avoid confusion or misinterpretation at the end.

Exit Sub is just fine then. It's a one liner.

With Exit Function we can't return a value without having two lines. But that's not the case for Exit Sub.

@bclothier
Copy link
Collaborator Author

It's probably a runtime error because there is no require to balance the Return to the GoSub. Also contrary to what the documentation says, one can actually leave in middle of a GoSub...Return. it is possible to make the Return conditional based on execution, which is frankly an insane way to code but there it is. It's also likely that VBx didn't have the capability to do code analysis to determine the execution path and thus statically prevent Return without GoSub. This code demonstrate that we can have unbalanced Return which may then behave differently depending on which execution path is taken:

Public Sub Maybe(N As Long)
    If N < 1 Or N > 3 Then GoSub Leave
    
    On N GoSub MakeAChoice, BailOut, Leave
    
MakeAChoice:
    If N > 0 Then
BailOut:
        Return
    Else
Leave:
        Exit Sub
    End If
End Sub

Given that a GoSub and Return must both exist together within a procedure for it, it seems dubious that anyone would put in a Return in a Sub routine with no GoSub and be content to get RTE3. Does it make sense to put in an additional check for whether there's any error handling for RTE3 anywhere in the project? If there aren't, then isn't that a strong indication that the use of Return was incorrect? I don't remember what happens if there's an unhandled error in a compiled VB6 application -- wouldn't it simply abort?

@wqweto
Copy link

wqweto commented Oct 23, 2021

GoSub/Return statements force VB6 codegen to generate a hidden local variable to keep track of "gosub list/call-stack" and there is ___vbaGosubFree call in routine prolog to deallocate this data structure no matter if it's empty or still contains outstanding GoSub calls that never Returned.

I don't remember what happens if there's an unhandled error in a compiled VB6 application -- wouldn't it simply abort?

Pretty much the same as in .Net -- the exception is handled by parent try/catch block a.k.a "On Error GoTo Label" (if any) or if unhandled at all a run-time msgbox is displayed with the unhandled error description before aborting the process.

@EduardoVB
Copy link

Does it make sense to put in an additional check for whether there's any error handling for RTE3 anywhere in the project? If there aren't, then isn't that a strong indication that the use of Return was incorrect?

I think that rules should be as simple as possible.
"If there is a Return, but there is no Gosub then the Return means something else..."
That can cause confusion.
I think enough is to have the same keyword ("Return") with already two completely different meanings (Return from Gosub and Return a value in a procedure) IMO.

@EduardoVB
Copy link

BTW, I think I recall there was a comment from Wayne about the new Return keyword couldn't be allowed if there was a Gosub in the same procedure (not sure if I'm right because I can't find that message now here, may be it was in Vbforums).

Since the new Return keyword needs to be followed by an expression, like Return thisValue and the old Return for Gosub can't be followed by an expresion (it causes a compilation error), I think both Return keywords can be allowed in the same procedure taking that into account.

@bclothier
Copy link
Collaborator Author

Pretty much the same as in .Net -- the exception is handled by parent try/catch block a.k.a "On Error GoTo Label" (if any) or if unhandled at all a run-time msgbox is displayed with the unhandled error description before aborting the process.

My point is that if the RTE3 goes unhandled (whether specifically or generally under catch-all handler), then we know that the program would have crashed and therefore was not written correctly in the first place. Or as C guys like to call it, "undefined behavior". 😄 Somewhere else we discussed that backward compatibility can stop at the point where code starts crashing. If unhandled RTE3 results in an abort where the execution is terminated, that's also where the backward compatibility can stop. Unfortunately to support that, this requires code path analysis to statically determine whether RTE3 is possible. That can get complicated when branching or jumping are involved.

Since the new Return keyword needs to be followed by an expression, like Return thisValue and the old Return for Gosub can't be followed by an expresion (it causes a compilation error), I think both Return keywords can be allowed in the same procedure taking that into account.

While this is technically possible, I think the 2 different uses within the same procedure would be quite jarring and confusing. Following the principle of least astonishment, I'd prefer that the Return had single behavior.

The irony here is that the GoSub...Return misfeature exists solely as a backward compatibility feature to port pre-VBx code into VBx but even so it failed abysmally in this regards because in the BASIC, you didn't have procedures at all. The GoSub...Return were basically a bastardized form of procedures. However, in VBx, you cannot write code outside of procedure blocks. Therefore to port the BASIC code, you had to least slap on a Private Sub BigOldUglyProcedure() and End Sub. Not quite 100% backward compatibility. Unfortunately because it exists, it has came to be used in other ways beyond what it originally was used for within BASIC.

@EduardoVB
Copy link

About the first point, you can write this legal code in VB6:

Private Sub Form_Load()
    On Error Resume Next
    Return
    Label1.Caption = Err.Description
End Sub

Why someone would do that I don't know, but it is possible.

About GoSub, you must be right about its origin (and how it came into VB6), but I do not agree with most people that think that it is intrinsically evil (or something like that).
I serves sometimes the function of an anonymous function within a procedure. When you have a lot of local variables and realize that you need to repeat some action that need those variables, it is very cumbersome to convert the code to put that part in another procedure, and it is handy just to add a GoSub... and keep working.
I try to avoid it, but I'm pragmatic and not idealistic, I did it sometimes and have no problem defending it.

@bclothier
Copy link
Collaborator Author

When you have a lot of local variables and realize that you need to repeat some action that need those variables, it is very cumbersome to convert the code to put that part in another procedure, and it is handy just to add a GoSub... and keep working.

See, that's why we want to make sure it's easy to refactor the code and the ability to refactor code is heavily dependent on whether the refactoring can be done without changing the behavior with the code. With GoTo and its associated cousins, it is nearly impossible to refactor without changing the behavior.

In a modern language & IDE, this would be easily done by doing a "Extract Method" refactoring which also take care of moving variables or converting into a parameter. But with a GoSub in the picture, additional analysis is needed to ensure that code can be safely refactored. The consequence of this is that we have a codebase that is impossible to fix, much less maintain let alone enhance. That's A Very Bad Thing™ and it makes the technical debt much more expensive to pay off.

We do not want tB to fall in the same position that VB* codebase has where people look at it, and say "Forget it. Let's just rewrite the whole thing from scratch" which is actually another mistake. The desire to rewrite rather than refactor will stem from the fact that you can't reliably refactor code with elements similar to GoTo and its cousins (not to mention other misfeatures from BASIC days). A language that is easy to refactor makes it much easier to be pragmatic and not get mixed up in "oh, I should not have written it like that. I guess I'm stuck with it for next 20 years." tarpits that VBx codebase invariably falls into.

@EduardoVB
Copy link

I see your point on refactoring, and I agree.

Well, GoSub seems at first glance easier to refactor, you need to look for the last Return and that's the End Sub, and others Returns in the middle are Exit Sub.

GoTo seems impossible, a GoTo can jump anywhere.

I also defend the occasional use of GoTos in VB6, because sometimes you need, for example, to do this:
You need to clear up some things before leaving the procedure, some API handles or whatever. And you have several Exit Function in the middle, for example because some error occurred.
What I sometimes did, instead of repeating the clear-up code, is to replace all those Exit Sub with GoTo Exit_Sub, like this:

Private Sub Foo()
    ' Set some handles or whatever that need to be destroyed after use
    
    ' code
    
    If Condition1 Then
        GoTo Exit_Sub
    End If
    
    ' code
    
    If Condition2 Then
        GoTo Exit_Sub
    End If
    
    ' code
    
    If Condition3 Then
        GoTo Exit_Sub
    End If
    
    ' code
Exit_Sub

 ' Clear handles
    
End Sub

@WaynePhillipsEA
Copy link
Contributor

For continuity reasons I believe we should strive to get this Return syntax allowed in Subs. The runtime error of the old syntax is a consideration, but this is largely mitigated by us simply disabling the new Return syntax when the legacy Gosub is encountered within a procedure.

That just leaves the nonsensical use of Return, with it throwing a runtime error in the old syntax without any Gosub appearing in the procedure... and I feel that supporting that 'feature' would be taking the 100% compatibility goal too literally here, without considering the bigger picture.

@mburns08109
Copy link

LoL

About GoSub, you must be right about its origin (and how it came into VB6), but I do not agree with most people that think that it is intrinsically evil (or something like that).

Well, if you or I as an adult, were to pick up a baby rattle and start playing with it, giggling like a pre-toddler again, I wouldn't call that intrinsically evil either; but any reasonable observer of such behavior would quickly conclude that there is some sort of significant maturity issue at play with us for doing so.

Supreme Court Justice Robert H. Jackson first used the "suicide pact" phrase in comparison to the US Constitution, and I begin to wonder at this discussion if we're not edging towards being able to use that phrase in comparison to the concept of "100% Backwards Compatibility". Perhaps we should clarify a little on the question of "How much Backwards Compatibility is 'Too Much' backwards Compatibility?" Otherwise we risk condemning tB to re-commit the sins of the past when a significant part of our hope for this was some more progress towards the future? ...to say nothing of the relative effort involved in getting every jot and tiddle "just like before" -vs- the opportunity cost of not making other important progress.

@DaveInCaz
Copy link

@mburns08109 I thought the point was so that pure VB6 code would compile and run in tB as-is. To me (someone who maintains a lot of old VB6 code - and there are many of us) that would be extremely valuable.

From other posts I had the impression that tB would also offer some set of selectable options to gradually move away from the legacy limitations of VB6. A separate mode where gosub / return is illegal or whatever.

@mburns08109
Copy link

mburns08109 commented Nov 19, 2021

I was kind-of hinting more towards what Ben was saying about refactorability -vs- trash-and-redo, and hint that if "backwards compatibility" questions don't get examined in the light of what he was saying, then perhaps we're edging more towards the "suicide pact" side of things than we should really be comfortable with (as opposed to offering any concrete advice about whether gosub-less Return statements should be either supported, treated with the same runtime error that VB6/VBA do, or treated as compiler errors).

I guess what I'm trying to say is: when is it okay to leave some old things on the trash heap of history -vs- assuring that every memory leak is repeated for the sake of compatibility? Could this gosub-less return topic be one of those issues?

@wqweto
Copy link

wqweto commented Nov 19, 2021

Could this gosub-less return topic be one of those issues?

Only if it's leaking memory :-)) Are we repeating a memory leak here indeed or I'm missing the hyperbole?

Btw, something innocuous as adding new keywords to the language breaks compatibility too.

For instance TB added Continue statement as a convenience in loops but it broke VBx code like GoTo Continue which cannot be compiled by TB as is, unless the compiler is switched to some compatibility mode.

@EduardoVB
Copy link

Well, if you or I as an adult, were to pick up a baby rattle and start playing with it, giggling like a pre-toddler again, I wouldn't call that intrinsically evil either; but any reasonable observer of such behavior would quickly conclude that there is some sort of significant maturity issue at play with us for doing so.

There are two ways of following rules 1) when you understand the reason behind the rule, 2) when you don't understand it.

When it comes to God's rules, you need to follow them for your own good, better if you understand, and if you don't understand, better for you to follow the rules anyway because God is an Authority and He knows why even if you don't (in your tiny human mind).

When it comes to men rules, there are laws, you are obliged to obey the laws whether you understand the reasons behind them or not. And the government is also an authority that enforce the laws.

When it comes to other rules, that more than rules are common beliefs, or traditions, you can also obey 1) if you understand, and 2) if you don't understand why that rule is (and perhaps where it comes from).

Many people believe something because many other people does, or prominent people say so.

When it comes to "never, ever using GoTo", what do you think?
Options:

  1. You think it is a sin.
  2. You think it is against the law.
  3. You think someone said so (perhaps a prominent person) and many other believed. But you agree with that anyway because you understand the matter.
  4. Same as 2) but you don't understand the matter.

When you understand the issue, you can agree or disagree (hence to obey or not obey, or just obey in part, according to what you understand that is more convenient), but if you don't understand it, you can only obey or disobey by blind faith (and perhaps criticize other that don't follow the rule that you follow -but don't understand-)

If you think that someone using occasionally GoTo in VB6 (I'm not talking about other languages) is like playing with a baby rattle, think what you want.
It is out of everyone else's control what you think.

@mburns08109
Copy link

It's not really what I think that's the issue that should concern you (which is partly why I didn't voice a direct opinion on the options). It's the question of how you our your successors will view the code when looking at it from a future perspective - especially if, as Ben hinted at, the question of the day at that point will be "refit/repair/refactor this?" - or "nuke it from orbit and start over?" (the extra Hyperbole is included free of charge.)

@EduardoVB
Copy link

It's not really what I think that's the issue that should concern you (which is partly why I didn't voice a direct opinion on the options). It's the question of how you our your successors will view the code when looking at it from a future perspective - especially if, as Ben hinted at, the question of the day at that point will be "refit/repair/refactor this?" - or "nuke it from orbit and start over?" (the extra Hyperbole is included free of charge.)

There are already many millions of VB6 code lines already, you cannot change that fact.

If tB adds anonymous functions, GoSub, and possibly GoTo too, won't be justified anymore and nobody should use that in new code.

When the programmers writes a GoSub or GoTo, tB will issue a warning, that should be enough.
And if someone wants to use GoTo anyway, what's your (or anybody else) problem?
It is his choice, it does nothing to you.

It looks to me like the vaccinated complaining because some others don't want to take the vaccine.

@mwolfe02
Copy link

Perhaps we should clarify a little on the question of "How much Backwards Compatibility is 'Too Much' backwards Compatibility?"

When you're trying to provide 100% backward compatibility, then there is no such thing as "Too Much" backwards compatibility.

I understand the point you are trying to make, but the difference between 100% compatible and 99% compatible is WAY MORE than 1% when it comes to developer migration effort. Microsoft understood this when they created Excel. Anything that prevents existing VBx code from running in twinBASIC will cause many developers that are tB-curious to simply throw up their hands and think, "just another failed VBx clone."

When you're trying to gain market share as an upstart, you need a product that Just Works™.

Now, I think it's reasonable to stop at 99.999% compatibility if the final 0.001% amounts to actual implementation bugs in VBx AND there is a project-wide "Quirks Mode" flag that will recreate those old bugs for the developers that relied on them for whatever reason. But GoSub does not fall into that category.

@mwolfe02
Copy link

The way to encourage better programming practices moving forward is to provide an opt-in mode that disables language features like GoSub. The community can then encourage usage of that as a best practice the way that Option Explicit is encouraged as a best practice in VBx now.

@bclothier bclothier transferred this issue from twinbasic/twinbasic Jul 27, 2022
@mansellan
Copy link

For continuity reasons I believe we should strive to get this Return syntax allowed in Subs. The runtime error of the old syntax is a consideration, but this is largely mitigated by us simply disabling the new Return syntax when the legacy Gosub is encountered within a procedure.

That just leaves the nonsensical use of Return, with it throwing a runtime error in the old syntax without any Gosub appearing in the procedure... and I feel that supporting that 'feature' would be taking the 100% compatibility goal too literally here, without considering the bigger picture.

Bump.

A Return outside of a Gosub block, then catching the runtime error that is certain to result is... bizarre?

I feel like this could easily be accommodated by an "absolute" project switch for legacy compatibility. Almost nobody would be caught by this, and a project switch could keep the 100% promise.

@bclothier
Copy link
Collaborator Author

Just wanted to put out a different approach:

Private Sub Foo()
  Return Default
End Sub

Private Function Bar() As Long
  Return Default ' Returns 0
End Function

Private Function Fizz() As String
  Return Default 'Returns vbNullString
End Function

Private Function Buzz() As Variant
  Return Default 'Returns vbEmpty
End Function

The Return Default is a illegal statement in VBx so there is no backward compatibility issues, and has the added bonus of simplifying the refactoring (e.g. changing a Sub into a Function and still getting a default value).

@fafalone
Copy link
Collaborator

Need to be really conservative with taking over keywords... Default isn't exactly an uncommon name so you might be creating issues with Function Default() for a lot of people.

@bclothier
Copy link
Collaborator Author

Hmm, Return Default is a syntax error so the problem would only exist in a tB project, rather than importing from legacy VBx projects. A warning could be issued if there's an ambiguity, though one might question whether it should be necessary in the first place. If we don't want the potential of an ambiguity, Return As Default would work, too if a bit more wordy.

@fafalone
Copy link
Collaborator

fafalone commented May 2, 2023

But it wouldn't be a result in Subs, and nobody is talking about removing Function Foo(): Foo = x

Function foo() As Long
        foo = 2
        Debug.Print bar(foo, 2)
    End Function
    Function bar(x As Long, y As Long) As Long
        Return x + y
    End Function

This is already legal and prints the expected 4.

@FullValueRider
Copy link

@fafalone Its not a third way. There would be no need for 'return Value' syntax so that new usgage in twinBasic could be retired before V1, a perfectly legitimate change. The proposals I made were to try and resolve the issue around the use of return in a Sub.

The = is needed as I envisage result (or whatever name is used) being an alias for the Method name variable.

@bclothier
Copy link
Collaborator Author

bclothier commented May 2, 2023

Oh, my.

This thread is sprawling far beyond what should be the original scope. Some of the points sounds like they are a separate proposal, and probably should be dealt in their own issue, and other points seems to be taking a different approach altogether.

Let's recap:

VBx currently:

  • Require that the return value be assigned to the implicit variable of the same name of the function it is defined.
  • The assignment does not immediately leave the routine. A separate Exit Function or Exit Sub would have to follow if one wants to leave the routine.
  • If there is a cleanup subroutine that must be performed, one has to be careful to use GoTo <cleanup label> instead of Exit <procedure>.
  • Return is a legal keyword, and the following code is legal and will compile but fail with a runtime error 3 which gets swallowed up by the error handler.
Private Sub Derp()
  On Error GoTo Duh
  
  Return
  
  Exit Sub

Duh:
  Debug.Print "Ha-ha!"
End Sub

While I seriously doubt there's anyone trapping for run-time error 3 Return without GoSub, I hope that shows how we might have a VB codebase that seems to work because it's swallowing the run-time error 3 and nobody has noticed.

tB currently:

  • Support the vB syntax for backward compatibility
  • In addition, provides a Return <value> for a function which both assigns a value and leaves the function immediately.
  • Does not allow Return (without a value) in a Sub for the backward compatibility with GoSub...Return syntax which is also legal.

Other languages tend to follow the same convention where:

  • Return usually leave the method immediately. For "sub" (aka void method), return without value is used to leave the method.
  • ...unless it is inside a try...finally block. In this case, the finally block is executed, thus guaranteeing that the code within the finally always execute regardless whether a value was returned or an exception was thrown.

tB currently doesn't have a try...catch...finally idiom but it will be provided via vbWatchDog once it is made available in tB. This is why error handling is relevant to the discussion because Return should not bypass the finally block and make it easier for programmer to avoid accidentally circumventing this by using Exit <procedure> instead of GoTo <label>. Changing the behavior of Exit <procedure> to always execute the finally block is potentially more problematic and may break backward compatibility in subtle ways even if we used a new keyword for the finally block. Using Return with the expectation that it will always execute the finally block is a preferable and safer way of enhancing the language in both how it handles the return value and error handling.

The ideal and simplest outcome would be to just allow Return as a legal syntax for use in Sub. This might be accomplished by either 1) disallowing GoSub...Return or 2) no longer raising runtime error 3 but instead exiting the procedure, which is a change in the behavior (e.g. the sample code shown above will no longer print Ha-ha!; if there's an OERN it might be even more drastic). I personally would rather prefer having simple Return in a Sub.

The other idea of Return Default (or a variant thereof) was meant to be an alternative just in case simple Return is not practical for some reasons that I might be unaware of.

Using Exit Sub instead of Return means there's two different ways of doing the same and runs into the problem of cleanup as I described above. By having a Return or variant thereof in Sub, we can then expect the same behavior with try...finally when dealing with Return.

@wqweto
Copy link

wqweto commented May 2, 2023

Now I figured out something which was not apparent immediately. With Return Default syntax in a sub the compiler can disambiguate from original Return statement while having Return Default working in a function is just a non-asked byproduct.

I'm all for having plain Return working in a sub without GoSub statements and disallowing it in a sub with GoSub so that bad style in past can have some consequences along the road :-))

@Greedquest
Copy link

Greedquest commented May 2, 2023

The only fully back compatible solution I can think of is to have the Return keyword to exit a sub (/ function, for consistency) only enabled in .twin files, and in .bas/.cls files etc the error 3 is raised.

@bclothier
Copy link
Collaborator Author

bclothier commented May 2, 2023

Hmm, that entails that for consistency, the .bas/.cls files wouldn't have other tB features (c.f. #13 ). I don't think that's the case currently. I'm not sure how I feel about the idea of having the file extension enabling/disabling the language features.

@Greedquest
Copy link

Have we considered using Exit instead of Return

Sub Foo()
    Exit
    Debug.Print "not called"
End Sub
Function Bar() As String
    Exit "return value"
    Debug.Print "not called"
End Sub

Or some combination with With, As e.g.:

Function Bar() As String
    Exit With "return value"
    Debug.Print "not called"
End Sub

@Greedquest

This comment was marked as off-topic.

@Greedquest
Copy link

Greedquest commented May 2, 2023

Changing the behavior of Exit <procedure> to always execute the finally block is potentially more problematic and may break backward compatibility in subtle ways even if we used a new keyword for the finally block

I disagree. If we use vbWatchdog as-is then yes. If instead a new Try... Finally...End Try block is introduced then no VBx code will already be written in that scope, since it would be a syntax error, so we can define Exit <procedure> to call finally without breaking any backwards compatibility I believe.

@bclothier
Copy link
Collaborator Author

Have we considered using Exit instead of Return

This would seem to avoid the compatibility issues though TBH I'm ambivalent about overloading the word Exit to mean to return a value which would make the language even more odd compared to other languages that uses the return keyword.

Also, from my POV, I find it mildly annoying that if you had a situation where you had to alter the procedure from Sub to Function (or vice versa), the End <procedure> was automatically updated but not the Exit <procedure>. @fafalone has pointed out that it could be something that IDE could make it easier to do. Then there's the fact that we have multiple ways of doing same thing (e.g. Exit Sub or Exit for subs, Return <value> for functions) which seems to me unnecessary distinction. I find it easier when I can use Return everywhere without going "oh, that's a sub, I should be using Exit" and backspace then re-type.

If we were to implement Exit <value> or Exit With <value> with corresponding Exit for subs, I can see how that might lead to subtle logic errors entering Exit Function when it should have been Exit With <value> or vice versa, which means more bugs and therefore a frustrating language to use. I think we want to avoid that, too.

I disagree. If we use vbWatchdog as-is then yes. If instead a new Try... Finally...End Try block is introduced then no VBx code will already be written in that scope, since it would be a syntax error, so we can define Exit <procedure> to call finally without breaking any backwards compatibility I believe.

Just to point this out, we have an open issue on Try/Finally as a keyword ( #61 ) which also hits on the issues we already have to deal with the different keywords & strategies for leaving a routine and error handling which as I explained over there is ironically errorprone.

Anyway, to your point; I need to ask - is it a good idea to alter the behavior of the Exit <procedure> so that it always calls the Finally block? Personally, I think yes and for everybody's sanity, it ought to but the backward compatibility may be harmed by this subtle change in the behavior. Using Return at least makes this an explicit opt-in and we can then have IDE encourage the use of Return over Exit <procedure> as a hint to help promote good coding practices, rather than forcing them to choose a legacy mode because they can't be sure that it will work as-is when they import their original VBx codebase.

@bclothier bclothier mentioned this issue May 3, 2023
@fafalone
Copy link
Collaborator

fafalone commented May 3, 2023

I'd be against modifying Exit <method> to jump to what's basically a label. Keywords that have different meanings in different contexts are a nightmare for language clarity, and are beginner-hostile substantial increases in learning curve. It's been a while since I used a language with this syntax, but isn't the traditional solution for this Exit Try? That's both far more clear, consistent with all the existing VB Exit syntax like Exit For or Exit Do, and has no impact on backwards compatibility at all as it's not legal syntax in VBx.

@bclothier
Copy link
Collaborator Author

I think we're talking about different things.

Consider this hypothetical pseudo-code:

Private Sub Fizz()
  Try
    OpenHandle
    Exit Sub
  Finally
    CloseHandle
  End Try
End Sub

The question is if there's a Exit Sub inside the Try block, should it execute the Finally block? In my opinion, yes it should. Otherwise, we destroy the value of the try...catch...finally block. However, I'm uncertain if there's a problem with breaking the backward compatibility if we change that behavior. Furthermore, what I dislike is that the behavior changes for the Exit <procedure> just because it's inside a try...finally block versus being not inside one. Being able to use Return and thus having a consistent behavior (e.g. being assured that Finally block will always execute even if we leave the routine early) and having IDE highlight a good coding practice seems preferable than overloading the Exit keyword and giving it new meaning that might be not as apparent.

@fafalone
Copy link
Collaborator

fafalone commented May 3, 2023

We're not talking about different things, I'm aware you meant if it was inside the Try block.

That's what I meant-- you're changing the meaning of Exit <method> based on where it is, which brings all the downsides I mentioned. If you want to get out of the Try block while executing Finally, you should need to use Exit Try or perhaps a new keyword after Exit could be added, like Exit Sub With Finally only more concise.

I'd say that yes, it's breaking backwards compatibility, because you're changing the meaning of existing code simply by virtue of someone wanting to add a Try block. I don't understand your statement that to not exit the procedure immediately is altering the behavior... there's no scenario in VBx where Exit <procedure> does anything other than exit the procedure immediately, but here you're proposing that it should.

This doesn't destroy the value of the Try block at all, because nothing stops someone from not using Exit <procedure> and instead using the proper Exit Try. I'd think if someone is going out of their way to add a Try block, they'd want Finally to execute so wouldn't use syntax that bypassed it. Being allowed to use syntax that bypasses it doesn't destroy the value.

@bclothier
Copy link
Collaborator Author

My point is that when inside a Try block, it should not be possible to leave the block without executing the Finally block where present. With Exit <procedure> as-is, that would bypass the Finally execution, which makes it superficial and put us back to square one WRT having to remember to use right syntax in right place which I think is not user-friendly as I explained in the linked issue #61 . Requiring using Return and disallowing the use of Exit <procedure> inside the Try block would at least avoid that problem without changing the behavior of the Exit <procedure> or alternatively do an Exit Try, then Exit <procedure> outside the block.

Note that the same problem would exist with GoTo <label> which allows us to leave any other blocks which feels totally wrong to me. The 90% of the value with the Try block is being guaranteed that the Finally will run, errors or no errors. Otherwise, it's just a keyword posing as a label.

@fafalone
Copy link
Collaborator

fafalone commented May 3, 2023

I really can't see how having to figure out that Exit <procedure> takes on an entirely new meaning if you use it inside a Try block makes the syntax less difficult to understand. IMHO it's just not user-friendly to have context-dependent alternative meanings for "Exit" like "If Exit is within Try, Exit doesn't mean exit, Exit means GoTo Finally, Then Exit".

I think the value in Try is in allowing the user to have Finally always run, error or no error. I don't think the value of forcing them into it by breaking backwards compatibility and creating context-dependent alternative behavior for long-standing, well known core language syntax outweighs the substantial drawbacks of that.

Mind you I would, as mentioned above, support a different Exit syntax that would execute Finally before exiting-- that's the best of all worlds. Allows opting into the benefits without breaking things to force it.

@FullValueRider
Copy link

I can't see the point in arguing about try/catch in the context of a return syntax. In VBA, vbWatchdog was constrained by the need to work within existing VBA code. With twinBasic, try/catch/finally syntax can be added as new twinBasic syntax. The only constraints on such an addition being any potental conflicts between code implementing vbWatchdog try/catch and any new try catch syntax.

@Greedquest
Copy link

Greedquest commented May 3, 2023

@bclothier

Anyway, to your point; I need to ask - is it a good idea to alter the behavior of the Exit so that it always calls the Finally block? Personally, I think yes and for everybody's sanity, it ought to but the backward compatibility may be harmed by this subtle change in the behavior.

@fafalone

[Using Exit in a try block would be] breaking backwards compatibility and creating context-dependent alternative behavior for long-standing, well known core language syntax outweighs the substantial drawbacks of that.

Apologies if I'm missing something here or misinterpreting. I'm confused by the idea of calling this "altering the behavior" of Exit or it posing a risk to backwards compatibility. Right now "Exit Sub" or "Exit Function" to me means not just jump out, there is also clean-up of COM objects calling Class_Terminate and probably clearing memory of variables.

The behaviour of "Exit " is not defined inside a try-catch-finally block because it doesn't exist in VBA yet, so we can't be "changing" the behaviour and breaking backwards compatibility, the behaviour is yet to be defined. And having Exit <Procedure> mean "Exit and perform any clean-up; such as memory, Class_Terminate and/or Finally blocks" is very unsurprising and the precedent from all other languages with early return syntax and finally blocks.

There is no backwards compatibility to break because this:

Function Foo()
     Try
         Exit Function
     Finally
          ' Clean up before exit
     End Try
Exit Function

...is not valid code yet. So there is no backwards compatibility to consider. There is "expected behaviour" to consider, and designing features to feel idiomatic in VBA and unsurprising. But you're not going to break any existing code by defining a finally block to run on an early Return or Exit, because no code exists yet to break. Or am I missing something?

@bclothier is right in that the value of a finally block is that it always runs regardless of what happens inside the try block. To free resources or close context managers.


note I'm ignoring vbWatchdog which I believe uses labels rather than keywords to mark the regions of code for a finally block, although I haven't used it myself. I'm assuming keywords will exist

@DaveInCaz
Copy link

@Greedquest

And having "Exit " mean "Exit and perform any clean-up, either memory, Class_Terminate or Finally blocks" is very unsurprising and the precedent from all other languages with early return syntax and finally blocks.

FWIW I had the same instinct, everyone has expectations for Finally from other languages. Would be non-controversial (IMO) to do the same, and potentially confusing or error-prone not to.

@bclothier
Copy link
Collaborator Author

Apologies if I'm missing something here or misinterpreting. I'm confused by the idea of calling this "altering the behavior" of Exit or it posing a risk to backwards compatibility. Right now "Exit Sub" or "Exit Function" to me means not just jump out, there is also clean-up of COM objects calling Class_Terminate and probably clearing memory of variables.

Those are handled by the compiler whereas the Finally points to user-defined code. In VBx code, you'd have to use GoTo <label> or Resume ` to perform user-defined code.

The point is that if Exit <procedure> is allowed inside a Try block, the behavior will be different - it must be in order for Finally to be guaranteed to be executed. This is a change in the expectation because in VBx code, one could (ab)use Exit <procedure> to leave a routine without even calling the cleanup routine. Personally, I think that's not good coding but it is legal code and I've seen people (ab)use that feature as to avoid doing additional work in the cleanup. Example:

Public Sub Foo()
  On Error GoTo ErrHandler

  If Bar() Then Exit Sub
  
  'Do Foo

  If Foo_d Then 
   GoTo ExitProc
  End If

  'Foo some more

ExitProc:
  'Unfoo
  Exit Sub
ErrHandler:
  Debug.Print "Bonk!"
  Resume ExitProc
End Sub

Now suppose we then update the procedure and put in a Try:

Public Sub Foo()
  Try
    If Bar() Then Exit Sub
    
    'Do Foo

    If Foo_d Then 
     GoTo ExitProc
    End If

    'Foo some more
  Catch
    Debug.Print "Bonk!"
  Finally
    'Unfoo
    Exit Sub
  End Try
ExitProc:
End Sub

What's the behavior of Exit Sub and GoTo ExitProc?

You are correct that right now, the behavior of Exit <procedure> inside a Try block is undefined but the concern is keeping thing consistent so that users are not confused or surprised by the changes in the behavior. That's going to be important when they start updating their existing VB6 codebase in the ways I illustrated above.

In this view, I think I'm arguing that Exit <procedure> and GoTo <label> (possibly all GoTo and GoSub) probably should be disallowed and be a compiler error inside the Try block. As I said earlier, we can have IDE hint at using Return and Try block as the preferred syntax to help promote good coding practices. This also argues against the idea of using just Exit or Exit With because it'll have the problem of being too similar to the Exit <procedure> but not quite that it's off-putting. It might be me but I gave up on VB.NET exactly for that reason -- there were too many "this is just like VBA! No, wait.... it's not." mindtraps that it was difficult to be productive in VB.NET. I'd rather that tB avoid that kind of mindtrap.

note I'm ignoring vbWatchdog which I believe uses labels rather than keywords to mark the regions of code for a finally block, although I haven't used it myself. I'm assuming keywords will exist

Just FYI. In VBx's vbWatchDog, the Exit Sub will not call the finally block. If one wants the Finally to always run, one has to use ErrEx.DoFinally instead of Exit <procedure>:

Private Sub CallIt()
    DoIt
End Sub

Private Sub DoIt()
    Exit Sub
ErrEx.Finally
    Debug.Print "Finally!" 'not printed
End Sub

I should note that I've made boneheaded mistakes like this:

Private Sub Derp()
  Debug.Print "Derping"
ErrEx.DoFinally 'Oops! Should be ErrEx.Finally! 
  Debug.Print "Derped" 'not printed
End Sub

@DaveInCaz
Copy link

People only have to convert their VBx code to tB once, but then they have to maintain it forever after. It seems better to me to err on the side of having a well behaved language feature vs. making the transition to tB a little easier. Otherwise the programmer will pay for that many times over instead of just once when they update to tB.

Personally I would expect that the Goto would not circumvent the Finally. Either that, or Goto within a Try to a label outside the Try should just be disallowed; but that seems less useful.


@bclothier thought experiment... If hypothetically there were an IDE refactoring that would transform from your first example to the second... that could indeed cause some surprises. Perhaps having the refactoring add a warning and/or comment about how it affects goto / exit would be a good idea in that case. The VS2010 VB migration assistant would very liberally add warnings like that. (E.g., for array bounds change from 1- to 0- based).

@bclothier
Copy link
Collaborator Author

This was supposed to be an example of refactoring by hand. If IDE were to provide a refactor, it'd be probably smarter than that and substitute in Return or Return <value> and removing all GoTo or Exit <procedure> as well as any labels so that there is no ambiguity over how the code leaves the procedure and is guaranteed to always leave the procedure in a consistent and well-defined manner. I just don't think it's good idea to mix the paradigms of gotos/exits with try/catch/finally.

The more recent languages avoid this problem by simply never giving the equivalent of GoTo in its syntax so that issue is a moot point in those languages; it's impossible to jump out in an uncontrolled manner. Because tB has the commitment to backward compatibility, this is not a moot point.

@fafalone
Copy link
Collaborator

fafalone commented May 3, 2023

Apologies if I'm missing something here or misinterpreting. I'm confused by the idea of calling this "altering the behavior" of Exit or it posing a risk to backwards compatibility. Right now "Exit Sub" or "Exit Function" to me means not just jump out, there is also clean-up of COM objects calling Class_Terminate and probably clearing memory of variables.

I don't think hidden non-user code is a reasonable comparison here. VB programmers don't think about, or consider, hidden codepoints, whereas we very much expect consistent behavior in user code flow. Hidden code isn't subject to all the random errors and bad practices of user code; there's no world where everyone writes flawless finally blocks that don't trigger errors.

The behaviour of "Exit " is not defined inside a try-catch-finally block because it doesn't exist in VBA yet, so we can't be "changing" the behaviour and breaking backwards compatibility, the behaviour is yet to be defined. And having Exit mean "Exit and perform any clean-up; such as memory, Class_Terminate and/or Finally blocks" is very unsurprising and the precedent from all other languages with early return syntax and finally blocks.

This also seems entirely unreasonable. Code doesn't cease to the VB6 language just because you've put it inside another type of code flow control block. This is like saying we can throw all the rules of the language right out the window because of some unrelated label. It's absolutely breaking backwards compatibility to change the meaning of existing code, end of story. It doesn't matter that it's a Try block any more than an If block, VBx code needs to execute as defined by VBx code in order to be compatible. If you want to argue for breaking backwards compatibility, ok, but let's not pretend this isn't what we're talking about.

@bclothier is right in that the value of a finally block is that it always runs regardless of what happens inside the try block. To free resources or close context managers.

And I for one certainly look forward to taking advantage of that value. I don't care to have it forced upon me by breaking the rules for existing syntax. I'll personally, in forseeable cases, not be using Exit <procedure> inside my Try blocks, because I do want Finally to run. But if, for some reason, I did want to break out without running Finally, I would not want to be in a situation where the language has forbidden that by altering the behavior of existing syntax.

If we are going to handcuff users, I much prefer Ben's alternative of disallowing Exit <procedure> inside a Try block.

People only have to convert their VBx code to tB once, but then they have to maintain it forever after. It seems better to me to err on the side of having a well behaved language feature vs. making the transition to tB a little easier. Otherwise the programmer will pay for that many times over instead of just once when they update to tB.

This was .NET's philosophy; I'm using VBx and tB because I prefer that language, despite it's shortcomings, over "better" languages, for the uses where it's practical (and even somewhat impractical, if it's possible).

@DaveInCaz
Copy link

... It's absolutely breaking backwards compatibility to change the meaning of existing code, end of story.

That seems to be 100% accurate - the essential definition of what "backwards compatibility" probably means to most people.

But once you alter your code deliberately, why would it be expected to behave the same way? Alteration could be updating it to use a tB language feature that did not exist in VBx, or anything else. I don't think this is the sense in which "backwards compatibility" really applies.

@bclothier
Copy link
Collaborator Author

But once you alter your code deliberately, why would it be expected to behave the same way?

Keep in mind that typically in a migration project, it's seldom all at once. More likely, they'll want to keep everything as-is and update only this and that and gradually update stuff as they go, which helps amortize the development costs. That's the biggest win with the backward compatibility promise of tB that other languages can't even begin to fulfill.

@mansellan
Copy link

mansellan commented Jun 30, 2023

Can't we just generalise Exit, so that it doesn't need to be told what it's exiting (other than it not being a loop)? Presumably for functions and property-gets, it just returns the default?

How about Exit Procedure, which is equivalent to Exit [Sub|Function|Property], without having to be explicit?

@bclothier
Copy link
Collaborator Author

Don't think Exit is quite basic-y when you consider the block structures:

Public Sub Foo()
  Dim i As Long
  If True Then
    For i = 0 To 0
      Exit For
    Next
    Do
      Exit Do
    Loop
  Else
    Exit If 'Not a real keyword, included for illustration
  End If
  Exit Sub
End Sub

Compare with a C-family pseudocode:

void foo() {
  if(true) {
    for(var i = 0; i < 1; i++) {
      break;
    }
    while(true) {
      break;
    }
  } else {
    break;
  }
}

The appeal of BASIC is that you can at least identify what type of blocks you're closing. A generalized Exit & End would start to resemble more like a verbose form of C-family language and probably even more confusing:

Public Sub Foo()
  Dim i As Long
  If True Then
    For i = 0 To 0
      Exit
    Next
    Do
      Exit
    Loop
  Else
    Exit
  End
  Exit
End

So yeah, I don't think a generalized form will work very well.

Exit Procedure would be a possible alternative.

I find it interesting that you cannot break nor continue out of a non-loop structure. You can only return in this case case. tB does have Break and Continue (yay!), but not sure I'd want to overload it in this way.

@mansellan
Copy link

mansellan commented Jun 30, 2023

Sorry, my second paragraph was disconnected from the first. I guess I initially envisaged a general Exit, but then saw the problems you detailed.

At this point, I'm only proposing Exit Procedure, which would be equivalent to Exit Sub, Exit Function, or Exit Property, depending on context.

@mansellan
Copy link

Wait...

How about, in a Sub, you can Return Nothing? If you change a Function to a Sub, you'd have to change the return, but maybe that's a good thing? In that you have to think what that change means? C# does that...

@bclothier
Copy link
Collaborator Author

FWIW, I think it comes close to Return Default proposed earlier in this thread. The thing with Nothing is that it's for an object which isn't always appropriate. Return Empty might be closer.

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

No branches or pull requests