-
-
Notifications
You must be signed in to change notification settings - Fork 3k
perf: deduplicate fast_container_type and fast_dict_type items before joining
#19409
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
Changes from 10 commits
2f2c391
48cb9e1
19f0260
7d9dcb6
0cc75eb
48bc430
ff26ebf
b608a66
46f7825
3aa9472
8882c80
0f560b3
c2fd9fa
af92439
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 | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5068,11 +5068,16 @@ def fast_container_type( | |||||||||||||||
| Limitations: | ||||||||||||||||
| - no active type context | ||||||||||||||||
| - no star expressions | ||||||||||||||||
| - the joined type of all entries must be an Instance or Tuple type | ||||||||||||||||
| - not after deferral | ||||||||||||||||
| - either exactly one distinct type inside, | ||||||||||||||||
| or the joined type of all entries must be an Instance or Tuple type | ||||||||||||||||
| """ | ||||||||||||||||
| ctx = self.type_context[-1] | ||||||||||||||||
| if ctx: | ||||||||||||||||
| return None | ||||||||||||||||
| if self.chk.current_node_deferred: | ||||||||||||||||
| # Guarantees that all items will be Any, we'll reject it anyway. | ||||||||||||||||
| return None | ||||||||||||||||
| rt = self.resolved_type.get(e, None) | ||||||||||||||||
| if rt is not None: | ||||||||||||||||
| return rt if isinstance(rt, Instance) else None | ||||||||||||||||
|
|
@@ -5082,15 +5087,27 @@ def fast_container_type( | |||||||||||||||
| # fallback to slow path | ||||||||||||||||
| self.resolved_type[e] = NoneType() | ||||||||||||||||
| return None | ||||||||||||||||
| values.append(self.accept(item)) | ||||||||||||||||
| vt = join.join_type_list(values) | ||||||||||||||||
| if not allow_fast_container_literal(vt): | ||||||||||||||||
|
|
||||||||||||||||
| typ = self.accept(item) | ||||||||||||||||
| if typ not in values: | ||||||||||||||||
sterliakov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||
| values.append(typ) | ||||||||||||||||
|
|
||||||||||||||||
| vt = self._first_or_join_fast_item(values) | ||||||||||||||||
| if vt is None: | ||||||||||||||||
| self.resolved_type[e] = NoneType() | ||||||||||||||||
| return None | ||||||||||||||||
| ct = self.chk.named_generic_type(container_fullname, [vt]) | ||||||||||||||||
| self.resolved_type[e] = ct | ||||||||||||||||
| return ct | ||||||||||||||||
|
|
||||||||||||||||
| def _first_or_join_fast_item(self, items: list[Type]) -> Type | None: | ||||||||||||||||
| if len(items) == 1 and not self.chk.current_node_deferred: | ||||||||||||||||
| return items[0] | ||||||||||||||||
| typ = join.join_type_list(items) | ||||||||||||||||
| if not allow_fast_container_literal(typ): | ||||||||||||||||
|
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 feels a bit restrictive together with the above 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. We could, but our join implementation for callables is horribly broken, so this will produce invalid results: e.g. we can confidently join two callables with named args with different names into one of them. def left(*, x: str): ...
def right(*, y: str): ...then I'm not sure if there are other similarly terrifying bugs there, so I'd really prefer to not expand this logic now. This is not related to expression source ( Lines 797 to 803 in 2996c91
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. OK, I see, makes sense. |
||||||||||||||||
| return None | ||||||||||||||||
| return typ | ||||||||||||||||
|
|
||||||||||||||||
| def check_lst_expr(self, e: ListExpr | SetExpr | TupleExpr, fullname: str, tag: str) -> Type: | ||||||||||||||||
| # fast path | ||||||||||||||||
| t = self.fast_container_type(e, fullname) | ||||||||||||||||
|
|
@@ -5277,13 +5294,23 @@ def fast_dict_type(self, e: DictExpr) -> Type | None: | |||||||||||||||
| self.resolved_type[e] = NoneType() | ||||||||||||||||
| return None | ||||||||||||||||
| else: | ||||||||||||||||
| keys.append(self.accept(key)) | ||||||||||||||||
| values.append(self.accept(value)) | ||||||||||||||||
| kt = join.join_type_list(keys) | ||||||||||||||||
| vt = join.join_type_list(values) | ||||||||||||||||
| if not (allow_fast_container_literal(kt) and allow_fast_container_literal(vt)): | ||||||||||||||||
| key_t = self.accept(key) | ||||||||||||||||
| if key_t not in keys: | ||||||||||||||||
| keys.append(key_t) | ||||||||||||||||
| value_t = self.accept(value) | ||||||||||||||||
| if value_t not in values: | ||||||||||||||||
| values.append(value_t) | ||||||||||||||||
|
|
||||||||||||||||
| kt = self._first_or_join_fast_item(keys) | ||||||||||||||||
| if kt is None: | ||||||||||||||||
| self.resolved_type[e] = NoneType() | ||||||||||||||||
| return None | ||||||||||||||||
|
|
||||||||||||||||
| vt = self._first_or_join_fast_item(values) | ||||||||||||||||
| if vt is None: | ||||||||||||||||
| self.resolved_type[e] = NoneType() | ||||||||||||||||
| return None | ||||||||||||||||
|
|
||||||||||||||||
| if stargs and (stargs[0] != kt or stargs[1] != vt): | ||||||||||||||||
| self.resolved_type[e] = NoneType() | ||||||||||||||||
| return None | ||||||||||||||||
|
|
||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.