Permalink
Reply
SafeRefCounted depends on DIP1000 to work. Because of that, it was requested early in the PR review adding it that it should only be @safe when DIP1000 is on, not otherwise. This sounded obviously a good idea back then, so the destructor of SafeRefCounted has a static if that checks for DIP1000 and does the deallocation inside a @trusted lambda if the switch is enabled, directly otherwise.
This makes the destructor @system for non-DIP1000 client code, @safe for DIP1000 code. But, I've come to think it'd have been better to leave it @safe for all code.
Have you looked at std.dirEntries recently? It has a compile-time template parameter called useDIP1000, that is always set automatically and is even checked with a static assert to be set to be default option. Isn't this crazy? But believe or not, the unit tests don't pass without the template parameter!
The issue is, the return value of DirEntries is implemented with SafeRefCounted. This means it, like SafeRefCounted itself, is safe iff DIP1000 is on. Without the useDIP1000 flag, the linker can attempt linking to non-DIP1000 dirEntries from dip1000 client code or vice-versa, causing an ABI mismatch since the safety of the function affects it's name mangling.
Imagine some average user trying to define their own type with SafeRefCounted and getting linker errors like that. When I encountered the linker errors, I knew that I had just made SafeRefCounter safety to depend on the DIP1000 flag, but it was still very tricky to figure out what was happening and how to solve it. We certainly can't expect the same from users.
Now, maybe this would let non-DIP1000 users to write unsafe code in @safe. I'm not actually sure if that's the case, but assuming it is I still think it would be the lesser evil here. Remember, @safe has pointer escaping holes in non-DIP1000 code anyway. I don't see how the possibility of the std.typecons.borrow argument function escaping the reference to the payload is any different from the possibility of leaking a slice of a static array in the stack.
Which leads to another issue: language modules and Phobos 3.
My understanding is the current plan with Phobos 3 is to not use DIP1000 in it. SafeRefCounted depends on DIP1000. Without it it isn't any better than the old RefCounted type.
Surprisingly, this doesn't automatically mean it can't be done in Phobos 3. Within the present language it wouldn't be possible, but modules are going to change the calculus.
In the future when DIP1000 is on or off per module as opposed to compilation unit, it is going to the client code which has to be DIP1000 for ref counting to be safe. The reference counted type can be implemented with our without DIP1000 just as well (although non-DIP1000 implementation of borrow would probably need a currently missing ability to manually inspect the scopeness of the argument function - currently the safety is inferred by calling the argument function with a scope argument which probably doesn't work outside DIP1000. Also @safety unit tests of it would need DIP1000.).
Modules are also one more reason to do away with inspecting for DIP1000. With modules, the current definition of the inspection
private auto dip1000Test(int x) {return *&x;}
// Used in the `static if`
package(std) enum dip1000Enabled
= is(typeof(&dip1000Test) : int function(int) @safe);
is going to inspect Phobos, not the client code. While we're probably going to have a way to inspect client code instead, IMO it is a code smell if you're inspecting the module / completion flags of your client code.
What are your thoughts?
