fix[lang]: fix list typing issues#5177
Conversation
| if len(node.elements) == 0: | ||
| # can't have an empty SArrayT | ||
| return [DArrayT(BottomT(), 1)] |
There was a problem hiding this comment.
Reject member calls on empty list literals
Because
[]now has one exact dynamic-array type, invalid code such asx: uint256 = [].pop()can resolve thepopmember and itsNeverreturn type type-checks asuint256; the function visitor then treats the list literal as the modified target and hits the_handle_modification()assertion because literals have noVarAccess. Before this change the same member lookup was rejected as an ambiguous empty-list type, so this should be diagnosed instead of reaching an internal compiler assertion.
There was a problem hiding this comment.
def foo():
tmp: uint256 = [empty(uint256)].pop() # StructureException: Ambiguous typeWhere the ambiguity is between DynArray[uint256, 1] and uint256[1].
So there are two ways of solving this:
- Implementing some sort of l-value system, required for Assign to call-return index crashes compiler #5138 and Writing self.balance panics under Venom codegen #5151 anyways
- Making
[]also aNever[0]
Both sounds like they would be useful, but 1. solves a bigger and more serious issue.
I added an xfail test for this.
| if len(node.elements) == 0: | ||
| # can't have an empty SArrayT | ||
| return [DArrayT(BottomT(), 1)] |
There was a problem hiding this comment.
For vararg builtins that require an exact argument type, such as
abi_encode([]), this makes the empty list exact asDynArray[Never, 1]instead of ambiguous.ABIEncode.fetch_call_return()then callsarg_t.abi_typeon that inferred argument, which descends intoBottomT.abi_typeand raises a compiler panic; the untyped empty list should still be rejected with a normal diagnostic unless an expected element type is available.
There was a problem hiding this comment.
Added an error message instead
Gas ChangesNo changes detected. Summary
|
📊 Bytecode Size Changes (venom)No changes detected. Full bytecode sizes
|
![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: 63d96353ad
ℹ️ 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".
| if c.is_subtype_of(t): | ||
| tmp.append(t) | ||
| break |
There was a problem hiding this comment.
When an empty sublist is the first element, common_types starts as DynArray[Never, 1]; this branch appends only the first later type that can contain it and then breaks. For a literal like [[], [1]], the first matching dynamic-array type is the default positive-integer choice (e.g. DynArray[uint256, 1]), so other valid contextual choices are discarded and assignments such as tmp: DynArray[DynArray[int128, 1], 2] = [[], [1]] are rejected depending only on element order ([[1], []] still keeps the int128 alternative).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This was confusingly worded, but I think what this means is: when we have [a, b], for each type of a we loop over all types of b, and we break at the first match, thus shadowing potential further matches.
Before since [] had a type for each builtin type, this "absorbtion" was never noticed, there could never be 3 types among the types of a and b that such that a V could form.
In what codex says, that would be Never for a and uint256 and int128 for b.
The loop would break at Never <: uint256, and never add int128 to the list.
I fixed it by not breaking on a match.
This can lead to an explosion in the number of types of the list, but only if one of the elements has at least 2 types such that one is a subtype of the other:
[_: {Bytes[1], Bytes[2]}, _: {Bytes[3], Bytes[4]}]
# would be inferred to have type:
{
Bytes[3], # Bytes[1] <: Bytes[3]
Bytes[4], # Bytes[1] <: Bytes[4]
Bytes[3], # Bytes[2] <: Bytes[3]
Bytes[4], # Bytes[2] <: Bytes[4]
}I was only able to find one kind of expression with redundant types like this: [<expr>][0]: {<types of expr>, <types of expr>}. Due to [<expr>] having a DArrayT and SArrayT with the same element type.
Given how contrived this is, and that the only impact is time/space used for combination, this is fine for now.
But we should avoiding looping entirely, by having a single type per expression (see #5017).
| if len(node.elements) == 0: | ||
| # can't have an empty SArrayT | ||
| return [DArrayT(BottomT(), 1)] |
There was a problem hiding this comment.
Because every empty literal is materialized as DynArray[Never, 1], an all-empty nested expression selected through another expression keeps that bottom element type even after contextual validation accepts it. For example, tmp: DynArray[Foo, 5] = [[], []][0] type-checks as an empty array of Foo, but parse_Subscript still returns an IR node typed as DynArray[Never, 1], so make_setter recursively rejects the Never element against the struct element type during codegen instead of compiling the empty array.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68ffe0ad32
ℹ️ 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".
1 participant
What I did
Fix: #5176
Fix #5161
More broadly, fixes
get_common_typesalways selecting the first type, as opposed to the supremum of the two:As a workaround, secondary checks where performed in
validate_expected_typeand invisit_Compare(a < bcase), as these are no longer required, they are removed by this PR.Independently, empty list literals (
[]) were inferred to be a list of all built-in types, this lead to issues when combining them with lists of user-made types.This PR fixes that, now empty list literals are compatible with lists of any type.
How I did it
To solve the
get_common_typesissue, it has been refactored to always return the bigger of the pair of matching types.Added the
Neverbottom type: a type which is a subtype of every type. As a consequence, the supremum ofNeverwith a typeTwill always beT.Given lists are covariant, a list of
Nevercan be widened to a list of any required type.Therefore, by having
[]have typeDynArray[Never, 1], it is a validDynArray[T, n]for anyTandn.How to verify it
pytestCommit message
Description for the changelog
Fixed some issues related to computing types of nested lists.
Cute Animal Picture