Conversation
WalkthroughConstant folding logic has been introduced for the built-in functions Changes
Sequence Diagram(s)sequenceDiagram
participant UserCode
participant ASTNode
participant Convert/Slice/Extract32
participant ConstantFolder
UserCode->>ASTNode: Call built-in with literal args
ASTNode->>Convert/Slice/Extract32: Dispatch to built-in
Convert/Slice/Extract32->>ConstantFolder: Attempt _try_fold(node)
alt All args are literals & valid
ConstantFolder-->>Convert/Slice/Extract32: Return folded AST node
Convert/Slice/Extract32-->>ASTNode: Return folded node
else Invalid or non-literal args
ConstantFolder-->>Convert/Slice/Extract32: Raise exception
end
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
vyper/builtins/functions.py (2)
420-425: SameEnumcomparison problem as inconvert.infer_arg_typesIdentical
sorted()lambda is used here – recommend the samestr(t.typ)guard.
968-973:sorted()ordering hazard (again)Same lambda pattern – see previous comments.
🧹 Nitpick comments (1)
vyper/builtins/functions.py (1)
330-373: Inconsistent return node type forHexinputs
slice()returns:
vy_ast.BytesforBytesandHexinputsvy_ast.StrforStrinputsReturning
Bytesfor aHexliteral discards the hex-ness (developers may expect aHex). Downstream passes may re-infer an incorrect type.Two options:
- Emit
vy_ast.Hexwhen the source isvy_ast.Hex.- Document that
slicealways produces runtime bytes, never a hex literal.Option 1 keeps surprise to a minimum:
- elif isinstance(value, vy_ast.Hex): - ... - return vy_ast.Bytes.from_node(node, value=result) + elif isinstance(value, vy_ast.Hex): + ... + return vy_ast.Hex.from_node(node, value="0x"+result.hex())
| self._validate_arg_types(node) | ||
| possible = sorted( | ||
| get_possible_types_from_node(node.args[0]), | ||
| key=lambda t: (t.typ, getattr(t, "bits", 0)) | ||
| ) | ||
| value_type = possible[0] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
sorted() key may raise at runtime
possible = sorted(..., key=lambda t: (t.typ, getattr(t, "bits", 0)))
If t.typ is an Enum (or any non-orderable object) Python 3 will raise TypeError. This sneaks through unit tests if all first elements are equal but explodes on heterogeneous sets.
Safer alternative:
- key=lambda t: (t.typ, getattr(t, "bits", 0))
+ key=lambda t: (str(t.typ), getattr(t, "bits", 0))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self._validate_arg_types(node) | |
| possible = sorted( | |
| get_possible_types_from_node(node.args[0]), | |
| key=lambda t: (t.typ, getattr(t, "bits", 0)) | |
| ) | |
| value_type = possible[0] | |
| self._validate_arg_types(node) | |
| possible = sorted( | |
| get_possible_types_from_node(node.args[0]), | |
| key=lambda t: (str(t.typ), getattr(t, "bits", 0)) | |
| ) | |
| value_type = possible[0] |
| def _try_fold(self, node): | ||
| validate_call_args(node, 2) | ||
| value = node.args[0].get_folded_value() | ||
| target_type = node.args[1].get_folded_value() | ||
| if not isinstance(target_type, vy_ast.Name): | ||
| raise UnfoldableNode | ||
|
|
||
| target_typedef = type_from_annotation(target_type) | ||
| if not isinstance(target_typedef, (BoolT, AddressT, IntegerT, BytesM_T)): | ||
| raise UnfoldableNode | ||
|
|
||
| if isinstance(target_typedef, BoolT): | ||
| if isinstance(value, vy_ast.Int): | ||
| result = bool(value.value) | ||
| elif isinstance(value, vy_ast.Bool): | ||
| result = value.value | ||
| else: | ||
| raise UnfoldableNode | ||
| return vy_ast.Bool.from_node(node, value=result) | ||
|
|
||
| elif isinstance(target_typedef, AddressT): | ||
| if isinstance(value, vy_ast.Hex): | ||
| if len(value.value) != 42: # 0x + 40 hex chars | ||
| raise InvalidLiteral("Address must be exactly 20 bytes", node.args[0]) | ||
| result = value.value | ||
| else: | ||
| raise UnfoldableNode | ||
| return vy_ast.Hex.from_node(node, value=result) | ||
|
|
||
| elif isinstance(target_typedef, IntegerT): | ||
| if not isinstance(value, (vy_ast.Int, vy_ast.Decimal)): | ||
| raise UnfoldableNode | ||
| result = int(value.value) | ||
| lo, hi = target_typedef.ast_bounds | ||
| if result < lo or result > hi: | ||
| raise InvalidLiteral( | ||
| f"value {result} out of range for {target_typedef}", node.args[0] | ||
| ) | ||
| return vy_ast.Int.from_node(node, value=result) | ||
|
|
||
| elif isinstance(target_typedef, BytesM_T): | ||
| if isinstance(value, vy_ast.Hex): | ||
| if len(value.bytes_value) != target_typedef.length: | ||
| raise InvalidLiteral( | ||
| f"Expected {target_typedef.length} bytes, got {len(value.bytes_value)}", | ||
| node.args[0] | ||
| ) | ||
| result = value.value | ||
| else: | ||
| raise UnfoldableNode | ||
| return vy_ast.Hex.from_node(node, value=result) | ||
|
|
||
| raise UnfoldableNode |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potentially unsafe conversions & missing edge-case validation in _try_fold
-
DecimalT ➞ IntegerTtruncates silently viaint(value.value).
Fractions are discarded instead of being rejected or rounded – this may yield unexpected compile-time values. -
Hex ➞ AddressTonly checkslen(value.value)==42, but does not:
• validate the literal actually represents 20 bytes (bytes.fromhex/.bytes_valuewould be safer)
• reject literals without the"0x"prefix (e.g."deadbeef…") – the length check alone would pass. -
Converting to
BytesM_Tassumes the slice must be exactlytarget_typedef.length. Allowing shorter literals to left-pad or throwing a dedicated error would be clearer. -
No constant-folding path exists for:
•Int ➞ Address(very common in assembly-style patterns)
•Bytesliterals toBytesM_T(onlyHexhandled).
Consider tightening validation and clarifying behaviour. Example for the DecimalT issue:
- result = int(value.value)
+ if isinstance(value, vy_ast.Decimal) and value.value % 1 != 0:
+ raise InvalidLiteral("Cannot truncate decimal when converting to integer", node.args[0])
+ result = int(value.value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _try_fold(self, node): | |
| validate_call_args(node, 2) | |
| value = node.args[0].get_folded_value() | |
| target_type = node.args[1].get_folded_value() | |
| if not isinstance(target_type, vy_ast.Name): | |
| raise UnfoldableNode | |
| target_typedef = type_from_annotation(target_type) | |
| if not isinstance(target_typedef, (BoolT, AddressT, IntegerT, BytesM_T)): | |
| raise UnfoldableNode | |
| if isinstance(target_typedef, BoolT): | |
| if isinstance(value, vy_ast.Int): | |
| result = bool(value.value) | |
| elif isinstance(value, vy_ast.Bool): | |
| result = value.value | |
| else: | |
| raise UnfoldableNode | |
| return vy_ast.Bool.from_node(node, value=result) | |
| elif isinstance(target_typedef, AddressT): | |
| if isinstance(value, vy_ast.Hex): | |
| if len(value.value) != 42: # 0x + 40 hex chars | |
| raise InvalidLiteral("Address must be exactly 20 bytes", node.args[0]) | |
| result = value.value | |
| else: | |
| raise UnfoldableNode | |
| return vy_ast.Hex.from_node(node, value=result) | |
| elif isinstance(target_typedef, IntegerT): | |
| if not isinstance(value, (vy_ast.Int, vy_ast.Decimal)): | |
| raise UnfoldableNode | |
| result = int(value.value) | |
| lo, hi = target_typedef.ast_bounds | |
| if result < lo or result > hi: | |
| raise InvalidLiteral( | |
| f"value {result} out of range for {target_typedef}", node.args[0] | |
| ) | |
| return vy_ast.Int.from_node(node, value=result) | |
| elif isinstance(target_typedef, BytesM_T): | |
| if isinstance(value, vy_ast.Hex): | |
| if len(value.bytes_value) != target_typedef.length: | |
| raise InvalidLiteral( | |
| f"Expected {target_typedef.length} bytes, got {len(value.bytes_value)}", | |
| node.args[0] | |
| ) | |
| result = value.value | |
| else: | |
| raise UnfoldableNode | |
| return vy_ast.Hex.from_node(node, value=result) | |
| raise UnfoldableNode | |
| elif isinstance(target_typedef, IntegerT): | |
| if not isinstance(value, (vy_ast.Int, vy_ast.Decimal)): | |
| raise UnfoldableNode | |
| + if isinstance(value, vy_ast.Decimal) and value.value % 1 != 0: | |
| + raise InvalidLiteral("Cannot truncate decimal when converting to integer", node.args[0]) | |
| result = int(value.value) | |
| lo, hi = target_typedef.ast_bounds | |
| if result < lo or result > hi: | |
| raise InvalidLiteral( | |
| f"value {result} out of range for {target_typedef}", node.args[0] | |
| ) | |
| return vy_ast.Int.from_node(node, value=result) |
| def _try_fold(self, node): | ||
| validate_call_args(node, 2) | ||
| value = node.args[0].get_folded_value() | ||
| start = node.args[1].get_folded_value() | ||
|
|
||
| if not isinstance(start, vy_ast.Int): | ||
| raise UnfoldableNode | ||
|
|
||
| start_val = start.value | ||
|
|
||
| if start_val < 0: | ||
| raise ArgumentException("Start index cannot be negative", node.args[1]) | ||
|
|
||
| output_type = self.infer_kwarg_types(node)["output_type"].typedef | ||
|
|
||
| if isinstance(value, vy_ast.Bytes): | ||
| if start_val + 32 > len(value.value): | ||
| raise ArgumentException( | ||
| f"extract32 out of bounds: start={start_val}, max_length={len(value.value)}", | ||
| node | ||
| ) | ||
| result = value.value[start_val:start_val + 32] | ||
| elif isinstance(value, vy_ast.Hex): | ||
| if start_val + 32 > len(value.bytes_value): | ||
| raise ArgumentException( | ||
| f"extract32 out of bounds: start={start_val}, max_length={len(value.bytes_value)}", | ||
| node | ||
| ) | ||
| result = value.bytes_value[start_val:start_val + 32] | ||
| else: | ||
| raise UnfoldableNode | ||
|
|
||
| if isinstance(output_type, BytesM_T): | ||
| return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}") | ||
| elif isinstance(output_type, IntegerT): | ||
| return vy_ast.Int.from_node(node, value=int.from_bytes(result, "big")) | ||
| elif isinstance(output_type, AddressT): | ||
| return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}") | ||
| else: | ||
| raise UnfoldableNode | ||
|
|
There was a problem hiding this comment.
extract32 may violate the requested output size
The function always extracts 32 bytes, yet:
output_typemay beBytesM_T(n)withn ≠ 32AddressTexpects 20 bytes but receives 32
This creates silently-wrong literals and type mismatch down the road.
Minimal fix:
@@
- if isinstance(output_type, BytesM_T):
- return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
+ if isinstance(output_type, BytesM_T):
+ expected = output_type.length
+ if expected != 32:
+ result = result[:expected]
+ return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
elif isinstance(output_type, IntegerT):
@@
- elif isinstance(output_type, AddressT):
- return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
+ elif isinstance(output_type, AddressT):
+ # right-align as per ABI: take the last 20 bytes
+ return vy_ast.Hex.from_node(node, value=f"0x{result[-20:].hex()}")Consider additionally rejecting BytesM_T with length > 32 at semantic level.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _try_fold(self, node): | |
| validate_call_args(node, 2) | |
| value = node.args[0].get_folded_value() | |
| start = node.args[1].get_folded_value() | |
| if not isinstance(start, vy_ast.Int): | |
| raise UnfoldableNode | |
| start_val = start.value | |
| if start_val < 0: | |
| raise ArgumentException("Start index cannot be negative", node.args[1]) | |
| output_type = self.infer_kwarg_types(node)["output_type"].typedef | |
| if isinstance(value, vy_ast.Bytes): | |
| if start_val + 32 > len(value.value): | |
| raise ArgumentException( | |
| f"extract32 out of bounds: start={start_val}, max_length={len(value.value)}", | |
| node | |
| ) | |
| result = value.value[start_val:start_val + 32] | |
| elif isinstance(value, vy_ast.Hex): | |
| if start_val + 32 > len(value.bytes_value): | |
| raise ArgumentException( | |
| f"extract32 out of bounds: start={start_val}, max_length={len(value.bytes_value)}", | |
| node | |
| ) | |
| result = value.bytes_value[start_val:start_val + 32] | |
| else: | |
| raise UnfoldableNode | |
| if isinstance(output_type, BytesM_T): | |
| return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}") | |
| elif isinstance(output_type, IntegerT): | |
| return vy_ast.Int.from_node(node, value=int.from_bytes(result, "big")) | |
| elif isinstance(output_type, AddressT): | |
| return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}") | |
| else: | |
| raise UnfoldableNode | |
| def _try_fold(self, node): | |
| validate_call_args(node, 2) | |
| value = node.args[0].get_folded_value() | |
| start = node.args[1].get_folded_value() | |
| if not isinstance(start, vy_ast.Int): | |
| raise UnfoldableNode | |
| start_val = start.value | |
| if start_val < 0: | |
| raise ArgumentException("Start index cannot be negative", node.args[1]) | |
| output_type = self.infer_kwarg_types(node)["output_type"].typedef | |
| if isinstance(value, vy_ast.Bytes): | |
| if start_val + 32 > len(value.value): | |
| raise ArgumentException( | |
| f"extract32 out of bounds: start={start_val}, max_length={len(value.value)}", | |
| node | |
| ) | |
| result = value.value[start_val:start_val + 32] | |
| elif isinstance(value, vy_ast.Hex): | |
| if start_val + 32 > len(value.bytes_value): | |
| raise ArgumentException( | |
| f"extract32 out of bounds: start={start_val}, max_length={len(value.bytes_value)}", | |
| node | |
| ) | |
| result = value.bytes_value[start_val:start_val + 32] | |
| else: | |
| raise UnfoldableNode | |
| if isinstance(output_type, BytesM_T): | |
| expected = output_type.length | |
| if expected != 32: | |
| result = result[:expected] | |
| return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}") | |
| elif isinstance(output_type, IntegerT): | |
| return vy_ast.Int.from_node(node, value=int.from_bytes(result, "big")) | |
| elif isinstance(output_type, AddressT): | |
| # right-align as per ABI: take the last 20 bytes | |
| return vy_ast.Hex.from_node(node, value=f"0x{result[-20:].hex()}") | |
| else: | |
| raise UnfoldableNode |
1 participant
What I did
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture
Summary by CodeRabbit
New Features
convert,slice, andextract32when all arguments are literals.Enhancements