www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - DIP44: scope(class) and scope(struct)

reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
I've written up a proposal to solve the partially-constructed object
problem[*] in D in a very nice way by extending scope guards:

	http://wiki.dlang.org/DIP44

[*] The partially-constructed object problem is when you have a class
(or struct) that must acquire some number of external resources, usually
to set them as member fields, but before the ctor is able to initialize
all of these fields, an Exception is thrown. Now the object is in a
partially-constructed state: some member fields have been initialized,
and need to be destructed in order to release the associated external
resources, but other fields are still uninitialized so should not be
destructed. This leads to the problem of, how do we clean up in this
situation? We can't call the dtor -- the dtor assumes *all* fields have
been set and will wrongly try to release resources that haven't been
acquired yet. But we can't ignore the issue either -- the resources that
*have* been required need to be released somehow. This DIP proposes a
nice solution to this problem that fits in very well with the existing
scope guards in D.

Destroy! ;-)


T

-- 
Only boring people get bored. -- JM
Aug 23 2013
next sibling parent reply "Andrej Mitrovic" <andrej.mitrovich gmail.com> writes:
On Saturday, 24 August 2013 at 00:45:46 UTC, H. S. Teoh wrote:

 Destroy! ;-)


I won't destroy but I'll say avoid the struct/class keywords and 
use `scope(this)` instead, it looks nicer and will work easier in 
generic code.
Aug 23 2013
next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Aug 24, 2013 at 02:48:45AM +0200, Andrej Mitrovic wrote:

 On Saturday, 24 August 2013 at 00:45:46 UTC, H. S. Teoh wrote:

Destroy! ;-)


 
 I won't destroy but I'll say avoid the struct/class keywords and use
 `scope(this)` instead, it looks nicer and will work easier in
 generic code.


Good point! I'll update the DIP to use scope(this) instead. It does look
nicer. And yes, it's important for generic code. :)


T

-- 
Nothing in the world is more distasteful to a man than to take the path
that leads to himself. -- Herman Hesse
Aug 23 2013
prev sibling parent reply "Andrej Mitrovic" <andrej.mitrovich gmail.com> writes:
On Saturday, 24 August 2013 at 00:48:46 UTC, Andrej Mitrovic 
wrote:

 On Saturday, 24 August 2013 at 00:45:46 UTC, H. S. Teoh wrote:

 Destroy! ;-)


 I won't destroy but I'll say avoid the struct/class keywords 
 and use `scope(this)` instead, it looks nicer and will work 
 easier in generic code.


Perhaps scope(~this) is actually more appropriate.
Aug 23 2013
next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Aug 24, 2013 at 03:06:39AM +0200, Andrej Mitrovic wrote:

 On Saturday, 24 August 2013 at 00:48:46 UTC, Andrej Mitrovic wrote:

On Saturday, 24 August 2013 at 00:45:46 UTC, H. S. Teoh wrote:

Destroy! ;-)


I won't destroy but I'll say avoid the struct/class keywords and
use `scope(this)` instead, it looks nicer and will work easier in
generic code.


 
 Perhaps scope(~this) is actually more appropriate.


Hmm. You do have a point. But I'm not sure I like breaking the pattern
of a single identifier inside scope(). I still think scope(this) looks
nicer and is nicer to type as well. And also reads like "scope of this
object", which kinda conveys the intent.

But anyway, we can bikeshed over the exact syntax later. What of the
proposal itself? Does it make any sense? Any obvious glaring flaws?

Also, it just occurred to me that Adam's manual implementation can be
made to handle partial object construction as well, by using
scope(failure) in the ctor:

	struct S {
		void delegate()[] cleanupFuncs;
		Resource1 res1;
		// ...
		this() {
			// assuming the dtor isn't invoked if the ctor
			// throws.
			scope(failure) cleanup();

			res1 = acquireResource();
			cleanupFuncs ~= { res1.release(); }
			...
		}

		void cleanup() {
			foreach_reverse (f; cleanupFuncs)
				f();
		}

		~this() { cleanup(); }
	}

So scope(this) could simply be lowered to the above code, perhaps with
some compiler low-level handling for the potential problem with calling
a method from ~this(), which I think can be a problem in the current
implementation of dtors? (IIRC 'this' may be invalid when ~this() is
invoked, or something dangerous like that?)

In any case, the above code has a lot of boilerplate which scope(this)
would eliminate, besides expanding the concept of scope guards to object
lifetimes.


T

-- 
Unix was not designed to stop people from doing stupid things, because
that would also stop them from doing clever things. -- Doug Gwyn
Aug 23 2013
prev sibling parent reply "Ramon" <spam thanks.no> writes:
I don't want to be picky but I feel one shouldn't pack too much 
into one thing and in a way break the logic flow and possibly 
introduce limitations.

The basic rule is to clean up in the destructor what has been 
allocated/aquired in the constructor - and this shouldn't be 
interrupted but stay that way.
What you want to address, if I got it right (If not, I apologize 
for my interruption) is the problem of incoherent state, i.e. 
(relating to your example) to have res1 and res2 aquired but not 
res3.
In my (possibly strongly limited) understanding scope(failure) is 
the right approach.

Having a mechanism (as suggested) that also takes care of the 
dtor's job breaks the normal logic and might create new problems 
or limitations by implicitly putting a dtor job into a ctor 
mechanism.

What, for instance, if I aquire 10 resources in ctor and during 
normal programm execution cleanup 7 of them, so only some are 
left for dtor?


If scope(failure) does already work in ctor, just keep it that 
way. If not,HS Teoh's idea to make it work is great. But that 
about it. Don't add more functionality in it. Normal ctor/dtor 
working should be kept and so should scope(). Whatever user wants 
beyond that can be coded by him. Don't break major flow for some 
luxury (having scope in ctor isn't luxury, having it reach into 
dtor, too up to the point of making dtor superflous is).

scope is a brilliant feature. But I feel we shouldn't have it do 
"miracles", even less behind curtains.

In case what I said is just plain stupid and I should have shut 
up as newbie, I apologize and won't disturb any more.
Aug 23 2013
parent reply "Simen Kjaeraas" <simen.kjaras gmail.com> writes:
On Sat, 24 Aug 2013 03:34:32 +0200, Ramon <spam thanks.no> wrote:


 What, for instance, if I aquire 10 resources in ctor and during normal  
 programm execution cleanup 7 of them, so only some are left for dtor?


Then they don't have the same lifetime as the object, and scope(this)
would be the wrong tool for the job.

-- 
Simen
Aug 24 2013
parent "Ramon" <spam thanks.no> writes:
On Saturday, 24 August 2013 at 08:34:10 UTC, Simen Kjaeraas wrote:

 On Sat, 24 Aug 2013 03:34:32 +0200, Ramon <spam thanks.no> 
 wrote:


 What, for instance, if I aquire 10 resources in ctor and 
 during normal programm execution cleanup 7 of them, so only 
 some are left for dtor?


 Then they don't have the same lifetime as the object, and 
 scope(this)
 would be the wrong tool for the job.


Understood. But there *are* objects with different lifetimes. If, 
for instance, I aquire some "nice" objects (GC'd, properly 
through D) - and - some "dirty" malloced stuff (not talking about 
externals).

Thinking about it I also dislike a special thing that also feels 
and shows a special thing (namely "this" rather than e.g. 
"failure").

"Holy rule": No special cases if any possible but everything in a 
consistent way. If there are special cases, somehow make them not 
look like it; try to make them look consistent. The scope 
mechanism deals with success, with failure and with exit. That's 
great, that's sufficient. That's how it should at least *look* 
like in ctors, too.

Another inconsistency: If this mechanism is to "guard" on class 
level, it should be there, at class level and not in a function 
like entity (albeit the rather special "function" this).

Maybe I'm overly shy but I feel that logical consistency is a 
major issue to be a good language and for safety, too.
Aug 24 2013
prev sibling next sibling parent reply "Meta" <jared771 gmail.com> writes:
"With this extension to scope guards, class and struct 
destructors will practically not be needed anymore, since 
scope(this) will take care of cleaning up everything."

I can't think of an example off the top of my head, but is it 
really okay to conflate destruction due to an error during 
construction, and destruction over the regular course of a 
struct's usage? What if one instance requires different code from 
the other? Maybe require that scope(this) statements *only* be 
run if there is an error during construction, while just the 
destructor will be run normally?
Aug 23 2013
parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Aug 24, 2013 at 06:19:25AM +0200, Meta wrote:

 "With this extension to scope guards, class and struct destructors
 will practically not be needed anymore, since scope(this) will take
 care of cleaning up everything."
 
 I can't think of an example off the top of my head, but is it really
 okay to conflate destruction due to an error during construction,
 and destruction over the regular course of a struct's usage? What if
 one instance requires different code from the other? Maybe require
 that scope(this) statements *only* be run if there is an error
 during construction, while just the destructor will be run normally?


If a particular cleanup operation only applies to a ctor failure, you
could just use scope(failure) in the ctor, and put the different cleanup
code in the dtor.

scope(this) is really meant to encapsulate the usual (I think!) case
where the same cleanup code applies in both cases.

Maybe I was a bit too strong to say dtors won't be needed anymore, but
if you leave dtors in, then they can still handle this case. :)


T

-- 
May you live all the days of your life. -- Jonathan Swift
Aug 23 2013
prev sibling next sibling parent reply Piotr Szturmaj <bncrbme jadamspam.pl> writes:
H. S. Teoh wrote:

 I've written up a proposal to solve the partially-constructed object
 problem[*] in D in a very nice way by extending scope guards:

 	http://wiki.dlang.org/DIP44

 Destroy! ;-)


I see some possible implementation problem. Using a scope guard in a 
single function is straightforward because code is executed in the same 
scope.

However, in your proposal scope guard is split to two different 
functions/scopes. This may require copying local stack variables and 
storing them somewhere if they're referred to in the scope(this).

For example:

ResourceManager resManager;

class C {
     Resource res;
     this(int resourceId) {
         res = resManager.acquireResource(resourceId);
         scope(this) resManager.releaseResource(resourceId);
     }
}

Here the resourceId value must be stored somewhere so it can be used 
later, when the object is destructed.

I'm thinking of two possible "solutions" to this problem:

1. disallow referring to local variables in scope(this). In this case 
resourceId should be stored in a field of class C or in the Resource 
itself (it may not always be allowed).

2. allow referring to local variables and create the closure. This 
causes additional memory allocation and also reference to the closure 
must be stored somewhere, perhaps in the class hidden field. Of course, 
this is a "no go", I'm writing this here for comparison.
Aug 23 2013
parent Artur Skawina <art.08.09 gmail.com> writes:
On 08/24/13 08:30, Piotr Szturmaj wrote:

 H. S. Teoh wrote:

 I've written up a proposal to solve the partially-constructed object
 problem[*] in D in a very nice way by extending scope guards:

     http://wiki.dlang.org/DIP44





 2. allow referring to local variables and create the closure. This causes
additional memory allocation and also reference to the closure must be stored
somewhere, perhaps in the class hidden field. Of course, this is a "no go", I'm
writing this here for comparison.


That is what he's actually proposing. And, yes, it's not a good idea.
Implementing it via runtime delegates, implicit captures/allocs and 
extra hidden fields inside aggregates would work, but the cost is too
high. It's also not much of an improvement over manually registering
and calling the delegates. Defining what happens when a ctor fails
would be a good idea, having a cleanup method which defaults to `~this`,
but can be overridden could help too.

There are other problems with that DIP, like making it harder to see
what's actually going on, by splitting the dtor code and having it
interleaved with another separate flow.


It *is* possible to implement a similar solution without any RT cost,
but it would need:
a) flow analysis - to figure out the cleanup order, which might not be 
    statically known (these cases have to be disallowed)
b) a different approach for specifying the cleanup code, so that 
    implicit capturing of ctor state doesn't happen and it's not
    necessary to read the complete body of every ctor just to find
    out what a dtor does.


artur
Aug 24 2013
prev sibling next sibling parent "deadalnix" <deadalnix gmail.com> writes:
On Saturday, 24 August 2013 at 00:45:46 UTC, H. S. Teoh wrote:

 I've written up a proposal to solve the partially-constructed 
 object
 problem[*] in D in a very nice way by extending scope guards:

 	http://wiki.dlang.org/DIP44

 [*] The partially-constructed object problem is when you have a 
 class
 (or struct) that must acquire some number of external 
 resources, usually
 to set them as member fields, but before the ctor is able to 
 initialize
 all of these fields, an Exception is thrown. Now the object is 
 in a
 partially-constructed state: some member fields have been 
 initialized,
 and need to be destructed in order to release the associated 
 external
 resources, but other fields are still uninitialized so should 
 not be
 destructed. This leads to the problem of, how do we clean up in 
 this
 situation? We can't call the dtor -- the dtor assumes *all* 
 fields have
 been set and will wrongly try to release resources that haven't 
 been
 acquired yet. But we can't ignore the issue either -- the 
 resources that
 *have* been required need to be released somehow. This DIP 
 proposes a
 nice solution to this problem that fits in very well with the 
 existing
 scope guards in D.

 Destroy! ;-)


 T


I like the feature. I wouldn't say this is the most important 
thing to add here, the same can be achieved with scope(failure) 
and destructor.
Aug 24 2013
prev sibling next sibling parent "Tobias Pankrath" <tobias pankrath.net> writes:
On Saturday, 24 August 2013 at 00:45:46 UTC, H. S. Teoh wrote:

 I've written up a proposal to solve the partially-constructed 
 object
 problem[*] in D in a very nice way by extending scope guards:


[snip]

What about a language rule, that for every type T destroy(t) must 
be valid if t == T.init?

Couldn't this problem be solved by using RAII and destructors? 
You would (only) need to make sure that every member is either 
correctly initialised or T.init.
Aug 24 2013
prev sibling next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
24-Aug-2013 04:44, H. S. Teoh пишет:

 I've written up a proposal to solve the partially-constructed object
 problem[*] in D in a very nice way by extending scope guards:

 	http://wiki.dlang.org/DIP44



 Destroy! ;-)


Instead of introducing extra mess into an already tricky ctor/dtor 
situation. (Just peek at past issues with dtors not being called, being 
called at wrong time, etc.)

I'd say just go RAII in bits and pieces. Unlike scope, there it works 
just fine as it has the right kind of lifetime from the get go. In 
function scope (where scope(exit/success/failure) shines) RAII actually 
sucks as it may prolong the object lifetime I you are not careful to 
tightly wrap it into { }.

Start with this:

class C {
     Resource1 res1;
     Resource2 res2;
     Resource3 res3;

     this() {
         res1 = acquireResource!1();
         res2 = acquireResource!2();
         res3 = acquireResource!3();
     }

     ~this() {
         res3.release();
         res2.release();
         res1.release();
     }
}

Write a helper once:

struct Handler(alias acquire, alias release)
{
	alias Resource = typeof(acquire());
	Resource resource;
	this(int dummy) //OMG when 0-argument ctor becomes usable?
	{
		resource = acquire();
	}

	static auto acquire()
	{
		return Handler(0); //ditto
	}

	~this()
	{
		release(resource);
	}
	alias this resource;
}


Then:

class C{
     Handler!(acquireResource!1, (r){ r.release(); }) res1;
     Handler!(acquireResource!2, (r){ r.release(); }) res2;
     Handler!(acquireResource!3, (r){ r.release(); }) res3;
     this(){
	res1 = typeof(res1).acquire();
	res2 = typeof(res2).acquire();
	res3 = typeof(res3).acquire();
     }
}

There are many more alternatives on how to auto-generate RAII helpers. 
The point is we can easily do so, after all our compile-time kung-fu is 
strong.

-- 
Dmitry Olshansky
Aug 24 2013
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 8/24/13 10:31 AM, Dmitry Olshansky wrote:

 24-Aug-2013 04:44, H. S. Teoh пишет:

 I've written up a proposal to solve the partially-constructed object
 problem[*] in D in a very nice way by extending scope guards:

     http://wiki.dlang.org/DIP44



 Destroy! ;-)


 Instead of introducing extra mess into an already tricky ctor/dtor
 situation. (Just peek at past issues with dtors not being called, being
 called at wrong time, etc.)

 I'd say just go RAII in bits and pieces.


Agreed. DIP44 supports an idiom of handling multiple unprotected 
resources within the same object, which should be infrequent and 
generally unrecommended.

Andrei
Aug 24 2013
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/23/2013 5:44 PM, H. S. Teoh wrote:

 I've written up a proposal to solve the partially-constructed object
 problem[*] in D in a very nice way by extending scope guards:

 	http://wiki.dlang.org/DIP44


Not a bad idea, but it has some issues:

1. scope(failure) takes care of most of it already

2. I'm not sure this is a problem that needs solving, as the DIP points out, 
these issues are already easily dealt with. We should be conservative about 
adding more syntax.

3. What if the destructor needs to do more than just unwind the transactions? 
Where does that code fit in?

4. The worst issue is the DIP assumes there is only one constructor, from which 
the destructor is inferred. What if there is more than one constructor?
Aug 24 2013
parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Aug 24, 2013 at 01:09:44PM -0700, H. S. Teoh wrote:
[...]

 On Sat, Aug 24, 2013 at 12:27:37PM -0700, Walter Bright wrote:


[...]

 4. The worst issue is the DIP assumes there is only one constructor,
 from which the destructor is inferred. What if there is more than
 one constructor?


 
 This is not a problem. If there is more than one constructor, then
 only those scope(this) statements in the ctor that were actually
 encountered will trigger when the object reaches the end of its
 lifetime.  You already have to do this anyway, since if the ctor
 throws an Exception before completely constructing the object, only
 those scope(this) statements that have been encountered up to that
 point will be triggered, not all of them. Otherwise, you'd still have
 the partially-initialized object problem.


[...]

Argh, that was poorly worded. What I mean is this:

	struct S {
		this(int) {
			scope(this) writeln("A");
		}
		this(float) {
			scope(this) writeln("B");
		}
	}

	void fun1() {
		auto s = S(1);
	}	// prints "A"

	void fun2() {
		auto s = S(1.0);
	}	// prints "B"


T

-- 
Dogs have owners ... cats have staff. -- Krista Casada
Aug 24 2013
prev sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Aug 24, 2013 at 01:01:12PM +0200, Artur Skawina wrote:

 On 08/24/13 08:30, Piotr Szturmaj wrote:

 H. S. Teoh wrote:

 I've written up a proposal to solve the partially-constructed
 object problem[*] in D in a very nice way by extending scope
 guards:

     http://wiki.dlang.org/DIP44




 

 2. allow referring to local variables and create the closure. This
 causes additional memory allocation and also reference to the
 closure must be stored somewhere, perhaps in the class hidden field.
 Of course, this is a "no go", I'm writing this here for comparison.


 
 That is what he's actually proposing. And, yes, it's not a good idea.
 Implementing it via runtime delegates, implicit captures/allocs and
 extra hidden fields inside aggregates would work, but the cost is too
 high. It's also not much of an improvement over manually registering
 and calling the delegates. Defining what happens when a ctor fails
 would be a good idea, having a cleanup method which defaults to
 `~this`, but can be overridden could help too.


The issue with that is that the initialization code and the cleanup code
has to be separated, potentially by a lot of unrelated stuff in between.
The genius of scope guards is that initialization and cleanup is written
in one place even though they actually happen in different places, so
it's very unlikely you will forget to cleanup correctly.



 There are other problems with that DIP, like making it harder to see
 what's actually going on, by splitting the dtor code and having it
 interleaved with another separate flow.


I think it's unhelpful to conflate scope(this) with dtors. Yes there is
some overlap, but if you treat them separately, then there is no
problem (assuming that a dtor is actually necessary).



 It *is* possible to implement a similar solution without any RT cost,
 but it would need:
 a) flow analysis - to figure out the cleanup order, which might not be 
     statically known (these cases have to be disallowed)
 b) a different approach for specifying the cleanup code, so that 
     implicit capturing of ctor state doesn't happen and it's not
     necessary to read the complete body of every ctor just to find
     out what a dtor does.


I think that's an unhelpful way of thinking about it. What about we
think of it this way: the ctor is acquiring X number of resources, and
by wrapping the resource-releasing code in scope(this), we guarantee
that these resources will be correctly released. Basically, scope(this)
will take care of invoking the release code whenever the object's
lifetime is over, whether it's unsuccessful construction, or
destruction.

It's just like saying scope guards are useless because it's equivalent
to a try-catch block anyway (and in fact, that's how the front end
implements scope guards). One may even argue scope guards are bad
because the cleanup code is sprinkled everywhere rather than collected
in one place. But it's not really about whether it's equivalent to
another language construct; it's about better code maintainability.
Separating the code that initializes something from the code that cleans
up something makes it harder to maintain, and more error-prone (e.g.
initialize 9 things, forget to clean up one of them). Keeping them
together in the same place makes code correctness clearer.


On Sat, Aug 24, 2013 at 02:48:53PM +0200, Tobias Pankrath wrote:
[...]

 Couldn't this problem be solved by using RAII and destructors? You
 would (only) need to make sure that every member is either correctly
 initialised or T.init.


How would you use RAII to solve this problem? If I have a class:

	class C {
		Resource1 res1;
		Resource2 res2;
		Resource3 res3;
		this() {
			...
		}
	}

How would you write the ctor with RAII such that if it successfully
inits res1 and res2, but throws before it inits res3, then only res1 and
res2 will be cleaned up?


On Sat, Aug 24, 2013 at 09:31:52PM +0400, Dmitry Olshansky wrote:
[...]

 Instead of introducing extra mess into an already tricky ctor/dtor
 situation. (Just peek at past issues with dtors not being called,
 being called at wrong time, etc.)
 
 I'd say just go RAII in bits and pieces. Unlike scope, there it
 works just fine as it has the right kind of lifetime from the get
 go. In function scope (where scope(exit/success/failure) shines)
 RAII actually sucks as it may prolong the object lifetime I you are
 not careful to tightly wrap it into { }.
 
 Start with this:
 
 class C {
     Resource1 res1;
     Resource2 res2;
     Resource3 res3;
 
     this() {
         res1 = acquireResource!1();
         res2 = acquireResource!2();
         res3 = acquireResource!3();
     }
 
     ~this() {
         res3.release();
         res2.release();
         res1.release();
     }
 }
 
 Write a helper once:
 
 struct Handler(alias acquire, alias release)
 {
 	alias Resource = typeof(acquire());
 	Resource resource;
 	this(int dummy) //OMG when 0-argument ctor becomes usable?
 	{
 		resource = acquire();
 	}
 
 	static auto acquire()
 	{
 		return Handler(0); //ditto
 	}
 
 	~this()
 	{
 		release(resource);
 	}
 	alias this resource;
 }
 
 
 Then:
 
 class C{
     Handler!(acquireResource!1, (r){ r.release(); }) res1;
     Handler!(acquireResource!2, (r){ r.release(); }) res2;
     Handler!(acquireResource!3, (r){ r.release(); }) res3;
     this(){
 	res1 = typeof(res1).acquire();
 	res2 = typeof(res2).acquire();
 	res3 = typeof(res3).acquire();
     }
 }


I don't see how code solves the problem. Suppose this() throws an
Exception after res1 and res2 have been initialized, but before res3 is
uninitialized. Now what? How would the language know to only clean up
res1 and res2, but not res3? How would the language know to only invoke
the dtors of res1 and res2, but not res3?


On Sat, Aug 24, 2013 at 12:27:37PM -0700, Walter Bright wrote:
[...]

 Not a bad idea, but it has some issues:
 
 1. scope(failure) takes care of most of it already
 
 2. I'm not sure this is a problem that needs solving, as the DIP
 points out, these issues are already easily dealt with. We should be
 conservative about adding more syntax.
 
 3. What if the destructor needs to do more than just unwind the
 transactions? Where does that code fit in?


I think it's unhelpful to conflate scope(this) with dtors. They are
related, but -- and I guess I was a bit too strong about saying dtors
are redundant -- if we allow both, then scope(this) can be reserved for
transactions, and you can still put code in ~this() to do non-trivial
cleanups.



 4. The worst issue is the DIP assumes there is only one constructor,
 from which the destructor is inferred. What if there is more than
 one constructor?


This is not a problem. If there is more than one constructor, then only
those scope(this) statements in the ctor that were actually encountered
will trigger when the object reaches the end of its lifetime.  You
already have to do this anyway, since if the ctor throws an Exception
before completely constructing the object, only those scope(this)
statements that have been encountered up to that point will be
triggered, not all of them. Otherwise, you'd still have the
partially-initialized object problem.


T

-- 
Without geometry, life would be pointless. -- VS
Aug 24 2013
next sibling parent "Ramon" <spam thanks.no> writes:
Maybe I'm just plain too unexperienced or dumb but I see problems.

- What's a dtor's job in D? This is not C++ (where cleaning up 
often means freeing/not leaking), in D the GC saves us lots of 
problems. Actually, we do not even really free anything (except 
malloc'ed stuff) but rather hinting the runtime expressis verbis 
that sth. isn't needed anymore.

- scope(this) adds something new to the scope mechanism wich is 
per se bad and imo not justified for luxury.

- I see H.S. Teoh's point to in a way have stuff aquired or 
allocated in ctor kind of "registered" in dtor for proper 
cleanup, but that *can* be done with what we have right now.

- From what I understand the proposal implies some kind of a 
transactional "all or nothing" mechanism. Is a ctor and class 
level the right place to implement that?


 H.S. Teoh

Maybe it would helpful to explain why you think this is an 
*important* problem (rather than a nuisance) and more clearly 
define it (and why it can't be solved using what we have)?

Thanks
Aug 24 2013
prev sibling next sibling parent reply "Tobias Pankrath" <tobias pankrath.net> writes:
On Saturday, 24 August 2013 at 20:11:14 UTC, H. S. Teoh wrote:

 How would you use RAII to solve this problem? If I have a class:

 	class C {
 		Resource1 res1;
 		Resource2 res2;
 		Resource3 res3;
 		this() {
 			...
 		}
 	}

 How would you write the ctor with RAII such that if it 
 successfully
 inits res1 and res2, but throws before it inits res3, then only 
 res1 and
 res2 will be cleaned up?


Like someone else already proposed: Using a wrapper type W that 
releases the resources in it's destructor. W.init wouldn't 
release anything. So by definition (if I recall the rules 
correctly) every new instance of C would have res3 = 
Resource3.init prior to it's constructor called.

Now make sure that a) res3's destructor gets called (check) b) 
res3's destructor may be called on Resource3.init. That would be 
a new rule similar to 'no internal aliasing' and c) that every 
constructor of C either sets res3 to a destructable value or does 
not touch it at all ('transactional programming').
Aug 24 2013
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sun, Aug 25, 2013 at 12:18:27AM +0200, Tobias Pankrath wrote:

 On Saturday, 24 August 2013 at 20:11:14 UTC, H. S. Teoh wrote:

How would you use RAII to solve this problem? If I have a class:

	class C {
		Resource1 res1;
		Resource2 res2;
		Resource3 res3;
		this() {
			...
		}
	}

How would you write the ctor with RAII such that if it successfully
inits res1 and res2, but throws before it inits res3, then only res1
and res2 will be cleaned up?


 
 Like someone else already proposed: Using a wrapper type W that
 releases the resources in it's destructor. W.init wouldn't release
 anything. So by definition (if I recall the rules correctly) every
 new instance of C would have res3 = Resource3.init prior to it's
 constructor called.
 
 Now make sure that a) res3's destructor gets called (check) b)
 res3's destructor may be called on Resource3.init. That would be a
 new rule similar to 'no internal aliasing' and c) that every
 constructor of C either sets res3 to a destructable value or does
 not touch it at all ('transactional programming').


[...]

But don't you still need to manually cleanup in the case of ctor
failure? AFAIK, the dtor is not invoked on the partially-constructed
object if the ctor throws. So you'd still have to use scope(failure) to
manually release the resource.

To prove my point, here is some sample code that (tries to) use RAII to
cleanup:

	import std.stdio;
	
	struct Resource {
		int x = 0;
		this(int id) {
			x = id;
			writefln("Acquired resource %d", x);
		}
		~this() {
			if (x == 0)
				writefln("Empty resource, no cleanup");
			else
				writefln("Cleanup resource %d", x);
		}
	}
	
	struct S {
		Resource res1;
		Resource res2;
		Resource res3;
	
		this(int) {
			res1 = Resource(1);
			res2 = Resource(2);
			assert(res1.x == 1);
			assert(res2.x == 2);
	
			throw new Exception("ctor failed!");
			res3 = Resource(3);	// not reached
			assert(res3.x == 3);	// not reached
		}
	}
	
	void main() {
		try {
			auto s = S(123);
		} catch(Exception e) {
			writeln(e.msg);
		}
	}

Here is the program output:

	Acquired resource 1
	Empty resource, no cleanup
	Acquired resource 2
	Empty resource, no cleanup
	ctor failed!

As you can see, the two resources acquired in the partially-constructed
object did NOT get cleaned up. So, RAII doesn't work in this case.

The two interspersed cleanups were presumably cause by Resource.init
being destructed when res1 and res2 were assigned to. But after being
assigned to, res1 and res2's dtors were NOT invoked.

So I think my scope(this) idea has some merit, since it will correctly
handle this case. :)


T

-- 
Many open minds should be closed for repairs. -- K5 user
Aug 26 2013
next sibling parent "Tobias Pankrath" <tobias pankrath.net> writes:
On Monday, 26 August 2013 at 21:31:40 UTC, H. S. Teoh wrote:



 But don't you still need to manually cleanup in the case of ctor
 failure? AFAIK, the dtor is not invoked on the 
 partially-constructed
 object if the ctor throws.


I didn't knew this. You are right, that this brakes my RAII 
scheme and something to work around this would be helpful. That's 
actually quite the pitfall.
Aug 27 2013
prev sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
27-Aug-2013 01:30, H. S. Teoh пишет:

 On Sun, Aug 25, 2013 at 12:18:27AM +0200, Tobias Pankrath wrote:

 On Saturday, 24 August 2013 at 20:11:14 UTC, H. S. Teoh wrote:

 How would you use RAII to solve this problem? If I have a class:

 	class C {
 		Resource1 res1;
 		Resource2 res2;
 		Resource3 res3;
 		this() {
 			...
 		}
 	}

 How would you write the ctor with RAII such that if it successfully
 inits res1 and res2, but throws before it inits res3, then only res1
 and res2 will be cleaned up?


 Like someone else already proposed: Using a wrapper type W that
 releases the resources in it's destructor. W.init wouldn't release
 anything. So by definition (if I recall the rules correctly) every
 new instance of C would have res3 = Resource3.init prior to it's
 constructor called.

 Now make sure that a) res3's destructor gets called (check) b)
 res3's destructor may be called on Resource3.init. That would be a
 new rule similar to 'no internal aliasing' and c) that every
 constructor of C either sets res3 to a destructable value or does
 not touch it at all ('transactional programming').


 [...]

 But don't you still need to manually cleanup in the case of ctor
 failure? AFAIK, the dtor is not invoked on the partially-constructed
 object if the ctor throws. So you'd still have to use scope(failure) to
 manually release the resource.

 To prove my point, here is some sample code that (tries to) use RAII to
 cleanup:

 	import std.stdio;
 	
 	struct Resource {
 		int x = 0;
 		this(int id) {
 			x = id;
 			writefln("Acquired resource %d", x);
 		}
 		~this() {
 			if (x == 0)
 				writefln("Empty resource, no cleanup");
 			else
 				writefln("Cleanup resource %d", x);
 		}
 	}
 	
 	struct S {
 		Resource res1;
 		Resource res2;
 		Resource res3;
 	
 		this(int) {
 			res1 = Resource(1);
 			res2 = Resource(2);
 			assert(res1.x == 1);
 			assert(res2.x == 2);
 	
 			throw new Exception("ctor failed!");
 			res3 = Resource(3);	// not reached
 			assert(res3.x == 3);	// not reached
 		}
 	}
 	
 	void main() {
 		try {
 			auto s = S(123);
 		} catch(Exception e) {
 			writeln(e.msg);
 		}
 	}

 Here is the program output:

 	Acquired resource 1
 	Empty resource, no cleanup
 	Acquired resource 2
 	Empty resource, no cleanup
 	ctor failed!

 As you can see, the two resources acquired in the partially-constructed
 object did NOT get cleaned up. So, RAII doesn't work in this case.


Fixed?


 			auto r1 = Resource(1);
 			auto r2 = Resource(2);
 			assert(res1.x == 1);
 			assert(res2.x == 2);
 	
 			throw new Exception("ctor failed!");
 			auto r3 = Resource(3);	// not reached
 			assert(res3.x == 3);	// not reached


			res1 = move(r1);
			res2 = move(r2);
			res3 = move(r3);


Exception-safe code after all has to be exception safe.
Unlike in C++ we have no explicit initialization of members (and strict 
order of this) hence no automatic partial cleanup.

-- 
Dmitry Olshansky
Aug 27 2013
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Aug 27, 2013 at 04:01:57PM +0400, Dmitry Olshansky wrote:

 27-Aug-2013 01:30, H. S. Teoh пишет:

On Sun, Aug 25, 2013 at 12:18:27AM +0200, Tobias Pankrath wrote:

On Saturday, 24 August 2013 at 20:11:14 UTC, H. S. Teoh wrote:

How would you use RAII to solve this problem? If I have a class:

	class C {
		Resource1 res1;
		Resource2 res2;
		Resource3 res3;
		this() {
			...
		}
	}

How would you write the ctor with RAII such that if it successfully
inits res1 and res2, but throws before it inits res3, then only res1
and res2 will be cleaned up?


Like someone else already proposed: Using a wrapper type W that
releases the resources in it's destructor. W.init wouldn't release
anything. So by definition (if I recall the rules correctly) every
new instance of C would have res3 = Resource3.init prior to it's
constructor called.

Now make sure that a) res3's destructor gets called (check) b)
res3's destructor may be called on Resource3.init. That would be a
new rule similar to 'no internal aliasing' and c) that every
constructor of C either sets res3 to a destructable value or does
not touch it at all ('transactional programming').


[...]

But don't you still need to manually cleanup in the case of ctor
failure? AFAIK, the dtor is not invoked on the partially-constructed
object if the ctor throws. So you'd still have to use scope(failure) to
manually release the resource.

To prove my point, here is some sample code that (tries to) use RAII to
cleanup:

	import std.stdio;
	
	struct Resource {
		int x = 0;
		this(int id) {
			x = id;
			writefln("Acquired resource %d", x);
		}
		~this() {
			if (x == 0)
				writefln("Empty resource, no cleanup");
			else
				writefln("Cleanup resource %d", x);
		}
	}
	
	struct S {
		Resource res1;
		Resource res2;
		Resource res3;
	
		this(int) {
			res1 = Resource(1);
			res2 = Resource(2);
			assert(res1.x == 1);
			assert(res2.x == 2);
	
			throw new Exception("ctor failed!");
			res3 = Resource(3);	// not reached
			assert(res3.x == 3);	// not reached
		}
	}
	
	void main() {
		try {
			auto s = S(123);
		} catch(Exception e) {
			writeln(e.msg);
		}
	}

Here is the program output:

	Acquired resource 1
	Empty resource, no cleanup
	Acquired resource 2
	Empty resource, no cleanup
	ctor failed!

As you can see, the two resources acquired in the
partially-constructed object did NOT get cleaned up. So, RAII doesn't
work in this case.


 
 Fixed?
 

 			auto r1 = Resource(1);
 			auto r2 = Resource(2);
 			assert(res1.x == 1);
 			assert(res2.x == 2);
 	
 			throw new Exception("ctor failed!");
 			auto r3 = Resource(3);	// not reached
 			assert(res3.x == 3);	// not reached


 
 			res1 = move(r1);
 			res2 = move(r2);
 			res3 = move(r3);
 
 
 Exception-safe code after all has to be exception safe.
 Unlike in C++ we have no explicit initialization of members (and
 strict order of this) hence no automatic partial cleanup.


[...]

What if move(r2) throws? Then res1 won't get cleaned up, because r1 has
already been nulled by the move.


T

-- 
Without outlines, life would be pointless.
Aug 27 2013
next sibling parent reply "deadalnix" <deadalnix gmail.com> writes:
On Tuesday, 27 August 2013 at 14:26:53 UTC, H. S. Teoh wrote:

 What if move(r2) throws? Then res1 won't get cleaned up, 
 because r1 has
 already been nulled by the move.


I don't think move can throw.
Aug 27 2013
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Aug 27, 2013 at 05:46:40PM +0200, deadalnix wrote:

 On Tuesday, 27 August 2013 at 14:26:53 UTC, H. S. Teoh wrote:

What if move(r2) throws? Then res1 won't get cleaned up, because r1
has already been nulled by the move.


 
 I don't think move can throw.


Well, it's not marked nothrow, so I wouldn't count on that.

Also, the fact that move() uses memcpy is a bit worrying; Adam Ruppe &
myself ran into a nasty bug involving closures over struct members when
the struct may get moved upon return from a function. For example:

	struct S {
		int id;
		void delegate()[] cleanups;

		this() {
			id = acquireResource();
			cleanups ~= {
				// N.B.: closure over this.id
				releaseResource(id);
			};
		}
	}

	S makeS() {
		// Problem: S.cleanups[0] is a closure over the struct
		// instance on this function's stack, but once S is
		// returned, it gets memcpy'd into the caller's stack
		// frame. This invalidates the delegate's context
		// pointer.
		return S(1);
	}

	void main() {
		auto s = makeS();
		// Problem: s.cleanups[0] now has an invalid context
		// pointer. If the stack is reused after this point, the
		// dtor of s will get a garbage value for s.id.
	}

Using move() to move a resource from a local variable into a member
looks like it might be vulnerable to this bug as well -- if the resource
has closures over member variables it might trigger this problem.


T

-- 
To provoke is to call someone stupid; to argue is to call each other stupid.
Aug 27 2013
parent reply "deadalnix" <deadalnix gmail.com> writes:
On Tuesday, 27 August 2013 at 16:57:36 UTC, H. S. Teoh wrote:

 On Tue, Aug 27, 2013 at 05:46:40PM +0200, deadalnix wrote:

 On Tuesday, 27 August 2013 at 14:26:53 UTC, H. S. Teoh wrote:

What if move(r2) throws? Then res1 won't get cleaned up, 
because r1
has already been nulled by the move.


 
 I don't think move can throw.


 Well, it's not marked nothrow, so I wouldn't count on that.

 Also, the fact that move() uses memcpy is a bit worrying; Adam 
 Ruppe &
 myself ran into a nasty bug involving closures over struct 
 members when
 the struct may get moved upon return from a function. For 
 example:

 	struct S {
 		int id;
 		void delegate()[] cleanups;

 		this() {
 			id = acquireResource();
 			cleanups ~= {
 				// N.B.: closure over this.id
 				releaseResource(id);
 			};
 		}
 	}

 	S makeS() {
 		// Problem: S.cleanups[0] is a closure over the struct
 		// instance on this function's stack, but once S is
 		// returned, it gets memcpy'd into the caller's stack
 		// frame. This invalidates the delegate's context
 		// pointer.
 		return S(1);
 	}

 	void main() {
 		auto s = makeS();
 		// Problem: s.cleanups[0] now has an invalid context
 		// pointer. If the stack is reused after this point, the
 		// dtor of s will get a garbage value for s.id.
 	}

 Using move() to move a resource from a local variable into a 
 member
 looks like it might be vulnerable to this bug as well -- if the 
 resource
 has closures over member variables it might trigger this 
 problem.


 T


Funny, I ran into this twice this week XD

struct are movable by definition, so the compiler should reject 
this unless the delegate is scope.
Aug 27 2013
parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Wed, Aug 28, 2013 at 02:13:39AM +0200, deadalnix wrote:

 On Tuesday, 27 August 2013 at 16:57:36 UTC, H. S. Teoh wrote:


[...]

Also, the fact that move() uses memcpy is a bit worrying; Adam Ruppe
& myself ran into a nasty bug involving closures over struct members
when the struct may get moved upon return from a function. For
example:

	struct S {
		int id;
		void delegate()[] cleanups;

		this() {
			id = acquireResource();
			cleanups ~= {
				// N.B.: closure over this.id
				releaseResource(id);
			};
		}
	}

	S makeS() {
		// Problem: S.cleanups[0] is a closure over the struct
		// instance on this function's stack, but once S is
		// returned, it gets memcpy'd into the caller's stack
		// frame. This invalidates the delegate's context
		// pointer.
		return S(1);
	}

	void main() {
		auto s = makeS();
		// Problem: s.cleanups[0] now has an invalid context
		// pointer. If the stack is reused after this point, the
		// dtor of s will get a garbage value for s.id.
	}

Using move() to move a resource from a local variable into a member
looks like it might be vulnerable to this bug as well -- if the
resource has closures over member variables it might trigger this
problem.


T


 
 Funny, I ran into this twice this week XD
 
 struct are movable by definition, so the compiler should reject this
 unless the delegate is scope.


Is scope even implemented right now? :-/ It's one of those things that
are really, really nice to have, but so far haven't materialized yet.


T

-- 
Guns don't kill people. Bullets do.
Aug 27 2013
prev sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
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 :)



It just blits the damn data. Even if this is currently broken..
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.


-- 
Dmitry Olshansky
Aug 27 2013
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
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 :)
 



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.

(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?)

Basically, the only way to be 100% sure is to use scope(this), or the
manual equivalent thereof (see below).



 It just blits the damn data. Even if this is currently broken..


It also calls destroy, which is not nothrow. :)



 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.

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

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

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

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

		~this() { __cleanup(); }

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

Which is basically what DIP44 is proposing under a nicer syntax.


T

-- 
"640K ought to be enough" -- Bill G., 1984. "The Internet is not a primary goal
for PC usage" -- Bill G., 1995. "Linux has no impact on Microsoft's strategy"
-- Bill G., 1999.
Aug 27 2013
parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
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 :)




 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
Aug 27 2013
parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Wed, Aug 28, 2013 at 01:02:27AM +0400, Dmitry Olshansky wrote:

 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 :)




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.


I'm not sure I understand you. Since the compiler tells me that
std.algorithm.move can't be made nothrow, I, from a user's POV, have no
choice but to believe that there are *some* circumstances in which it
*will* throw.



(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.


Yes, but if you could do something today to prevent bugs in the future,
why not?



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.


How is it flawed?



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


Yes, this is a common bug. So it's a real possibility that move() may
throw. And now we know that if File's were involved, it could actually
happen.  ;-)

I'd argue that, independently of DIP44, destroy() should be marked
nothrow:

	void destroy(T)(ref T t) nothrow {
		...
		try {
			callDtor(t);
			// or whatever the syntax is, I don't remember
			// now.
		} catch(Error e) {
			// If we get here, we're royally screwed anyway,
			// so might as well give up.
			assert(0);
		}
	}

I mean, if an object is going out of scope or being GC'd and the dtor is
invoked, and the dtor throws an exception, then what you are gonna do?
Just catch the exception and try to destroy the object again? You can't,
because by the time the stack unwinds the object doesn't exist anymore.

Or, in the case of move(), the code cannot continue because it needs to
destroy the previous object but the destruction failed, so what is the
catcher going to do? It may not have access to the scope in which the
object is defined, so it can't possibly do any real cleanup. And if the
object was created in a scope below the catch block, the unwinding code
will encounter the dtor exception a second time... in any case, we're
completely screwed so there's no point in continuing.

Once move() is nothrow, then I agree you have a point that you can
initialize resources as ctor local variables first, then assign them to
member fields.

Ultimately, though, in machine code it's not really any different from
putting scope(failure) __cleanup() in the ctor. Just the same solution
implemented in different ways.



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.


Does the compiler keep track of what has been initialized? If it does,
then I agree that it should call dtors on "cooked" fields on ctor
failure. That would eliminate a large part of the reason for DIP44.

As it stands, though, partially-initialized objects are just left as-is,
with no attempt to cleanup (as I've shown in a previous post). The only
way around this currently is to manually keep track of what has been
initialized and use scope(failure) (or equivalent) to cleanup.



 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:)


Well, OK, so make the definition of __cleanups nothrow:

	alias CleanupFunc = void delegate() nothrow;
	CleanupFunc[] __cleanups;

Then the compiler can statically reject any cleanup code that may throw.
Currently, I can't say the same for std.algorithm.move().


T

-- 
Маленькие детки - маленькие бедки.
Aug 27 2013
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Aug 26, 2013 at 02:30:09PM -0700, H. S. Teoh wrote:

 On Sun, Aug 25, 2013 at 12:18:27AM +0200, Tobias Pankrath wrote:

 On Saturday, 24 August 2013 at 20:11:14 UTC, H. S. Teoh wrote:

How would you use RAII to solve this problem? If I have a class:

	class C {
		Resource1 res1;
		Resource2 res2;
		Resource3 res3;
		this() {
			...
		}
	}

How would you write the ctor with RAII such that if it successfully
inits res1 and res2, but throws before it inits res3, then only res1
and res2 will be cleaned up?


 
 Like someone else already proposed: Using a wrapper type W that
 releases the resources in it's destructor. W.init wouldn't release
 anything. So by definition (if I recall the rules correctly) every
 new instance of C would have res3 = Resource3.init prior to it's
 constructor called.
 
 Now make sure that a) res3's destructor gets called (check) b)
 res3's destructor may be called on Resource3.init. That would be a
 new rule similar to 'no internal aliasing' and c) that every
 constructor of C either sets res3 to a destructable value or does
 not touch it at all ('transactional programming').


 [...]
 
 But don't you still need to manually cleanup in the case of ctor
 failure? AFAIK, the dtor is not invoked on the partially-constructed
 object if the ctor throws. So you'd still have to use scope(failure) to
 manually release the resource.
 
 To prove my point, here is some sample code that (tries to) use RAII to
 cleanup:
 
 	import std.stdio;
 	
 	struct Resource {
 		int x = 0;
 		this(int id) {
 			x = id;
 			writefln("Acquired resource %d", x);
 		}
 		~this() {
 			if (x == 0)
 				writefln("Empty resource, no cleanup");
 			else
 				writefln("Cleanup resource %d", x);
 		}
 	}
 	
 	struct S {
 		Resource res1;
 		Resource res2;
 		Resource res3;
 	
 		this(int) {
 			res1 = Resource(1);
 			res2 = Resource(2);
 			assert(res1.x == 1);
 			assert(res2.x == 2);
 	
 			throw new Exception("ctor failed!");
 			res3 = Resource(3);	// not reached
 			assert(res3.x == 3);	// not reached
 		}
 	}
 	
 	void main() {
 		try {
 			auto s = S(123);
 		} catch(Exception e) {
 			writeln(e.msg);
 		}
 	}
 
 Here is the program output:
 
 	Acquired resource 1
 	Empty resource, no cleanup
 	Acquired resource 2
 	Empty resource, no cleanup
 	ctor failed!
 
 As you can see, the two resources acquired in the partially-constructed
 object did NOT get cleaned up. So, RAII doesn't work in this case.
 
 The two interspersed cleanups were presumably cause by Resource.init
 being destructed when res1 and res2 were assigned to. But after being
 assigned to, res1 and res2's dtors were NOT invoked.
 
 So I think my scope(this) idea has some merit, since it will correctly
 handle this case. :)


[...]

P.S. If you comment out the throw line, you will see that the resources
are released correctly. So, RAII lets us handle automatic cleanup
without needing an explicit dtor, but it (currently) does NOT correctly
handle cleanup for the partially-constructed object case.

To solve this problem, basically the compiler will have to keep track of
which fields have been initialized already, and insert scope(failure)
statements for them in the ctor. But once you have that, you have
basically already implemented scope(this), so might as well go all the
way and give it a nice syntax.


T

-- 
Let's not fight disease by killing the patient. -- Sean 'Shaleh' Perry
Aug 26 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/24/2013 1:09 PM, H. S. Teoh wrote:

 On Sat, Aug 24, 2013 at 12:27:37PM -0700, Walter Bright wrote:
 [...]

 Not a bad idea, but it has some issues:

 1. scope(failure) takes care of most of it already

 2. I'm not sure this is a problem that needs solving, as the DIP
 points out, these issues are already easily dealt with. We should be
 conservative about adding more syntax.

 3. What if the destructor needs to do more than just unwind the
 transactions? Where does that code fit in?


 I think it's unhelpful to conflate scope(this) with dtors. They are
 related, but -- and I guess I was a bit too strong about saying dtors
 are redundant -- if we allow both, then scope(this) can be reserved for
 transactions, and you can still put code in ~this() to do non-trivial
 cleanups.


If you take out automatic dtor generation, I see no difference between 
scope(this) and scope(failure).



 4. The worst issue is the DIP assumes there is only one constructor,
 from which the destructor is inferred. What if there is more than
 one constructor?


 This is not a problem. If there is more than one constructor, then only
 those scope(this) statements in the ctor that were actually encountered
 will trigger when the object reaches the end of its lifetime.


Do you mean multiple dtors will be generated, one for each constructor?
Aug 24 2013
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Aug 24, 2013 at 06:50:11PM -0700, Walter Bright wrote:

 On 8/24/2013 1:09 PM, H. S. Teoh wrote:

On Sat, Aug 24, 2013 at 12:27:37PM -0700, Walter Bright wrote:
[...]

Not a bad idea, but it has some issues:

1. scope(failure) takes care of most of it already

2. I'm not sure this is a problem that needs solving, as the DIP
points out, these issues are already easily dealt with. We should be
conservative about adding more syntax.

3. What if the destructor needs to do more than just unwind the
transactions? Where does that code fit in?


I think it's unhelpful to conflate scope(this) with dtors. They are
related, but -- and I guess I was a bit too strong about saying dtors
are redundant -- if we allow both, then scope(this) can be reserved
for transactions, and you can still put code in ~this() to do
non-trivial cleanups.


 
 If you take out automatic dtor generation, I see no difference
 between scope(this) and scope(failure).
 
 

4. The worst issue is the DIP assumes there is only one constructor,
from which the destructor is inferred. What if there is more than
one constructor?


This is not a problem. If there is more than one constructor, then
only those scope(this) statements in the ctor that were actually
encountered will trigger when the object reaches the end of its
lifetime.


 
 Do you mean multiple dtors will be generated, one for each
 constructor?


No. Given this code:

	class C {
		this(int) {
			scope(this) writeln("A1");
			scope(this) writeln("A2");
		}

		this(float) {
			scope(this) writeln("B1");
			scope(this) writeln("B2");
		}
	}

The lowered code looks something like this:

	class C {
		this(int) {
			scope(failure) __cleanup();

			// scope(this) writeln("A1");
			// Translated into:
			__cleanups ~= { writeln("A1"); };

			// scope(this) writeln("A2");
			// Translated into:
			__cleanups ~= { writeln("A2"); };
		}

		this(float) {
			scope(failure) __cleanup();

			// scope(this) writeln("B1");
			// Translated into:
			__cleanups ~= { writeln("B1"); };

			// scope(this) writeln("B2");
			// Translated into:
			__cleanups ~= { writeln("B2"); };
		}

		void delegate()[] __cleanups;

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

		~this() {
			__cleanup();
		}
	}

So, there is only one dtor. But it automatically takes handles
triggering the scope(this) statements of only the ctor that was actually
used for creating the object. And if the ctor didn't successfully
complete, it only triggers the scope(this) statements that have been
encountered up to that point.

Of course, the above lowered code is just to illustrate how it works.
The actual implementation can be optimized by the compiler. E.g., if
there is only one ctor and the sequence of scope(this) can be statically
determined, then the cleanup statements can be put into the dtor
directly without incurring the overhead of allocating the __cleanups
array. If the cleanups don't need closure over ctor local variables,
then they can just be function ptrs, not delegates. Only when you're
doing something complicated do you actually need an array of delegates,
which should be relatively rare.


T

-- 
Political correctness: socially-sanctioned hypocrisy.
Aug 24 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/24/2013 10:35 PM, H. S. Teoh wrote:

 On Sat, Aug 24, 2013 at 06:50:11PM -0700, Walter Bright wrote:

 On 8/24/2013 1:09 PM, H. S. Teoh wrote:

 On Sat, Aug 24, 2013 at 12:27:37PM -0700, Walter Bright wrote:
 [...]

 Not a bad idea, but it has some issues:

 1. scope(failure) takes care of most of it already

 2. I'm not sure this is a problem that needs solving, as the DIP
 points out, these issues are already easily dealt with. We should be
 conservative about adding more syntax.

 3. What if the destructor needs to do more than just unwind the
 transactions? Where does that code fit in?


 I think it's unhelpful to conflate scope(this) with dtors. They are
 related, but -- and I guess I was a bit too strong about saying dtors
 are redundant -- if we allow both, then scope(this) can be reserved
 for transactions, and you can still put code in ~this() to do
 non-trivial cleanups.


 If you take out automatic dtor generation, I see no difference
 between scope(this) and scope(failure).



 4. The worst issue is the DIP assumes there is only one constructor,
from which the destructor is inferred. What if there is more than
 one constructor?


 This is not a problem. If there is more than one constructor, then
 only those scope(this) statements in the ctor that were actually
 encountered will trigger when the object reaches the end of its
 lifetime.


 Do you mean multiple dtors will be generated, one for each
 constructor?


 No. Given this code:

 	class C {
 		this(int) {
 			scope(this) writeln("A1");
 			scope(this) writeln("A2");
 		}

 		this(float) {
 			scope(this) writeln("B1");
 			scope(this) writeln("B2");
 		}
 	}

 The lowered code looks something like this:

 	class C {
 		this(int) {
 			scope(failure) __cleanup();

 			// scope(this) writeln("A1");
 			// Translated into:
 			__cleanups ~= { writeln("A1"); };

 			// scope(this) writeln("A2");
 			// Translated into:
 			__cleanups ~= { writeln("A2"); };
 		}

 		this(float) {
 			scope(failure) __cleanup();

 			// scope(this) writeln("B1");
 			// Translated into:
 			__cleanups ~= { writeln("B1"); };

 			// scope(this) writeln("B2");
 			// Translated into:
 			__cleanups ~= { writeln("B2"); };
 		}

 		void delegate()[] __cleanups;

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

 		~this() {
 			__cleanup();
 		}
 	}

 So, there is only one dtor. But it automatically takes handles
 triggering the scope(this) statements of only the ctor that was actually
 used for creating the object. And if the ctor didn't successfully
 complete, it only triggers the scope(this) statements that have been
 encountered up to that point.

 Of course, the above lowered code is just to illustrate how it works.
 The actual implementation can be optimized by the compiler. E.g., if
 there is only one ctor and the sequence of scope(this) can be statically
 determined, then the cleanup statements can be put into the dtor
 directly without incurring the overhead of allocating the __cleanups
 array. If the cleanups don't need closure over ctor local variables,
 then they can just be function ptrs, not delegates. Only when you're
 doing something complicated do you actually need an array of delegates,
 which should be relatively rare.


Your example, again, is of an auto-generated dtor. But you said earlier that 
wasn't the point.

Without the auto-generated dtor, it is just scope(failure), which is already a
D 
feature.

I don't get it.
Aug 24 2013
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Aug 24, 2013 at 11:30:26PM -0700, Walter Bright wrote:
[...]

 Your example, again, is of an auto-generated dtor. But you said
 earlier that wasn't the point.
 
 Without the auto-generated dtor, it is just scope(failure), which is
 already a D feature.
 
 I don't get it.


It's a way to avoid code duplication in the dtor. If you just had
scope(failure), you have to factor out a __cleanup method in order to do
cleanup on both a failed ctor call or a dtor call, as I did in the
example.

I don't know if it will clarify things, but if you already have a dtor,
then scope(this) just adds to it:

	class C {
		this() {
			scope(this) writeln("A");
			scope(this) writeln("B");
		}
		~this() {
			writeln("In dtor");
		}
	}

This would get lowered to the equivalent of:

	class C {
		this() {
			scope(failure) __cleanup();
			__cleanups ~= { writeln("A"); };
			__cleanups ~= { writeln("B"); };
		}

		void delegate()[] __cleanups;

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

		~this() {
			writeln("In dtor");
			__cleanup();
		}
	}


T

-- 
Skill without imagination is craftsmanship and gives us many useful
objects such as wickerwork picnic baskets.  Imagination without skill
gives us modern art. -- Tom Stoppard
Aug 25 2013
parent Walter Bright <newshound2 digitalmars.com> writes:
On 8/25/2013 12:14 AM, H. S. Teoh wrote:
[...]

Ok, you had thrown me off by saying it wasn't about dtors. It is all about
dtors 
:-) I think it is an implementable proposal.

However, I tend to agree with Andrei's comment on the utility of it.
Aug 25 2013