On Wednesday, 16 November 2022 at 12:27:27 UTC, Adam D Ruppe wrote:
>
> ... (random, unjustifed) Phobos change ...
>

If you don't mind me asking, what was the change?

On Wednesday, 16 November 2022 at 14:30:14 UTC, bauss wrote:
> If you don't mind me asking, what was the change?

Socket.close is now `scope` on `this`.

It isn't necessarily bad, like close prolly is scope anyway, but it is it worth the breakage to user code? I don't see the benefit.

On Wednesday, 16 November 2022 at 15:39:18 UTC, Dennis wrote:
> class destructors shouldn't escape `this` in the constructor,
> so a PR was made to make class destructors implicitly `scope`.

The current implementation of class dtors is broken anyway; it doesn't even correctly implement the D spec! (The spec claims they are "always virtual", but they are actually never virtual, but the implementation tries to emulate it) and the statement that "The destructor for the super class automatically gets called when the destructor ends." is again only half-true, this does happen when you call rtFinalize, but it is not done in general when you call the dtor through other means.

This has profound consequences with `@safe` and other static attributes - they just plain don't work! You can't call `.destroy()` in a @safe (or nogc, etc.) contexts because destroy has no way of proving the destructor actually follows those rules at all.

If it followed the spec, you could prove it, just like with any other virtual method, because subclasses would be forced to follow the rules. But it would be fairly strict:

```
class A {
   void dispose() @safe {}
}

class B : A {
   override void dispose() @system // compile error, cannot override safe with system

   // btw since attributes are inherited when overriding, you
   // don't actually have to write out `@safe`
   override void dispose() @safe {
       super.dispose(); // all good!
   }

   // but can you tighten?
   override void dispose() @safe @nogc {
       // signature OK, you can tighten restrictions, but...
       // the spec says it must call the parent...
       super.dispose(); // uh oh, @nogc this cannot call non-@nogc super
   }
}

void destroy(T)(T t) {
   t.dispose();
}

A a;
destroy(a); // infers to @safe because A.dispose
B b;
destroy(b); // the best this can do is also be @safe because B.dispose must be able to call A.dispose

// even if you made destroy call this.dtor(); then this.super.dtor(); in a loop, like rt_finalize does today, it could still at best infer to whatever the top-most class that defines the dtor actually specified
```

That "problem" with the mandatory call to `super.x()`... in effect, a destructor would not be allowed to tighten conditions. It'd have to exactly match the parent class' interface, attributes and all.

I use the quotes on "problem" because that's not necessarily a problem! It'd work just fine, just you can't tighten things like you normally do in a subclass, since the parent chain is also required to be called.


But the current implementation we have doesn't actually treat destructors as virtual. It just pretends they are when it is called. The fix we need is for the compiler to treat them that way too, and force all child classes to inherit the attributes from the parent class.

Once you do that, you actually CAN meaningfully make a `@safe` destructor. But until then, all destructors are necessarily called from a `@system` context regardless; safe dtors just plain don't work, even if you try to .destroy() it explicitly in your `@safe` scope, because the static type system cannot prove that a dtor in a child type, only dynamically known, didn't do something `@system`.

So, since `@safe` dtors are currently broken anyway, what do we gain by this change?

I'd note that typing this post took about 50x the effort than fixing the code; this specific change is annoying but not terribly problematic. Just I wish breaking changes came with real world usability fixes rather than just patching one specific scenario while leaving the rest of the design as-is. I'd rather have to do a batched bigger fix for a bigger gain than a sequence of small fixes.


BTW, I'd be surprised if either of those bugzilla things happened in the real world, since it'd most likely immediately crash if you did anyway. (Which is true of a lot of random things you do in finalizers. They are quite tricky in what you can actually do in them - obviously, no GC operations since it'd deadlock, it is fairly well known you can't access GC'd members since they might already be collected, but did you know even manually managed members might be problematic? Since a finalizer is called from an arbitrary thread, of the manually managed member refers at all to a thread-local piece of data, you're back in crash city. Notably, many Win32 gui handles are thread local, so don't try to clean them up in a finalizier either. And there's no attribute to help with that. Except maybe pure which is overkill lol)

> Since std.socket calls `close` in the destructor of a socket, `close` had to be marked `scope` as well.

This btw is also a bit iffy; I wish Socket had a method to release its file descriptor so it wouldn't try to close it anymore. That'd be a fairly easy additive change though, maybe I'll PR it myself now that I'm thinking about it.

On Wednesday, 16 November 2022 at 18:57:54 UTC, Iain Buclaw wrote:
> before [bitfields] gets properly accepted into the language.

...which they hopefully never will. Such a godawful design, this would be another miss for D that Walter will regret later.