DIP44: scope(class) and scope(struct)

[Dmitry Olshansky]

28-Aug-2013 00:41, H. S. Teoh пишет:
> On Wed, Aug 28, 2013 at 12:24:43AM +0400, Dmitry Olshansky wrote:
>> 27-Aug-2013 18:25, H. S. Teoh пишет:
>>> On Tue, Aug 27, 2013 at 04:01:57PM +0400, Dmitry Olshansky wrote:
>>
>>>
>>> What if move(r2) throws? Then res1 won't get cleaned up, because r1 has
>>> already been nulled by the move.
>>>
>>
>> Then something is terribly wrong :)
>>
>> Rule #1 of move should be is that it doesn't throw.
>
> std.algorithm.move is not nothrow. I tried to make it nothrow, and the
> compiler told me:
>
> std/algorithm.d(1604): Error: 'object.TypeInfo.destroy' is not nothrow
> std/algorithm.d(1596): Error: function 'std.algorithm.move!(DirIteratorImpl).move' is nothrow yet may throw
> std/typecons.d(3522): Error: template instance std.algorithm.move!(DirIteratorImpl) error instantiating
> std/file.d(2526):        instantiated from here: RefCounted!(DirIteratorImpl, cast(RefCountedAutoInitialize)0)
> std/algorithm.d(1604): Error: 'object.TypeInfo.destroy' is not nothrow
> std/algorithm.d(1596): Error: function 'std.algorithm.move!(Impl).move' is nothrow yet may throw
> std/typecons.d(3522): Error: template instance std.algorithm.move!(Impl) error instantiating
> std/net/curl.d(2040):        instantiated from here: RefCounted!(Impl)
> std/algorithm.d(1604): Error: 'object.TypeInfo.destroy' is not nothrow
> std/algorithm.d(1596): Error: function 'std.algorithm.move!(Impl).move' is nothrow yet may throw
> std/typecons.d(3522): Error: template instance std.algorithm.move!(Impl) error instantiating
> std/net/curl.d(2747):        instantiated from here: RefCounted!(Impl)
> std/algorithm.d(1604): Error: 'object.TypeInfo.destroy' is not nothrow
> std/algorithm.d(1596): Error: function 'std.algorithm.move!(Impl).move' is nothrow yet may throw
> std/typecons.d(3522): Error: template instance std.algorithm.move!(Impl) error instantiating
> std/net/curl.d(3065):        instantiated from here: RefCounted!(Impl)
>
> Isn't that scary? This means that the case I presented above is not
> hypothetical -- if destroy throws, then you're screwed.

Sorry but this is hypocritical. Obviously nothrow attribute wouldn''t 
work as the only thing we must ensure is that T.init is safely destroyable.

>
> (And yes it's a very very rare case... but would you want to find out
> the hard way when a problem that triggers a throw in destroy slips past
> QA testing 'cos it's so rare, and shows up in a customer's environment
> where it may take hours or days or even months to debug?)

I'm not sold. Bugs are bugs in any case.

>
> Basically, the only way to be 100% sure is to use scope(this),

That yet has to be implemented and is full of interesting interplay with 
other features. No thanks.
>  or the
> manual equivalent thereof (see below).

That is flawed or use a different code practice.

>
>> It just blits the damn data. Even if this is currently broken..
>
> It also calls destroy, which is not nothrow. :)

Oh my. And how many things are by @safe but can't be.
The only assertion missing is that destroy of T.init can't throw.
It is btw a common bug...
https://github.com/D-Programming-Language/phobos/pull/1523

>
>> At the very least 2-arg version certainly can avoid throw even if T
>> has a bogus destructor that goes bottom up on T.init.
>>
>> BTW same with swap and at least in C++ that's the only way to avoid
>> exceptions creeping up from nowhere.
> [...]
>
> Well this is kinda getting away from the main point, which is that
> scope(this) allows us to hide all these dirty implementation details
> under a compiler-implemented solution that makes sure things are always
> done right. This is part of the reason I wrote DIP44: although it *is*
> possible to manually write code that behaves correctly, it's pretty
> tricky, and I doubt many people even realize the potential pitfalls.
> Ideally, straightforward D code should also be correct by default.

It's always seems easy to hide away an elephant in the compiler. Please 
let's stop this tendency.
Anyhow if we are to reach there a better solution that occurred to me 
would be to make compiler call destructors on "cooked" fields as it 
already keeps track of these anyway. It rides on the same mechanism as 
initializing immutables in constructor. Then original code with 3 RAII 
wrappers works out of the box and job is done.

 > Anyway, none of this move/nothrow nonsense is needed if we write things
 > this way:

Wrong. Make that 3 resources and then what if res2.release throws? - you 
never get to the first one. Or simply res2.release throws in destructor 
even with 2 of them, chain of cleanups doesn't work. You have to have 
primitives that don't throw at some level of this or continue wrapping 
in try/finally:)

	struct S {
		void delegate()[] __cleanups;
		Resource res1;
		Resource res2;
		Resource res3;

		this() {
			scope(failure) __cleanup();
			res1 = acquireResource();
			__cleanups ~= { res1.release(); };

			res2 = acquireResource();
			__cleanups ~= { res2.release(); };

			res3 = acquireResource();
			__cleanups ~= { res3.release(); };
		}

		~this() { __cleanup(); }

		private void __cleanup() {
			foreach_reverse (f; __cleanups)
				f();
		}
	}


-- 
Dmitry Olshansky