Fix returnCount arity for sum-returning HULL functions#486
Conversation
returnCount in Language/Hull/TcEnv.hs collapsed every non-TFun/TPair type to a single word slot, so a sum-returning function called from a user asm block passed HULL's YAssign/YLet arity check (1 == 1). yule's sizeOf, however, lowers a TSum return as `1 + max (sizeOf t1) (sizeOf t2)` words, emitting a multi-word Yul return into a single-slot LHS that solc later rejects. Make returnCount mirror yule's sizeOf exactly by adding explicit TSum, TSumN and TBool cases (and an explicit TFun error), so the asm arity check agrees with the eventual Yul lowering. Add a Hull type-checker test (11-err-asm-sum-return.hull) that assigns a `word + word` returning function into a single word in an assembly block; it now correctly fails the return-count check. Co-Authored-By: Alex Beregszaszi <alex@rtfs.hu>
| returnCount TBool = 1 | ||
| returnCount (TPair a b) = returnCount a + returnCount b | ||
| returnCount (TSum a b) = 1 + max (returnCount a) (returnCount b) | ||
| returnCount (TSumN ts) = 1 + maximum (map returnCount ts) |
There was a problem hiding this comment.
Not sure if these are correct?
I think the error below should be enough?
There was a problem hiding this comment.
Compressed sums can change the outcome and with the current pipeline we have no way of knowing whether the compression will be enabled or not.
See test/examples/spec/037dwarves.solc for a bigger sum example (compile with -s -g --pe-fuel 0 to see sums in the Hull output).
There is also the comment
-- Collapsing them to 1 here lets a sum-returning function be assigned into a
-- single-word asm LHS that yule then lowers to a multi-word Yul return.
(from the original code, but unchanged in this PR) which I do not understand, @rodrigogribeiro, can you help?
There was a problem hiding this comment.
Having thought about this for a while, I think a proper fix might be to remove unoptimised sums from yule and change the returnCount formula or TNamed appropriately.
There was a problem hiding this comment.
I my view, the returnCount implementation for TSum and TSumN makes sense. Is there any problem that I was not able to perceive?
There was a problem hiding this comment.
I was worried that compressing sums may break this arithmetic, but compression actually runs before compression, so this is fine. Sorry.
rodrigogribeiro
left a comment
rodrigogribeiro
left a comment
There was a problem hiding this comment.
I believe that this change will fix a problem that I've found on implementing generic deriving. Nice catch!
4 participants
returnCount in Language/Hull/TcEnv.hs collapsed every non-TFun/TPair type to a single word slot, so a sum-returning function called from a user asm block passed HULL's YAssign/YLet arity check (1 == 1). yule's sizeOf, however, lowers a TSum return as
1 + max (sizeOf t1) (sizeOf t2)words, emitting a multi-word Yul return into a single-slot LHS that solc later rejects.Make returnCount mirror yule's sizeOf exactly by adding explicit TSum, TSumN and TBool cases (and an explicit TFun error), so the asm arity check agrees with the eventual Yul lowering.
Add a Hull type-checker test (11-err-asm-sum-return.hull) that assigns a
word + wordreturning function into a single word in an assembly block; it now correctly fails the return-count check.