prevent instantiation of Destroyable types outside of try()

[gavinking]

Perhaps the typechecker should actually prevent you from instantiating a subtype of Destroyable unless:

• the instantiation occurs as the resource expression of a try, or
• the instantiation occurs in the body of an implementation of Destroyable.open().

WDYT?

UPDATE: or perhaps we should automatically destroy the thing when the scope in which it was instantiated is destroyed!

[FroMage]

I think we decided that was a bad idea because it prevents you from instantiating these in functions, didn't we already remove such a restriction in the past?

[gavinking]

didn't we already remove such a restriction in the past?

I don't think so, but I believe you if you say we discussed it.

[FroMage]

We did: https://github.com/ceylon/ceylon-spec/issues/636#issuecomment-17275174 ;)

[gavinking]

Doesn't look like the same issue to me.

[FroMage]

That comment points to an initial limitation where you only allowed Closeable initialisers to be used in try-with-resource, which prevented people from using functions to instantiate them, which you removed (the limitation). If you prevent Closeable from being instantiated outside of try then you prevent them being instantiated in functions and that pretty much adds that limitation back. It's the same thing viewed from the other side of the fence.

[gavinking]

If you prevent Closeable from being instantiated outside of try then you prevent them being instantiated in functions

Well that's not quite what I proposed. Quoting:

or the instantiation occurs in the body of an implementation of Closeable.open().

[FroMage]

It would still prevent this:

Closeable openThis(){
  // instantiate it here
  return ...;
}

void doit(){
 try(r = openThis()){
 }
}

I don't see the difference with the original reasons for removing the initial similar restriction.

[gavinking]

it would still prevent this:

That is already prevented today.

[FroMage]

Is it? I thought we lifted that restriction back then after we decided it was too heavy-handed?

[FroMage]

Ah no indeed you did not remove those restrictions despite everyone else saying you should, and now you want to add more? Just remove them ;)

[quintesse]

Indeed, why those restrictions? What's the point?

[gavinking]

Indeed, why those restrictions? What's the point?

The point is that a Closeable is supposed to be used once and then discarded. That's the model.

[gavinking]

Worse, it would let you share the same Closeable between multiple trys.

[tombentley]

Does does this affect selector-like APIs though? Doesn't it prevent having a container of Closeable instances which will be closed at some point, but whose open/close semantics are not block structured?

[gavinking]

@tombentley if the open/close semantics were not block structured we would not be using try to close the resource.

[FroMage]

I still think it's a gigantic mistake to have the original limitation in place, and adding the new one would only make it worse. I vote we remove the original limitation and rejoice.

[gavinking]

I think you're out of your mind.

[FroMage]

The feeling is mutual, because despite the many arguments we already made in favour of dropping the limitation, you're still against it. I'm not going to argue on this topic any more, we've already provided a long list of good reasons for dropping the limitation and you persist in trying to make it harder for people to use try/resource, obviously nothing we can say make any difference, so go ahead and add more restrictions if it makes you happy.

[gavinking]

Look, RAII is not a cheap way to automatically call a method at the end of a block, which is what you guys seem to think it is. Rather, it is a pattern for having objects whose lifecycles are lexically scoped to a block.

Now, yes, sure, there are ways to break that pattern today if you do something obviously wrong like storing a reference to the Closeable somewhere, and it's unfortunate that we can't reasonably prevent that. But if we allow the resource to be obtained from a method or attribute, we can be sure that people are going to break the pattern, and use shared Closeables, or reuse Closeables.

Indeed, that's precisely why you guys are against this restriction, because you want to have shared Closeables. But that means that the interface Closeable would have a very, very weak contract. Some Closeables would be sharable, others would not be. Some would be reusable, others would not be. And it would be really fucking difficult for the user to know what he can fucking do with a reference to a Closeable.

No, much better to give Closeable a strong lifecycle, and restrict the usage to a single pattern that everyone knows.

It is of course, trivially easy to express the concept of sharing or reuse in terms of this pattern. For example:

try (resource.GetLock()) {
    //do stuff with the locked resource
}

Is absolutely not harder to write than:

try (resource.lock()) {
    //do stuff with the locked resource
}

[tombentley]

But

try (resource.GetLock()) {
    //do stuff with the locked resource
}

requires something to actually be allocated every time you want to acquire a lock, which would have terrible performance consequences. Which means people won't use it for that sort of thing, which is a shame.

[gavinking]

requires something to actually be allocated every time you want to acquire a lock

One object!?

[FroMage]

It prevents every case of reusing a Closeable instance and reference counting unless you go through hoops. It also make it impossible to instantiate a new Closeable in a function, since we are forced to call an instantiator. This is not only a big WTF for new users, but one that is very hard to justify. I'm also afraid it would lead to more Closeable subtypes being defined than would otherwise be required if we allowed them to be instantiated in abstracting functions.

[gavinking]

It prevents every case of reusing a Closeable instance

@FroMage That's literally true. We're preventing you from reusing a Closeable. So you have to very slightly refactor your code so that the thing you're reusing is not the Closeable itself, but a thing that can be used to produce a Closeable.

That is, you don't make LockableResource a Closeable, you give it a Lock member class that is Closeable.

It also make it impossible to instantiate a new Closeable in a function, since we are forced to call an instantiator.

Again, you just need to separate in your mind the thing you want to instantiate in the function from the thing that satisfies Closeable.

This is not only a big WTF for new users, but one that is very hard to justify.

Oh c'mon, nonsense. What this guarantees is that all APIs will treat Closeables in a consistent way, and that there is truly a well-defined lifecycle for anything Closeable.

[FroMage]

and that there is truly a well-defined lifecycle for anything Closeable

One that only you seem to think is a good thing.

[gavinking]

So I think the problem here is that we have this single interface Closeable that is trying to be too many things to too many people. We can see that by the fact that it has an open() method, which is actually pretty nasty when you look at stuff like Sql.Results and Sql.Transaction(). If the user types this:

 Transaction tx = sql.Transaction();
 try {
     //do stuff
     tx.close(null);
 }
 catch (e) {
     tx.close(e);
 }

Then that code is actually broken, because it's missing a call to open().

The reason open() actually found its way into this in the first place was that it seemed useful for shit like locks which were going to be shared between multiple trys. But when you're talking about that use case, you're not really talking about "closeable" things (RAII) but about things with a totally different kind of semantics: temporary enlistment, or obtainment.

So what about if we decouple these two kind of things, resulting in:

interface Resource of Closeable|Enlistable {}

interface Closeable satisfies Resource {
    shared formal void close(Exception? e);
}

interface Enlistable satisfies Resource {
    shared formal void enlist();
    shared formal void unlist(Exception? e);
}

In the case of a Closeable, the current restriction about having to instantiate the thing in the try would be enforced by the typechecker. In the case of an Enlistable it would not. A resource expression would have to be either of type Closeable, or of type Enlistable (but not of type Resource).

[gavinking]

Or, even better:

interface Resource of Destroyable|Enlistable {}

interface Destroyable {
    shared formal void destroy(Exception? e);
}

interface Enlistable {
    shared formal void enlist();
    shared formal void unlist(Exception? e);
}

[tombentley]

I think that's a good idea.

[thradec]

I like it too, just a note, why common supertype Resource?

2014/1/8 Tom Bentley notifications@github.com

I think that's a good idea.

— Reply to this email directly or view it on GitHubhttps://github.com/ceylon/ceylon-spec/issues/894#issuecomment-31829942 .

[gavinking]

why common supertype Resource?

Because I wanted to force the two interfaces to be disjoint. Perhaps that's unnecessary, since I'm requiring the resource expression to be either Closeable or Enlistable. Hell, perhaps we could make them classes, and get disjointness for free.

[FroMage]

This looks more practical indeed.

[gavinking]

Not for 1.1.

[gavinking]

An even more interesting idea would be to have things instantiated outside of try be automatically destroyed when the scope in which they were instantiated finishes executing.