-
-
Notifications
You must be signed in to change notification settings - Fork 911
fix[lang]: fix list typing issues #5177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 10 commits
a830b63
ae7175c
73382d9
74f2004
63e2031
ea497fc
da98b1b
5fbaaad
63d9635
68ffe0a
1d6f5c2
b2245f5
1d111c8
5922d14
354f96f
197c117
3cf6c87
d093d07
c2b866d
88fd55b
7106325
b21d489
efbe641
a08ef05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ | |
| from vyper.semantics.analysis.base import ExprInfo, Modifiability, ModuleInfo, VarAccess, VarInfo | ||
| from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions | ||
| from vyper.semantics.namespace import get_namespace | ||
| from vyper.semantics.types.base import TYPE_T, VyperType | ||
| from vyper.semantics.types.base import TYPE_T, BottomT, VyperType | ||
| from vyper.semantics.types.bytestrings import BytesT, StringT | ||
| from vyper.semantics.types.infinity import is_bounded_length | ||
|
|
||
|
|
@@ -349,28 +349,10 @@ def types_from_IfExp(self, node): | |
|
|
||
| def types_from_List(self, node): | ||
| # literal array | ||
| if _is_empty_list(node): | ||
| ret = [] | ||
|
|
||
| if len(node.elements) > 0: | ||
| # empty nested list literals `[[], []]` | ||
| subtypes = self.get_possible_types_from_node(node.elements[0]) | ||
| else: | ||
| # empty list literal `[]` | ||
| # subtype can be anything | ||
| subtypes = types.PRIMITIVE_TYPES.values() | ||
|
|
||
| for t in subtypes: | ||
| # 1 is minimum possible length for dynarray, | ||
| # can be assigned to anything | ||
| if isinstance(t, VyperType): | ||
| ret.append(DArrayT(t, 1)) | ||
| elif isinstance(t, type) and issubclass(t, VyperType): | ||
| # for typeclasses like bytestrings, use a generic type acceptor | ||
| ret.append(DArrayT(t.any(), 1)) | ||
| else: | ||
| raise CompilerPanic(f"busted type {t}", node) | ||
| return ret | ||
| if len(node.elements) == 0: | ||
| # can't have an empty SArrayT | ||
| return [DArrayT(BottomT(), 1)] | ||
|
Comment on lines
+353
to
+355
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. def foo():
tmp: uint256 = [empty(uint256)].pop() # StructureException: Ambiguous typeWhere the ambiguity is between So there are two ways of solving this:
Both sounds like they would be useful, but 1. solves a bigger and more serious issue.
Comment on lines
+353
to
+355
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added an error message instead
Comment on lines
+353
to
+355
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because every empty literal is materialized as Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @codex I wasn't able to replicate, please go more into details There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To use Codex here, create a Codex account and connect to github.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @codex I wasn't able to replicate, please go more into details (this time connected codex with github, let's see if it works) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To use Codex here, create an environment for this repo.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @codex I wasn't able to replicate, please go more into details (the link tells me it's connected, but third time's a charm I guess) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To use Codex here, create an environment for this repo.
Sporarum marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When both arms of a conditional expression are empty lists and the contextual element type is not one of the old primitive candidates, this new bottom-typed Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to this change, see #5199
Sporarum marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should instead be raised during constant folding, since the types do match For now while There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because bare Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done: a08ef05 |
||
|
|
||
| types_list = get_common_types(*node.elements) | ||
|
|
||
|
|
@@ -432,18 +414,6 @@ def types_from_UnaryOp(self, node): | |
| return _validate_op(node, types_list, "validate_numeric_op") | ||
|
|
||
|
|
||
| def _is_empty_list(node): | ||
| # Checks if a node is a `List` node with an empty list for `elements`, | ||
| # including any nested `List` nodes. ex. `[]` or `[[]]` will return True, | ||
| # [1] will return False. | ||
| if not isinstance(node, vy_ast.List): | ||
| return False | ||
|
|
||
| if not node.elements: | ||
| return True | ||
| return all(_is_empty_list(t) for t in node.elements) | ||
|
|
||
|
|
||
| def _is_type_in_list(obj, types_list): | ||
| # check if a type object is in a list of types | ||
| return any(i.is_equivalent_to(obj) for i in types_list) | ||
|
|
@@ -528,12 +498,16 @@ def get_common_types(*nodes: vy_ast.VyperNode, filter_fn: Callable = None) -> Li | |
| tmp = [] | ||
| for c in common_types: | ||
| for t in new_types: | ||
| # TODO: This can add either the supertype or the subtype to tmp depending on | ||
| # the order | ||
| if t.compare_type(c) or c.compare_type(t): | ||
| # The common type is the one such that both are subtypes | ||
|
|
||
| if t.is_subtype_of(c): | ||
| tmp.append(c) | ||
| break | ||
|
|
||
| if c.is_subtype_of(t): | ||
| tmp.append(t) | ||
| break | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an empty sublist is the first element, Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was confusingly worded, but I think what this means is: when we have I fixed it by not breaking on a match. [_: {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: |
||
|
|
||
| common_types = tmp | ||
|
|
||
| if filter_fn is not None: | ||
|
|
@@ -598,17 +572,9 @@ def validate_expected_type(node, expected_type): | |
|
|
||
| given_types = _ExprAnalyser().get_possible_types_from_node(node) | ||
|
|
||
| if isinstance(node, vy_ast.List): | ||
| # special case - for literal arrays we individually validate each item | ||
| for expected in expected_type: | ||
| if not isinstance(expected, (DArrayT, SArrayT)): | ||
| continue | ||
| if _validate_literal_array(node, expected): | ||
| return | ||
| else: | ||
| for given, expected in itertools.product(given_types, expected_type): | ||
| if given.is_subtype_of(expected): | ||
| return | ||
| for given, expected in itertools.product(given_types, expected_type): | ||
| if given.is_subtype_of(expected): | ||
| return | ||
|
Comment on lines
+561
to
+563
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the expected type is the generic Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Known issue, was reported above. |
||
|
|
||
| # validation failed, prepare a meaningful error message | ||
| if len(expected_type) > 1: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.