You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I've tried a few experiments in refactoring #projectedUpdate#572 and #570. Both pass the current tests, but I'm not convinced they would pass more complicated tests, in fact I'm not convinced the current algorithm would pass more complicated tests.
Specifically, if we have a mutable field inside an immutable ref, for example if using "Interior Mutability" via RefCell, we should allow writes to the internal field. The current logic for computing projections will stop as soon as it hits its first immutable marker.
Another issue I see is that #projectedUpdate is really doing two things, (i) it's first finding the location that should be written to, which can change via the projectionElemDeref. This projectionElemDeref is also used for #readProjection and basically can switch which different location (<locals> or <stack> for example) is being dereferenced. In the case we hit one of these, we actually throw out the entire context we've been computing thus far in #projectedUpdate, and start over on the new location.
In summary, I think that:
We don't need to compute contexts until we're on the last segment of the ProjectionElems, where a "segment" is defined as section between projectionElemDeref markers.
We don't need to check mutability until we're on the very last lookup, it's OK to step through immutable elements to reach the inner mutable that we are actually update. @dkcumming has offered to come up with some tests of inner mutability so we can test that (on the current implementation, and on updates).
As a refactoring, I would suggest:
Implement syntax MaybeValueAndContext ::= #lookupFieldWithCtx(TypedValue, ProjectionElems), which handles all of the ProjectionElem, but if it has any ProjectionElem after a projectionElemDeref, it returns a None. If there is a type-mismatch in the projection and the typed value it's looking at, it will also return None. On successful lookup, it returns both a Value (the contents at the lookup path), and a Context (the information needed to reconstruct the original surrounding value).
First break the ProjectionElems into "segments", where each segment contains all the things between the projectionElemDeref (and does include the last projectionElemDeref). Then for each segment (except the last), call #lookupField(...) on the appropriate field to reach in and get the Reference(...) nested in there. Repeat until we're at the last segment. For the last segment, again call #lookupFieldWithCtx(...) and get back both the value and the context of it. There, do the mutability check on just that value, and if that passes, then use the context to build up the new overall value (with the mutated interior), and write it back to the location it came from.
The text was updated successfully, but these errors were encountered:
I've tried a few experiments in refactoring
#projectedUpdate
#572 and #570. Both pass the current tests, but I'm not convinced they would pass more complicated tests, in fact I'm not convinced the current algorithm would pass more complicated tests.Specifically, if we have a
mutable
field inside animmutable
ref, for example if using "Interior Mutability" viaRefCell
, we should allow writes to the internal field. The current logic for computing projections will stop as soon as it hits its firstimmutable
marker.Another issue I see is that
#projectedUpdate
is really doing two things, (i) it's first finding the location that should be written to, which can change via theprojectionElemDeref
. ThisprojectionElemDeref
is also used for#readProjection
and basically can switch which different location (<locals>
or<stack>
for example) is being dereferenced. In the case we hit one of these, we actually throw out the entire context we've been computing thus far in#projectedUpdate
, and start over on the new location.In summary, I think that:
ProjectionElems
, where a "segment" is defined as section betweenprojectionElemDeref
markers.As a refactoring, I would suggest:
syntax MaybeValueAndContext ::= #lookupFieldWithCtx(TypedValue, ProjectionElems)
, which handles all of theProjectionElem
, but if it has anyProjectionElem
after aprojectionElemDeref
, it returns aNone
. If there is a type-mismatch in the projection and the typed value it's looking at, it will also returnNone
. On successful lookup, it returns both aValue
(the contents at the lookup path), and aContext
(the information needed to reconstruct the original surrounding value).ProjectionElems
into "segments", where each segment contains all the things between theprojectionElemDeref
(and does include the lastprojectionElemDeref
). Then for each segment (except the last), call#lookupField(...)
on the appropriate field to reach in and get theReference(...)
nested in there. Repeat until we're at the last segment. For the last segment, again call#lookupFieldWithCtx(...)
and get back both the value and the context of it. There, do the mutability check on just that value, and if that passes, then use the context to build up the new overall value (with the mutated interior), and write it back to the location it came from.The text was updated successfully, but these errors were encountered: