@Araq, as I have mentioned in other posts, I am trying to apply some of the new
memory management ideas under the current version 0.20.0 system, with problems
as follows:
1\. The "old" closures aren't compatible with what closures will be as they
rely on a (local) GC'ed ref to an environment structure, but one can work
around that by wrapping "old" closures in an object with a sidecar ForeignCell
field which contains the state of protect called on the environment pointer
when created and dispose called on this field for destruction, thus making it
look like the new closures that will rely on owned ref to do this. This is a
kludgy but viable work around which should mean there are minimal changes when
the new closures are available.
2\. My major problem is due to the different syntax and use of the old =sink
versus the new =move as per the new destructor spec 2:
> under the old design so called "self assignments" could not work.
> Consequence: sink parameters for objects that have a non-trivial destructor
> must be passed as by-pointer under the hood. A further advantage is that
> parameters are never destroyed, only variables are. The caller's location
> passed to a sink parameter has to be destroyed by the caller and does not
> burden the callee. (with the following problem example:)
from ranom import randomize, rand
proc select(cond: bool; a, b: sink string): string =
if cond:
result = a # moves a into result
else:
result = b # moves b into result
proc main =
var x = "abc"
var y = "xyz"
# possible self-assignment:
x = select(rand(1.0) < 0.5, x, y)
# 'select' must communicate what parameter has been
# consumed. We cannot simply generate:
# (select(...); wasMoved(x); wasMoved(y))
# NOTE, WE DON'T HAVE TO; IT JUST WORKS...
Run
However, it seems that things have been fixed so the above example does work in
Nim version 0.20.0 with the compiler using a combination of the hooks and
ismoved called on the parameter that is used and not on the other one; the main
difference between as it works now and as proposed in Destructors spec 2 seems
to be the different type signature on =sink than =move with the current one
having a var only on the destination parameter where the second has a var on
both parameters, and what the compiler is doing underneath rather than
combining the destination destruction and source resetting in the assignment
and moving "hooks"
The new =move[T](dst, src: var T) not available yet has a different type
signature than the =sink[T](dst: var T; src: T) it functionally replaces due to
having to modify its source to reset it to show that it has been transferred
(an implicit move); As these are automatically invoked when ownership is
transferred under move semantics along with "cheats" so that the var parameter
can be applied to a let variable, the only work around is to make all bindings
to which we want to apply it var's and not let's and manually invoke a move on
the source of the sink parameter when in the sink proc.
I don't know enough about the implementation details to know what is the
easiest way, but it seems to me that a decision should be made whether to use
the current =sink syntax or the new =move syntax (along with the changed
definition of all of the "hooks") so that there aren't breaking changes forward.
In other words, it seems to me that the new Destructors spec 2 has two distinct
parts as follows:
1. Redefinition of the move semantics and syntax
2. Possible implementation of the new non-GC'ed ref T and owned ref T
If implemented as per the spec, the first would be a breaking change due to the
different signature of the =move hook and I would recommend that, if desired,
it be included pre-version 1.0. This doesn't seem to be risky at all as the
functionality seems to be the same as currently, although it could break some
libraries due to the different syntax.
The second isn't very much a breaking change other than making such kludges as
are necessary to make closures work across threads no longer be necessary and
requiring an occasional owned, and as you say the compiler will guide where
those are necessary. As this is relatively untested as yet, it likely should be
deferred to a later point upgrade version or version 2.0.
If somehow development of the second could be accelerated to be included in
version 1.0 if it works, it would make me happy ;-)
