www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 5058] New: invariant() should not be called before opAssign()

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058

           Summary: invariant() should not be called before opAssign()
           Product: D
           Version: unspecified
          Platform: Other
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: jmdavisProg gmx.com



PDT ---
As it stands, the invariant is called before every public function is called
and after every public function is called. On the whole, this is correct
behavior. However, there is at least one case where this is undesirable:
opAssign().

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. So, the invariant fails and an AssertError is
thrown. Since, opAssign() is theoretically supposed to completely replace the
state of the object, there's no need for the object to be in a correct state
prior to opAssign() being called. It obviously needs to be in a correct state
afterwards, and if the invariant is run afterwards, it will catch if opAssign()
did not correctly replace the state of the object such that it no longer
violates its invariant. However, there's no need to call the invariant before
opAssign() is called. The state prior to opAssign() is irrelevant, and calling
the invariant just makes it really hard to have invariants in a struct where
you can't make it so that the init value for that struct doesn't violate its
invariant.

So, _please_ make it so that the invariant is _not_ called prior to opAssign()
being called.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 15 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug yahoo.com.au



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: -------
Oct 27 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058




PDT ---
The problem is that that then seriously reduces the useability of struct
invariants. At least if init violates the invariant, then the invariant will
fail if you used init rather than properly initializing a variable of that
struct type.

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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 27 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058


Peter Alexander <peter.alexander.au gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |peter.alexander.au gmail.co
                   |                            |m



12:02:23 PDT ---


 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: -------
Oct 27 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058




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: -------
Oct 27 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058


Steven Schveighoffer <schveiguy yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy yahoo.com



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: -------
Oct 27 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058




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: -------
Oct 27 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058




23:52:41 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.


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: -------
Oct 27 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058




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: -------
Nov 01 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058




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: -------
Nov 01 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         OS/Version|Linux                       |All



PST ---

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

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: -------
Jan 31 2011
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5058


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla digitalmars.com
         Resolution|                            |INVALID



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: -------
Jan 23 2012