Skip to content
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

dont forget to test recursive function definitions.... #84

Closed
edwadli opened this issue Nov 22, 2015 · 26 comments
Closed

dont forget to test recursive function definitions.... #84

edwadli opened this issue Nov 22, 2015 · 26 comments
Labels

Comments

@edwadli
Copy link
Owner

edwadli commented Nov 22, 2015

The type checker might run forever!!!!!!!!!!

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

A cheap way to fix this: add an exception handler for the stack overflow exception

Sent from my Apple Watch

On Nov 22, 2015, at 2:57 AM, Edward Li [email protected] wrote:

The type checker might run forever!!!!!!!!!!


Reply to this email directly or view it on GitHub.

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

Also did you mean recursive type definitions? Not sure how we can have recursive function-definition in our language

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

fun Hello x = Hello x

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

not fun at all

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

There's a similar challenge w/ C++ templates: since they are turing complete, the compiler would have to solve the halting problem. Instead clang has a maximum template instantiation depth of 256 to work around it.

screen shot 2015-11-22 at 1 55 45 pm
code via http://stackoverflow.com/q/6079603/1489823

Another interesting thing. The program below (which is equivalent of your NH code above) gives the error "no matching function for call to hello."

It think it's because C++ can't overload functions based on return type.

#include <iostream>

template <typename TRet, typename TArg>
TRet hello(TArg x) {
    return hello(x);
}

int main(int argc, const char * argv[]) {
    hello(5);
    return 0;
}

However, by making the call type and the return type the same, the program now compiles and we get a stack overflow, as expected.

template <typename T>
T hello(T x) {
    return hello(x);
}

@kevin1 kevin1 added the bug label Nov 22, 2015
@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

i believe the current implementation of type checking for functions runs DFS. We will have to change that to BFS on all exprs that have multiple exprs inside. Otherwise, if you write a recursive function with the base case after the recursion, our type checker will fail to halt, and produce the wrong answer if we just stop it at some arbitrary depth

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

Ah was just about to say that but got pulled away from computer for a minute

We can check if the function has a return other than the recursive call, which can distinguish between these two cases:

// Yes:
fun Hello x = (if x == 1 then x else Hello x - 1)
// No:
fun Hello x = Hello x

Eventually we'll find that our compiler accidentally includes a static analyzer :)

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

i can smell another dependency graph coming

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

instead of checking for cycles we'd check for leaf nodes

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

i think that's sufficient?

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

Can you clarify? The nodes of this graph are function calls?

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

yeah each function is a node in the graph, and the edges are calls to other functions

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

Yeah, I think that works. One caveat is you can only ignore a call (edge) if the destination and source's arg types are the same.

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

Some things to watch out for
binop -> both branches need a leaf
block -> sequentially check each expr for a leaf
funapply -> every arg expr needs a leaf

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

why binop?

fun Hello x = ( 
  if x == 0 then 0 else ( 1 + Hello (x-1) )
)

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

^ I think in that case, you'd parse the if-else. Type of if is int, type of else is also int because we already know Hello int returns int.

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

yeah conditional -> bool expr needs leaf; one of the two expr needs leaf

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

Right but I don't think both children of the binop (+) need leaves

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

Hello x = 1 + Hello x

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

That case is thrown out because the function doesn't have a base case. I think the binop itself is not the problem.

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

in the example u gave the conditional only requires on of the two expr to be valid (which would be the 'then' case in that example)

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

i think checking for leaves is the same thing as checking for base cases

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

i think base cases are when u hit something with definite type (literal), and every branch of each expr needs to have a leaf, except for conditionals, in which the bool expr needs a leaf, and only one of the two exprs needs a leaf.

@edwadli
Copy link
Owner Author

edwadli commented Nov 22, 2015

I think i was wrong about the BFS, the DFS is fine but we need an extra halting condition so we don't get stuck in a cycle; but we might have to go back and figure out the types of those calls after we find the base cases in other branches...

@kevin1
Copy link
Collaborator

kevin1 commented Nov 22, 2015

OK, let's hold off on thinking about this until we actually implement it. I think we are both nerd-sniping oureslves

@edwadli
Copy link
Owner Author

edwadli commented Dec 1, 2015

closed by #104

@edwadli edwadli closed this as completed Dec 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants