caterinaurban / Typpete

34 stars 6 forks source link

Implemented isinstance inference #6

Closed mostafa-abdullah closed 7 years ago

mostafa-abdullah commented 7 years ago

There's a conceptual problem with the inference of isinstance. See the following example:

def f(x):
    if isinstance(x, A):
         return x.f()
    else:
        return x + 2

The type of variable x in the then part is A, while it is int in the else part. As a result, the type of function argument x is inferred to be the super-type of both, which is object. As a result, a function call like f("string") will be allowed because string <: object, which makes it unsound.

What do you think?

mostafa-abdullah commented 7 years ago

I think to solve this problem, the type of argument needs to be Union[int, A]

marcoeilers commented 7 years ago

So to summarize (from my point of view) what we just discussed:

mostafa-abdullah commented 7 years ago

Implemented isinstance as agreed.

The overall type of the variable compared in the isinstance test is inferred from only the else branch (and of course the rest of the program).

The isinstance test is currently limited to a simple test like the following:

if isinstance(x, y):
    pass

Where x is a variable. Doing conjunction or disjunction in the isinstance test (like isinstance(x,y) or isinstance(x,z)) is NOT possible.

Example:

class A:
    def f(self):
        return 1

class B:
    def g(self):
        return 1

def f(x):
    if isinstance(x, A):
        res = x.f()
    elif isinstance(x, B):
        res = x.g()
    else:
        res = x + 1
    return res

f := Callable[[int], int]

TODO: use built in types in isinstance test.

marcoeilers commented 7 years ago

Does the TODO mean that we can't currently do something like if isinstance (x, int): ...? If so, why is that more difficult? Also, I assume when you say doing conjunctions etc is not possible, you mean that we don't change the type of the variable in question in those cases, but users can still write that code (and will simply have to manually cast the variable to a different type in the following block)? Or do we crash or report an error?

Otherwise, looks good to me.

mostafa-abdullah commented 7 years ago

Does the TODO mean that we can't currently do something like if isinstance (x, int): ...?

Yes this is not currently supported.

If so, why is that more difficult?

The reason is that in every function call subtype axioms are added between the types of the arguments the corresponding ones in the function definition, however we don't have Z3 types for built-ins like int. To get it working, we need to have z3 constants corresponding to the built-in types in our context, however they will conflict with the stub functions int(x), float(x), str(x), etc. We can also special case this by ignoring these axioms in an isinstance call. What do you think?

you mean that we don't change the type of the variable in question in those cases, but users can still write that code (and will simply have to manually cast the variable to a different type in the following block)?

Yes exactly. We do nothing and the user must cast the variables.