[Issue 5058] invariant() should not be called before opAssign()
https://issues.dlang.org/show_bug.cgi?id=5058 Andrei Alexandrescu changed: What|Removed |Added Version|unspecified |D2 --
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 Walter Bright changed: What|Removed |Added Status|NEW |RESOLVED CC||bugzi...@digitalmars.com Resolution||INVALID --- Comment #11 from Walter Bright 2012-01-23 23:41:34 PST --- An invariant should be written so that .init passes. Anything else would thoroughly break how D initializes objects. This is not a bug, it is as designed. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 Jonathan M Davis changed: What|Removed |Added OS/Version|Linux |All --- Comment #10 from Jonathan M Davis 2011-01-31 17:44:55 PST --- I believe that bug# 5500 is caused by this issue. Appender has uninitialized data that it assigns to. And since the invariant is called before opAssign is, the invariant is called on garbage data which may or may not pass the invariant. Given that opAssign is supposed to completely replace the state of the object (as it is doing in the case of bug# 5500), I really do think that the invariant should not be called before opAssign is called. It's easy to have a struct which actually passes its invariant in its init state which fails the invariant upon opAssign, because the memory was initialized to void or otherwise not initialized when allocated. And stuff like appender is going to do that quite legitimately and safely. So, I think that bug# 5500 is more evidence that the invariant should _not_ be called before opAssign is called. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 --- Comment #9 from Jonathan M Davis 2010-11-01 13:10:01 PDT --- Generally, I would agree with you. However, it's quite easy to have a struct which has things which _should_ be checked in the invariant but which aren't true with T.init - either because it's stuff (like class objects) which cannot be created with CTFE or it requires that stuff be done at runtime to set up properly. The real problem there is that T.init doesn't go through any constructor. Personally, I hate the fact that T.init exists as it does for structs, but apparently no one has come up with a good enough solution to allow proper default constructors or to disallow default construction entirely for a struct (both of which would ideally be possible). So, for the most part, I'm not trying to check that the user is using the struct properly, but I _would_ like to be able to make the invariant fail if they use T.init for a struct which should never have T.init used. As for segfaults resulting in a stack trace, I've often seen them not result in stack traces, but I don't know if that's because of the segfault being in C code rather than D code or because it was an older version of dmd. Regardless, I'd prefer to be able to have the invariant complain as soon as they try to use the struct rather than later on when they happen to use one of its functions which would result in a segfault. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 --- Comment #8 from Steven Schveighoffer 2010-11-01 12:47:25 PDT --- I look at invariants differently than you do I guess. To me, an invariant is a self-checking mechanism that says "Every public function is going to leave this item in a sane state, and therefore, every public function should expect this to be in a sane state". It should not be possible for a user who is using a struct to break an invariant unless they violate the type system. To me, T.init should always pass the invariant because the user is allowed to declare: T t; And this is guaranteed by the language. Therefore, it's always part of the public interface. In other words, invariants are more to protect the user against the struct misbehaving, not to protect the struct against the user misbehaving. For example, a poorly constructed invariant: struct S { public int i; invariant() { assert(i == 0); } } This looks to me like what you are doing -- ensuring the user has not mucked with your struct data. What an invariant really should do is ensure that the struct has not mucked with the struct data. If the user does it, they do so at their own risk. It would be like expecting the invariant of a class to assert the reference is not null first. And really, what is the difference between a segfault and an assert error? Both should halt execution, and both should print out a valid stack trace. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 --- Comment #7 from Peter Alexander 2010-10-27 23:52:41 PDT --- (In reply to comment #4) > All it takes is assigning to a public member variable or a reference to > private > member data, and you can invalidate an invariant. Granted, having an invariant > with a type where you can do that, isn't necessarily the best idea, but it's > quite possible to have a type where you have an invariant and it's invalid > before opAssign() (or any public function) is called, and, unlike most public > functions, there's no need for the invariant to be true before opAssign() is > called. 1. The whole point of invariant is to ensure that invariants are always true. If your class allows people to invalidate the invariant by assigning to public member variables then the class is outright broken and you need to fix it. In this case the invariant will fire (before opAssign or any other function), which is correct and desired behaviour. 2. As I explained above, there is a need for the invariant to be true before opAssign in the case where objects have resources to release prior to building up the new value. > Of course, what would really help here would be proper default constructors > for > structs, but no one has been able to figure that one out yet it seems. I agree with this. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 --- Comment #6 from Jonathan M Davis 2010-10-27 22:13:01 PDT --- The problem with that is what if you don't _want_ T.init to pass the invariant? It seems to me that if T.init doesn't pass the invariant, then really, T.init shouldn't be used or should at least be assigned to before it's used. For instance, SysTime in the datetime module that I've been working on has a TimeZone class as a member. It's not possible to properly initialize it at compile time, so you're stuck with SysTime.init with a null TimeZone - which should _never_ happen. Any code which uses SysTime.init _should_ fail the invariant that the TimeZone is non-null. The alternative is segmentation faults when the null TimeZone is used. Granted, making T.init pass T's invariant would solve the opAssign() problem, but I think that it would be bad in the general case. I also think that it would be bad if T.init had to pass the invariant, since then struct invariants become border-line useless in many cases. Now, making it so that the invariant for opAssign() is run normally but not in the case where the value is T.init, then that could work, but I don't like the idea of making T.init pass the invariant in the general case. If default struct constructors were possible, then the problem with SysTime.init and inits for other structs like it could be solved, but we're stuck for now it seems. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 Steven Schveighoffer changed: What|Removed |Added CC||schvei...@yahoo.com --- Comment #5 from Steven Schveighoffer 2010-10-27 19:37:33 PDT --- While I agree technically, the invariant can be required to pass T.init, this does not serve the purpose -- it makes invariants more annoying to write. Would it be feasible to have the invariant check function first do a pre-check to see if the value is T.init, and if so, pass? This way, the user is not bothered with the init case, and it should solve the opAssign issue as well. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 --- Comment #4 from Jonathan M Davis 2010-10-27 15:50:46 PDT --- All it takes is assigning to a public member variable or a reference to private member data, and you can invalidate an invariant. Granted, having an invariant with a type where you can do that, isn't necessarily the best idea, but it's quite possible to have a type where you have an invariant and it's invalid before opAssign() (or any public function) is called, and, unlike most public functions, there's no need for the invariant to be true before opAssign() is called. Of course, what would really help here would be proper default constructors for structs, but no one has been able to figure that one out yet it seems. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 Peter Alexander changed: What|Removed |Added CC||peter.alexander...@gmail.co ||m --- Comment #3 from Peter Alexander 2010-10-27 12:02:23 PDT --- (In reply to comment #2) > Regardless, I don't see why it would matter what the state of the object is > prior to opAssign() being called. That's like caring whether the invariant is > true prior to the constructor call. It matters if the object being assigned to have resources that it needs to free (with the invariant possibly being that a pointer to the resource is non-null). I agree 100% with Don here: .init should satisfy the invariant, which makes this bug into a non-bug (unless you can think of other valid situations where the invariant is broken prior to an opAssign call?) -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5058] invariant() should not be called before opAssign()
http://d.puremagic.com/issues/show_bug.cgi?id=5058 Don changed: What|Removed |Added CC||clugd...@yahoo.com.au --- Comment #1 from Don 2010-10-27 02:58:32 PDT --- I don't think this is a bug. I think the real bug is described in this paragraph: "Because it is quite possible to have struct which violates its invariants (thanks to the whole init vs default constructor fiasco), it's quite easy to have a struct which was default-initialized which you're trying to assign to and which violates the invariant." Which is closely related to bug 519. It should NOT be possible to have a struct which violates its invariant. For all instantiated structs which have an invariant, the compiler should insert a check that .init satisfies the invariant. This only has to be done once per struct (it doesn't need to be checked for each instance of the struct). If the invariant is CTFEable, it could even be checked at compile time. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---