add static type checking for function call#1
add static type checking for function call#1ishwar00 wants to merge 15 commits intoSamyak2:masterfrom
Conversation
|
I also made an attemp to add marker in error message for each argument that did not match declared type. It had limitations, because i believe there is not enough info to locate(line_number, column_number) arguments in current AST. Currently error message marks callee name. |
|
I have tried printing the the declared function's parameter along with the error in 852e1fa. But it looks like it's pointing to the wrong parameter at times. For a simple example like this: package main
func a(b int, c float64) {
return b + c
}
func main() {
a(10, "string")
}It does work: But seems to be incorrect for the test cases you have added. |
|
For arguments though, we'll also need to keep track of |
|
That said, this is really nice already. I don't mind merging it at this stage. |
yes!, i have that in mind. I'll do it, once type declarations are fixed. |
Good to hear that!, but still static type checking gets wrong for new type declarations. |
|
I have been thinking about resolving that syntax error.
one represents type name other represents a set of values, in a certain scope. So i think we should add types in SymbolTable instead of TypeTable. But this might look mingling of two identifiers representing different things. But since we cannot declare a identifier twice in the same block, it should not be a problem. So a identifier in the given scope either represents type name or value. Let me know what you think. |
|
Makes sense to me. Although, I wonder how errors will look like when you pass a type name instead of a value - we can get to that once type names are working. Also, I see this comment under the grammar rule you linked: I'm guessing this wasn't implemented because of time constraints. Fixing conflicts in a grammar can be annoying xD |
|
Actually i did not understand why we would get reduce/reduce conflict. But when i changed """ParameterDecl : Type
| ELLIPSIS Type
| IdentifierList Type
| IdentifierList ELLIPSIS Type
"""rule is the cause. Type and IdentifierList are producing shift/reduce conflict. But i think it's not a problem, since parser prefers to shift, it naturally solves it. If we remove Type from above rule, parser doesn't warn about any conflict. Implying Type and IdentifierList are conflicting(i think). """ParameterDecl : ELLIPSIS Type
| IdentifierList Type
| IdentifierList ELLIPSIS Type
"""P.S: After looking at comments it seems that, decision to store types in TypeTable was made at the last moment, to simply things a bit, i assume it was time contraint. |
|
please ignore this. parser warns about reduce/reduce conflicts, after changing production the above conflicting productions are TypeName and OperandName : So where are the conflicts? they must be right below between OperandName and Literal, because OperandName occurs only in Operand production. I don't quite get it, how is this a reduce/reduce conflict. because CompositeLit cannot be reduced to an IDENTIFIER, can it? |
|
Hey, could you please explain me, why update_info was called here? # just for reference
class Function(Node):
"""Node to store function declaration"""
def __init__(self, name, signature, lineno: int, body=None):
super().__init__("FUNCTION",
children=[signature, body],
data=(name, lineno))
self.data: tuple
self.fn_name = name
self.lineno = lineno
self.signature = signature
self.body = body
if name is not None:
symtab.update_info(name[1],
lineno,
0,
type_="FUNCTION",
const=True,
value=self) |
|
I think it's because of this: The function is added to the symbol table as soon as we see a function name. Anything after that - parameters, return type, body, etc. - is declared in that function's scope. Edit: so when the function declaration is completely parsed, it is updated in the symtab. |
Oh, i see. I did not trace FunctionDecl production at all, my bad. About the reduce/reduce conflicts, as of now we can get away with it, by removing TypeName from LiteralType, and we don't see them as long as we don't support StructType. But getting away with it like this, won't be so satifying. |
|
Things are still broken, still fixing, but things are changed enough for suggestions. |
|
It's almost done, except shift/reduce conflict. |
|
Just to update: I believe it is not yet ready to merge, it needs few more changes, especially code generation part, i am not confident enough. Also static type checking gets wrong for recursive calls, since signature gets updated after parsing body of a function. |
2 participants
To get the type of arguments, i have used pretty much same approach as you used for type inference. I have added space in the beginning of some comments, just because LSP was yelling at me to add. Error messages need some work.