fix[lang]: abi-encode with expr CompilerPanic#5154
Conversation
Gas ChangesNo changes detected. Summary
|
📊 Bytecode Size Changes (venom)No changes detected. Full bytecode sizes
|
| assert compiler.compile_code(good_code) is not None | ||
|
|
||
|
|
||
| # UnfoldableNode on legacy, CompilerPanic in venom |
There was a problem hiding this comment.
where is the compiler panic from?
There was a problem hiding this comment.
CompilerPanic: unfoldable boolean kwarg: unwrap_tuple
From _get_bool_kwarg in misc.py
I tried to make it raise an UnfoldableNode as well, but actually the proper fix is to fix #5153
The front-end checks that this is a Constant, so we should never be in a situation were we need to raise in the first place
There was a problem hiding this comment.
can we fix the venom pipeline too so that it doesn't panic? what is the point of "fixing" the panic in one pipeline but not the other?
There was a problem hiding this comment.
This PR fixes the panic on both backends for cases like:
_abi_decode(x, uint256, unwrap_tuple=(0 < 1))This xfail test is instead due to #5153
I did not want to burden this PR with some codegen changes just to change the type of VyperInternalException which is reported
There was a problem hiding this comment.
Claude-authored explanation of why this happens here (and not on abi_encode):
Semantic analysis passes cleanly, and the kwarg is only consumed at codegen:
- Legacy codegen: @process_inputs → process_kwarg (vyper/builtins/_signatures.py:37) calls kwarg_node.get_folded_value().value → UnfoldableNode (same underlying failure, just later).
- Venom codegen: vyper/codegen_venom/builtins/abi.py:36-48's _get_bool_kwarg uses reduced() (which returns self when there's no folded value, no exception), then does an isinstance(kw_node, vy_ast.NameConstant | vy_ast.Int) check. empty(bool) is a Call, falls through, and hits the explicit raise CompilerPanic("unfoldable boolean kwarg: …") at line 48.
This reverts commit 7aad857.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39e1b340fe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| assert compiler.compile_code(good_code) is not None | ||
|
|
||
|
|
||
| @pytest.mark.xfail(raises=UnfoldableNode) |
There was a problem hiding this comment.
When the CI matrix runs the experimental pipeline (.github/workflows/test.yml includes --experimental-codegen), this new test reaches venom's _get_bool_kwarg path for empty(bool), which raises CompilerPanic for an unreduced boolean kwarg; the adjacent decode test handles that with raises=(UnfoldableNode, CompilerPanic), but this xfail only permits UnfoldableNode. As a result, the newly added encode regression test fails in every experimental-codegen job instead of being reported as xfailed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is empirically wrong, the venom tests pass
2 participants
What I did
Fix #5144
But still emits user-visible
NotFoldabledue to #5153How I did it
How to verify it
Commit message
Description for the changelog
Cute Animal Picture