refactor[codegen]: refactor venom builtins for uniformity and cleanliness#5001
refactor[codegen]: refactor venom builtins for uniformity and cleanliness#5001banteg wants to merge 15 commits into
Conversation
|
hmm, i think the point is actually that
the keys of |
|
Pushed a normal follow-up commit for source order, no force push. Runtime kwargs are now lowered by walking source-order node.keywords, and raw_create/create_from_blueprint materialize positional constructor args before keyword expressions. I also converted the existing raw_create source-order xfails into Venom-passing tests and added raw_call gas/value kwarg order coverage. |
|
Addressed in 9e9070f. Changes:
I also checked the Venom builtin tree after the refactor; the only direct node.keywords iteration left is inside _kwargs.py. extract32(output_type=...) is type-only in semantic analysis and has no Venom runtime lowering. Validation:
|
charles-cooper
changed the title
charles-cooper
changed the title
|
@banteg please fix merge conflicts |
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class BuiltinCall: |
There was a problem hiding this comment.
represents a builtin callsite with additional metadata
There was a problem hiding this comment.
Added a BuiltinCall docstring describing it as the builtin callsite plus the codegen context used to lower it.
| from vyper.codegen_venom.expr import Expr | ||
|
|
||
| arg_nodes = self.node.args if arg_nodes is None else arg_nodes | ||
| return [Expr(arg, self.ctx).lower() for arg in arg_nodes] |
There was a problem hiding this comment.
lower() or lower_value() should be called exactly once for every single arg/kwarg, in order, and it's not clear by inspection that (or why) this invariant holds
There was a problem hiding this comment.
Clarified the invariant at the helper boundary: positional args are lowered in AST/source order, and explicit runtime kwargs are lowered once by iterating the collected keyword nodes in the user's keyword order rather than by allowed-name order.
…uiltin-kwargs # Conflicts: # vyper/codegen_venom/builtins/system.py
|
Resolved the merge conflicts by merging current I kept the refactored Validation:
|
re. the lowering invariant, i didn't really mean a docs clarification. i meant a code change so that the invariant is clear by inspection. like i think better would be if BuiltinCall calls |
|
Pushed a normal follow-up commit,
I left the explicit opt-outs ( Validation:
|
2 participants
What I did
Follow-up to #4984 after maintainer review. Refactored Venom builtin lowering so
lower_builtin()constructs oneBuiltinCallentry object and everyHANDLERStarget consumes that object instead of each builtin having its own(node, ctx)entry plumbing.This keeps the PR Venom-only while replacing the copied
_get_kwarg_valuestyle with shared infrastructure for reduced kwarg validation, default-filled compile-time constants, and default-filled runtime kwargs. Runtime kwargs are lowered by walkingnode.keywordsin source order, andraw_create/create_from_blueprintmaterialize positional constructor args before keyword expressions so side effects follow source order.How I did it
vyper/codegen_venom/builtins/_kwargs.pywithBuiltinCall, uniform kwarg validation, default-filledget_kwarg_ast_constants(), and source-orderget_kwarg_values().BuiltinCall(node, ctx)to all builtin handlers.BuiltinCall, including non-kwarg builtins, so the entrypoint shape is uniform across the wholeHANDLERSregistry.How to verify it
uv run --python 3.12 make lintuv run --python 3.12 pytest --experimental-codegen tests/unit/compiler/venom tests/functional/builtins/codegen tests/functional/syntax/test_print.py -qCommit message
refactor[codegen]: refactor venom builtins for uniformity and cleanliness
Centralize Venom builtin call lowering around a single
BuiltinCallentry object. Use shared kwarg helpers for validation, default-filled folded constants, and source-order runtime kwarg lowering so each builtin follows the same shape instead of open-coding kwarg lookup.Description for the changelog
Refactor Venom builtin lowering to use a uniform dispatcher entry object and shared source-order keyword argument handling.