bmx-ng / bcc

A next-generation bcc parser for BlitzMax
zlib License
33 stars 12 forks source link

Using blocks #333

Open HurryStarfish opened 6 years ago

HurryStarfish commented 6 years ago

Now that NG has Finally blocks, I think it would be useful to go one step further and add Using blocks.

Possibly the most important application for Try...Finally blocks is to make sure that some resource that has been allocated is released/deleted/cleaned up again as soon as possible once the program is done using it. For example, when working with a file, first the file is opened before a Try block, then used inside the Try block, and afterwards the Finally block makes sure to close it:

Local file:TStream = OpenFile("path/to/file")
If Not file Then Return
Try
    ' do things with the file
Finally
    file.Close
End Try

A Using block would be a new kind of block tailored to this use case, and would allow simplifying the above code to something like this:

Using Local file:TStream = OpenFile("path/to/file")
    If Not file Then Return
    ' do things with the file
End Using

This would have two main advantages:

  1. It makes the code slightly easier to read and the intent more clear: the file is only supposed to be used within the block. To reinforce this, the file variable is declared in the header of the block and is only visible inside of it, similarly to a loop counter variable, e.g. For Local i:Int = .... This is in contrast to the Try...Finally code, where file is accessible even after having been closed. (To prevent accidentally misuse, assignments to the variable declared in the header of the block should not be allowed. Otherwise, the variable could be overwritten, resulting in the object not being disposed and leaking resources.)
  2. It makes resource disposal standardized: instead of having to explicitly call file.Close (or something else, for a different kind of resource), there will be an interface IDisposable in BRL.Blitz containing a Dispose() method. Anything that implements this interface can be put in a Using block, and when control leaves the block, the Dispose method will be called automatically, in the same way a Finally block would run. That also makes it easier to use an unknown class (like from a module written by someone else) correctly. Iit's relatively easy to see whether a class implements IDisposable or has a Dispose method, compared to having to look for some unknown, arbitrarily named cleanup method which might need to be called.

Instead of having a variable declaration in the block header like in the example above, it should also be possible to only provide an expression. This would be useful when the object only exists to be disposed again, and isn't otherwise of interest inside the block, for example:

Global mutex:TMutex = New TMutex

...

Function DoStuff()
    Using mutex.Lock() ' returns an object whose Dispose method unlocks the mutex again
        ' this is thread safe
    End Using
End Function



Syntax and naming considerations:

Other languages that have this kind of feature are C# (using statement), Java (try with resources) and Python (with statement). I've modeled the above code examples and descriptions after the C# feature for the following reasons: Java's syntax is that of a Try block with a declaration in the header. While that could also work in BlitzMax, the syntax would be more awkward - Try Local x:IDisposable = ... is not an option because a) it would break backwards compatibility (it's already valid syntax with the same meaning as Try; Local x:IDisposable = ...) and because b) it would be entirely unrecognizable as a new feature to someone who doesn't know about it. Try Using x:IDisposable = ... could work, but then we've got a new keyword anyway and might as well give it its own kind of block. This also keeps it separate from Try blocks, which I think is a good thing, because the fact that Java's try-with-resources statement can have catch blocks and a finally block (in addition to the implicit finally block that releases the resources) makes it more complicated than it needs to be, in my opinion. When comparing the other keyword names, between C#'s using and Python's with, I think the former is a slightly better choice: it gets the point across that inside the block specifically is where you are meant to be "using" the object, and nowhere else. As for the interface, Java calls it AutoCloseable and its method close, which is fine for files or streams, but doesn't really fit other things. Unlocking a mutex, for example, doesn't really involve "closing" anything (quite the opposite). Python works a bit differently and uses __enter__ and __exit__ methods, but again, I'd probably prefer the simpler variant. C#'s Dispose isn't ideal either, but it seems an okay general-purpose name, and I can't come up with anything better myself. Maybe someone else here has another good alternative idea for the interface/method/keyword name or syntax?

GWRon commented 6 years ago

It already sounds pretty mich drafted/sketched out. Good thing.

I also liked that little aspect about a generic "clean up" method so you know what to call in 3rd party code/modules.

"dispose" is a verb..so an "action". "CleanUp" is nice but already a combination of two words... Also "clean up" might get mixed up with "tidy up"... and this means removing/reinitializing stuff. "remove" could be also used to remove an entity from a parent...

For all the other stuff I am a bit too unexperiened to be of help.

HurryStarfish commented 6 years ago

"CleanUp" is nice but already a combination of two words

Yeah, though I also think "Dispose" is gramatically icky because the real verb is actually "dispose of". The nice thing about it is that the meaning of the word is generic enough to fit most uses.

Some other words I had considered are:

They've all got their own problems.

woollybah commented 6 years ago

Using sounds fine. As does IDisposable, otherwise I'd opt for something like ICloseable and have it call Close(), but it's all much the same. But I think if we are adding a shiny new thing, we may as well have it require a new method - rather than a typically existing one such as Close().

Using should also allow for a comma-separated list of these disposable variables - as per C#.

To prevent accidentally misuse, assignments to the variable declared in the header of the block should not be allowed

So, in essence the variable becomes Final or essentially Final. So, if we are going to add support for the concept of Final, we may want to consider allowing Final to be used in general - for variables and method arguments...

I prefer Final rather than Const - as you might use in C/C++.

Function test(Final x:Int, Final y:Int)
    x :+ 1 ' error : 'X' is Final.
End Function

Final Local obj:TThing = New TThing

.... or something like that...

GWRon commented 6 years ago

Chances to have "Close()" defined in ones code are way higher than for "Dispose()". The former is just more often used in computer terms. For me it is the opposite of "open". And "delete" the opposite of "new" (in coding terms of course).

Aside of that you could call ot Xdhebg meanwhile as it is just a matter of changing some bits in the compiler source then.

HurryStarfish commented 6 years ago

Using should also allow for a comma-separated list of these disposable variables - as per C#.

We could allow Using to be followed by either a comma-separated list of expressions, or a Local decl list, or even both. So in its most complex form, it could be something like

Using someExpression, someOtherExpression, Local x:T1 = New T1, y:T2 = x.Blah()
    ...
End Using

Chances to have "Close()" defined in ones code are way higher than for "Dispose()".

For TStream, this would actually be nice, since Close already exists and does exactly what Dispose should. But as Brucey mentioned, in some other class somewhere, there might already be some other method called "Close" which does something else, in which case, the names might collide.


So, if we are going to add support for the concept of Final, we may want to consider allowing Final to be used in general - for variables and method arguments...

If we add something like this, I'd say it would be better suited as an additional modifier than a replacement of the declaration keyword (Local/Global/...). Personally though, I am not really a fan of allowing "final" local variables or parameters. I've found that in Java, in my opinion, the ability to do this increases confusion more often than clarity. The reason is that in smaller sized, well designed functions, "final" is mostly unnecessary because the scope and use of variables is obvious at a glance. If a function is complicated and hard to understand enough to make it useful however, making variables final only treats the symptoms a little bit, whereas refactoring the function would likely be more helpful. "Final" doesn't carry as much information as it seems: while a variable marked as "final" cannot be reassigned (and even then, its members can be changed), there's nothing to enforce that a variable that never gets reassigned is also marked as "final". If there were, it would likely be quite tedious having to add/remove "final"s all the time while editing code; not to mention it would break all existing code. As a result, in most Java code I've seen, a select few variables are marked as final whereas most aren't, and my usual thought process when reading such code is "Is this particular variable not declared final because the programmer forgot/thought it would be obvious, or because it actually gets modified somewhere? Better read all of the code anyway to check.", which I think somewhat defeats the purpose. On the other hand, some Java programmers, out of self-discipline, mark every variable they can as final, but this is a) a lot of effort to maintain (whenever you delete an assignment somewhere, you might have to go to the declaration and add "finally" there, which is easily forgotten) and b) in the majority of cases, which are small methods, it merely adds clutter and might actually decrease readability. Imo it's extra trouble for the writer and reader of the code alike, without much benefit. Now, the above is mostly just my personal opinion, but speaking objectively, this might also be quite hard to implement: with a final local variable, you'd expect to be able to do something like this:

Local x:Int Final
If condition Then
    x = 1
Else
    x = 2
End If

To implement this, the compiler needs to understand that x is only ever assigned once, which would need a bunch of static analysis that afaik we don't currently have. This is not an issue with a Using Local ... variable, since we require the declaration to have an initializer and can prohibit any further assignment within the Using block (which should be quite similar to the volatile check already implemented for #312). Technically we don't need this special restriction for Using block variables either - we could simply allow assignments like to any other variable. But if someone ever does this, it's likely to be a mistake, since what they'd assign would be another disposable object and the Using block would only dispose one of the two. This could easily lead to leaking resources by accident, that's why I might be best to make an exception and prevent assignments to that particular variable. C# does the same: it doesn't allow declaring local variables as read-only, but certain syntax elements (such as using blocks and, unlike BlitzMax, also foreach loops) do not allow assignments to their variables.

I do think something like "final" it would be a nice feature to have for Fields however. Since we currently have neither that nor properties in BlitzMax, it isn't really possible to create an immutable class, other than by making all its fields private and writing getter methods for each of them. It would be nice to either have properties, or to be able to specify that a field can't assigned more than once / outside the constructor. Still, I would prefer a different keyword for this than Final. Going by its english meaning, the word fits. But it already means "can't be overridden" when applied to a functions/methods, or in other words "you can't have declare something with the same signature in a derived class", and that's completely different from the meaning it would have on fields. C#/VB use readonly/ReadOnly for this which makes the difference more clear. Unfortunately, we also have the same problem of requireing static analysis here. Making the field only assignable in the constructor would be the simplest implementation, but it would be nice to be able to set such fields in an Init or Create method called from the constructor as well. Doing that would require the compiler to make sure that said method isn't called from anywhere else in the code, which seems difficult...

Back on topic: it seems that "Using"/"IDisposable"/"Dispose" appear to be the preferred names after all? I'll look into implementing this then. :)

HurryStarfish commented 6 years ago

it would be nice to be able to set such fields in an Init or Create method called from the constructor as well. Doing that would require the compiler to make sure that said method isn't called from anywhere else in the code, which seems difficult...

...Nevermind this part, actually. I've just realized that even Java and C# both don't allow this. And we could work around this in BlitzMax in a way that Java can't: by passing the final/readonly fields to an Init method as Var parameters.

HurryStarfish commented 6 years ago

I've implemented Using statements as "syntactic sugar" so far (turning them into Try...Finally statements in their OnSemant method), but I've now realized that this is not a good approach for performance reasons, since it would result in multiply nested Try blocks when "Using" multiple ressources. They should be handled separately by the C translator. This means that in the future, the translator's tryStack can not only contain TTryStmts, but TUsingStmts as well. What would be a good name for a common base class of both, more generally representing something that catches and examines exceptions bubbling up the stack? TExceptionBarrierStmt maybe?

HurryStarfish commented 6 years ago

Update: The feature is basically done and works in release builds. Now I only have the same problem left that I had with Finally blocks - trying to solve the "Invalid scope kind" errors and other strange things that happen when stepping through code with the debugger...