Ben's Proposal #3
Conversation
There was a problem hiding this comment.
First of all, thanks for your proposal @montymxb ! It helped me to dive into type checking and to identify several challenges, I didn't thought about before.
At first, I missed your proposal, but finally, I saw, that you continued the proposal of Mark, my bad ...
In general, I had several problems to understand the ideas behind: Therefore I asked lots of questions, which might sound negative. But in my eyes, it is "normal" to have problems to understand the ideas of others, and it is the only chance for us to combine the best design for Typir from multiple, different proposals. I am looking forward to discuss the proposals in our next meetings!
My comments to Mark's part (which I wrote earlier):
- I had some problems to understand several concepts of the proposal, not because of the proposal itself, but because of my lack of knowledge about type checking concepts. Therefore, some of my comments might be missleading.
- Could you sketch an example, how to apply the type system? That would help me to understand the ideas behind.
- (I didn't reviewed all kinds.)
| import type { TypeSystem } from "./type-system"; | ||
| import { Disposable } from "./utils"; | ||
|
|
||
| export interface TypeCategory<T> { |
There was a problem hiding this comment.
What is a type category? Could you add some comments?
| import { Disposable } from "./utils"; | ||
|
|
||
| export interface Type<T> { | ||
| readonly literal?: T; |
There was a problem hiding this comment.
What is T and the literal? Would AstNode be a value for T in the context of Langium? If a type like string is the type for multiple literals, do I have multiple Type instances representing string, one for each literal?
|
|
||
| export interface Type<T> { | ||
| readonly literal?: T; | ||
| readonly members: Iterable<TypeMember<T>>; |
There was a problem hiding this comment.
Does it really make sense to have members, even when lots of types don't have any members, e.g. all primitive types like int, string, ...? For functions, I would prefer to call them "parameters" instead of "members" ...
| readonly typeParameters: TypeParameter<T>[]; | ||
| readonly typeArguments: Type<T>[]; | ||
| applyTypeArguments(args: Type<T>[]): FunctionType<T>; | ||
| readonly parameters: FunctionParameter<T>; |
There was a problem hiding this comment.
Sorry, I am lost: What are members in contrast to parameters?
(What about FunctionParameter<T> extends TypeMember<T>?)
| assignable(callback: AssignabilityCallback<PrimitiveType<T>>): Disposable; | ||
| } | ||
|
|
||
| export interface PrimitiveTypeConstant<T> extends Type<T> { |
There was a problem hiding this comment.
Did I understand correctly, that PrimitiveTypeConstant is an instance/element of the set defined by a PrimitiveType?
| return reachableTypes.has(fromType); | ||
| } | ||
|
|
||
| inferType(expression: any): any { |
There was a problem hiding this comment.
Does Typir provides this as default implementation? So the user of Typir adapts the inference by sub-classing Typir and overriding this function? In Langium, we would register a different implementation for an "inference service"
| @@ -0,0 +1,228 @@ | |||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | |||
There was a problem hiding this comment.
Now I found the correct file to review, sorry
| // binding symbols with types in the environment | ||
| // | ||
| const xSymbol = typirInstance.loadSymbol('x', intType); | ||
| const ySymbol = typirInstance.loadSymbol('y', boolType); |
There was a problem hiding this comment.
What are "x" and "y"? I don't see them again in the example AST ...
|
|
||
| // Example concrete visitor implementing TypirVisitor | ||
| // Which accepts an AST object and updates the typir instance w/ types, relationships, etc. | ||
| class ExampleASTVisitor implements TypirVisitor { |
There was a problem hiding this comment.
I still have the feeling, that I didn't got the main idea behind the visitor.
From my current thinking, the visitor seems to be not necessary, since the visitor provides only API (a single line!):
Neither the traversal of the AST (which calls this API) nor the implementation of the API (what is called) seems to be generic, but both are depending on our current DSL. Therefore, what is the benefit of the API? Where is some simplification or reuse or ... ?
| // inject the visitor into the typir instance, and get the visitor to traverse the AST | ||
| // result is that the typir instance is updated with the relationships and types defined in the visitor | ||
| // and then we can use the typir instance to do type inference, type checking, etc. | ||
| const visitor = new ExampleASTVisitor(typirInstance); |
There was a problem hiding this comment.
Regarding your explanations (well, what I belief to understand), the visitor aims to fill the type graph with information about existing types and their relationships by traversing the current AST:
Why can I call isAssignable (line 202) before using the visitor (line 225)?
No description provided.