Skip to content

Conversation

@Stevengre
Copy link
Contributor

@Stevengre Stevengre commented Nov 28, 2025

@Stevengre Stevengre marked this pull request as draft November 28, 2025 02:34
@Stevengre Stevengre self-assigned this Nov 28, 2025
@Stevengre Stevengre marked this pull request as ready for review November 29, 2025 02:50
| #finishResolvedSPLRc ( Place , MaybeBasicBlockIdx )
| #resolveSPLRcRef ( Value , Place , MaybeBasicBlockIdx )
| #finishResolvedSPLRc ( Place , MaybeBasicBlockIdx )
| #writeThroughRef ( Value , Value ) [seqstrict(2)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the seqstrict to make sense, the 2nd argument has to be an Evaluation not a Value (the Value is already a KResult so there is nothing to evaluate further).

Copy link
Contributor Author

@Stevengre Stevengre Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted the seqstrict. It's always Value when used.

Comment on lines 551 to 573
// Cross-frame write helper for SPL ref cells
rule <k> #writeThroughRef(Reference(0, place(local(I), PROJS), _, _), VAL)
=> #forceSetPlaceValue(place(local(I), PROJS), VAL)
...
</k>
<locals> LOCALS </locals>
requires 0 <=Int I andBool I <Int size(LOCALS)
andBool isTypedLocal(LOCALS[I])
rule <k> #writeThroughRef(Reference(OFFSET, place(local(I), PROJS), _, _), VAL)
=> #traverseProjection(
toStack(OFFSET, local(I)),
#localFromFrame({STACK[OFFSET -Int 1]}:>StackFrame, local(I), OFFSET),
PROJS,
.Contexts
)
~> #writeProjection(VAL)
...
</k>
<stack> STACK </stack>
requires 0 <Int OFFSET
andBool OFFSET <=Int size(STACK)
andBool isStackFrame(STACK[OFFSET -Int 1])
andBool 0 <=Int I
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The #writeThroughRef only operates on Reference values. It seems the WriteTo helper sort from data.md could be used instead of the first Value argument.
Many symbol arguments have changed sort from Place to Value, was this the reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to support borrow mut in a callee function with different height.

Comment on lines 171 to 187
// Adjust references when moving across stack frames
rule #adjustRef(SPLRefCell(PLACE, VAL), OFFSET) => SPLRefCell(#adjustRef(PLACE, OFFSET), #adjustRef(VAL, OFFSET))
rule #adjustRef(SPLDataBorrow(PLACE, VAL), OFFSET) => SPLDataBorrow(#adjustRef(PLACE, OFFSET), #adjustRef(VAL, OFFSET))
rule #adjustRef(SPLDataBorrowMut(PLACE, VAL), OFFSET) => SPLDataBorrowMut(#adjustRef(PLACE, OFFSET), #adjustRef(VAL, OFFSET))
rule #adjustRef(SPLDataBuffer(VAL), OFFSET) => SPLDataBuffer(#adjustRef(VAL, OFFSET))
rule #adjustRef(SPLPubkeyRef(VAL), OFFSET) => SPLPubkeyRef(#adjustRef(VAL, OFFSET))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PLACE variables have changed sort to Value so they should probably not be called PLACE any more.
Could we implement #adjustRef for SPL* values directly instead of recurring to a Value? The Place will probably always have a known shape, but I don't know if double references (Deref in the projection) can be excluded. Maybe the Place should become a WrteTo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we implement #adjustRef for SPL* values directly instead of recurring to a Value?

Done!

Maybe the Place should become a WrteTo?

WriteTo cannot hold the projections.

The Place will probably always have a known shape, but I don't know if double references (Deref in the projection) can be excluded.

I don't quite understand this.

@Stevengre Stevengre force-pushed the jh/spl/fix-traverse branch from 0a522e7 to 51eb312 Compare December 1, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants