refactor[next]: new ITIR type inference #1531
Conversation
src/gt4py/next/ffront/type_info.py
Outdated
|
|
||
| new_args = [*args] | ||
| for i, (param, arg) in enumerate( | ||
| zip(function_type.pos_only_args + list(function_type.pos_or_kw_args.values()), args) | ||
| zip(list(function_type.pos_only_args) + list(function_type.pos_or_kw_args.values()), args) |
There was a problem hiding this comment.
Just a question: should zip() be called here with strict=True?
There was a problem hiding this comment.
Yes. At some point I split the structural checks (e.g. is the number of arguments correct) and the type checks. Now the structural checks are already run before the types are promoted so we can actually use strict=True here.
| @@ -32,6 +31,9 @@ | |||
| class Node(eve.Node): | |||
| location: Optional[SourceLocation] = eve.field(default=None, repr=False, compare=False) | |||
|
|
|||
| # TODO(tehrengruber): include in comparison if value is not None | |||
| """ | ||
| Convert to Sym if necessary. | ||
|
|
||
| Examples | ||
| -------- | ||
| >>> sym("a") | ||
| Sym(id=SymbolName('a'), kind=None, dtype=None) | ||
| Sym(id=SymbolName('a')) |
There was a problem hiding this comment.
Could you add an example using of type_ != None or mention that case in the docstring ?
(By the way, although it's outside the scope of this PR, the current implementation of ensure_type() doesn't make a lot of sense to me)
There was a problem hiding this comment.
I've added an example. type is not part of the __repr__ of itir.Node which makes the example somewhat odd. Adding it means I need to change many (doct)tests and output is often annoyingly verbose. Should I add it? I'm leaning towards a yes by now.
| declaration. This is useful for testing or inference on partially inferred sub-nodes. | ||
| """ | ||
| if not allow_undeclared_symbols: | ||
| node = RemoveTypes().visit(node) |
There was a problem hiding this comment.
Can you elaborate on this?
| if "offset_provider" in inspect.signature(self.type_rule).parameters: | ||
| return_type = self.type_rule(*args, offset_provider=self.offset_provider) | ||
| else: | ||
| return_type = self.type_rule(*args) |
There was a problem hiding this comment.
This is ugly and probably not super efficient. I think it would be cleaner to check the signature of the type rule only when registering the rule and set a flag in the returned wrapper exposing whether the wrapped type rule requires the offset provider or not.
| return_type = self.type_rule(*args) | ||
|
|
||
| # return type is a typing rule by itself | ||
| if callable(return_type): |
There was a problem hiding this comment.
Just a comment: checking only if it's callable is slightly dangerous because it's a bit too general and returning by mistake things like a type would be interpreted as a type rule because types are callables (the constructor)
| assert isinstance(node.fencil.type, it_ts.FencilType) | ||
| return node.fencil.type | ||
|
|
||
| def visit_Program(self, node: itir.Program, *, ctx): |
There was a problem hiding this comment.
Missing return type annotation.
WIP