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 :) Rule #1 of move should be is that it doesn't throw. 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 :) 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. (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 :) 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
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 :) 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.

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