From 7c4cfbf3d4021f20ff10691d125639901fd8b126 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Sun, 21 Jun 2020 00:47:26 -0700 Subject: [PATCH] Fix typechecking of mutually recursive functions. --- code/compiler/12/definition.cpp | 2 +- code/compiler/12/type.cpp | 8 +++++- code/compiler/12/type_env.cpp | 8 +++--- code/compiler/12/type_env.hpp | 5 ++-- .../blog/12_compiler_let_in_lambda/index.md | 25 +++++++++++++------ 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/code/compiler/12/definition.cpp b/code/compiler/12/definition.cpp index 60ac44a..c6e69ae 100644 --- a/code/compiler/12/definition.cpp +++ b/code/compiler/12/definition.cpp @@ -139,7 +139,7 @@ void definition_group::typecheck(type_mgr& mgr, type_env_ptr& env) { def_defn->typecheck(mgr); } for(auto& def_defnn_name : group->members) { - this->env->generalize(def_defnn_name, mgr); + this->env->generalize(def_defnn_name, *group, mgr); } } } diff --git a/code/compiler/12/type.cpp b/code/compiler/12/type.cpp index 30852f8..c4ff35a 100644 --- a/code/compiler/12/type.cpp +++ b/code/compiler/12/type.cpp @@ -205,7 +205,13 @@ void type_mgr::find_free(const type_ptr& t, std::set& into) const { void type_mgr::find_free(const type_scheme_ptr& t, std::set& into) const { std::set monotype_free; - find_free(t->monotype, monotype_free); + type_mgr limited_mgr; + for(auto& binding : types) { + auto existing_position = std::find(t->forall.begin(), t->forall.end(), binding.first); + if(existing_position != t->forall.end()) continue; + limited_mgr.types[binding.first] = binding.second; + } + limited_mgr.find_free(t->monotype, monotype_free); for(auto& not_free : t->forall) { monotype_free.erase(not_free); } diff --git a/code/compiler/12/type_env.cpp b/code/compiler/12/type_env.cpp index 048eb82..f42e46a 100644 --- a/code/compiler/12/type_env.cpp +++ b/code/compiler/12/type_env.cpp @@ -8,11 +8,11 @@ void type_env::find_free(const type_mgr& mgr, std::set& into) const } } -void type_env::find_free_except(const type_mgr& mgr, const std::string& avoid, +void type_env::find_free_except(const type_mgr& mgr, const group& avoid, std::set& into) const { if(parent != nullptr) parent->find_free(mgr, into); for(auto& binding : names) { - if(binding.first == avoid) continue; + if(avoid.members.find(binding.first) != avoid.members.end()) continue; mgr.find_free(binding.second.type, into); } } @@ -65,7 +65,7 @@ void type_env::bind_type(const std::string& type_name, type_ptr t) { type_names[type_name] = t; } -void type_env::generalize(const std::string& name, type_mgr& mgr) { +void type_env::generalize(const std::string& name, const group& grp, type_mgr& mgr) { auto names_it = names.find(name); if(names_it == names.end()) throw 0; if(names_it->second.type->forall.size() > 0) throw 0; @@ -73,7 +73,7 @@ void type_env::generalize(const std::string& name, type_mgr& mgr) { std::set free_in_type; std::set free_in_env; mgr.find_free(names_it->second.type->monotype, free_in_type); - find_free_except(mgr, name, free_in_env); + find_free_except(mgr, grp, free_in_env); for(auto& free : free_in_type) { if(free_in_env.find(free) != free_in_env.end()) continue; names_it->second.type->forall.push_back(free); diff --git a/code/compiler/12/type_env.hpp b/code/compiler/12/type_env.hpp index 271a599..5292904 100644 --- a/code/compiler/12/type_env.hpp +++ b/code/compiler/12/type_env.hpp @@ -2,6 +2,7 @@ #include #include #include +#include "graph.hpp" #include "type.hpp" struct type_env; @@ -29,7 +30,7 @@ struct type_env { type_env() : type_env(nullptr) {} void find_free(const type_mgr& mgr, std::set& into) const; - void find_free_except(const type_mgr& mgr, const std::string& avoid, + void find_free_except(const type_mgr& mgr, const group& avoid, std::set& into) const; type_scheme_ptr lookup(const std::string& name) const; bool is_global(const std::string& name) const; @@ -41,7 +42,7 @@ struct type_env { void bind(const std::string& name, type_scheme_ptr t, visibility v = visibility::local); void bind_type(const std::string& type_name, type_ptr t); - void generalize(const std::string& name, type_mgr& mgr); + void generalize(const std::string& name, const group& grp, type_mgr& mgr); }; diff --git a/content/blog/12_compiler_let_in_lambda/index.md b/content/blog/12_compiler_let_in_lambda/index.md index 106346f..8b0f3db 100644 --- a/content/blog/12_compiler_let_in_lambda/index.md +++ b/content/blog/12_compiler_let_in_lambda/index.md @@ -486,17 +486,17 @@ we're trying to operate on is global or not? I propose a flag in our this, we update the implementation of `type_env` to map variables to values of a struct `variable_data`: -{{< codelines "C++" "compiler/12/type_env.hpp" 13 22 >}} +{{< codelines "C++" "compiler/12/type_env.hpp" 14 23 >}} The `visibility` enum is defined as follows: -{{< codelines "C++" "compiler/12/type_env.hpp" 10 10 >}} +{{< codelines "C++" "compiler/12/type_env.hpp" 11 11 >}} As you can see from the above snippet, we also added a `mangled_name` field to the new `variable_data` struct. We will be using this field shortly. We also add a few methods to our `type_env`, and end up with the following: -{{< codelines "C++" "compiler/12/type_env.hpp" 31 44 >}} +{{< codelines "C++" "compiler/12/type_env.hpp" 32 45 >}} We will come back to `find_free` and `find_free_except`, as well as `set_mangled_name` and `get_mangled_name`. For now, we just adjust `bind` to @@ -812,9 +812,12 @@ void type_env::find_free_except(const type_mgr& mgr, const std::string& avoid, Why `find_free_except`? When generalizing a variable whose type was already stored in the environment, all the type variables we could generalize would not be 'free'. If they only occur in the type we're generalizing, though, -we shouldn't let that stop us! Thus, when finding free type variables, we will -avoid looking at the particular variable whose type is being generalized. The -implementations of the two methods are straightforward: +we shouldn't let that stop us! More generally, if we see type variables that +are only found in the same mutually recursive group as the binding we're +generalizing, we are free to generalize them too. Thus, we pass in +a reference to a `group`, and check if a variable is a member of that group +before searching it for free type variables. The implementations of the two +methods are straightforward: {{< codelines "C++" "compiler/12/type_env.cpp" 4 18 >}} @@ -824,7 +827,15 @@ that have the same name as the variable we're generalizing, but aren't found in the same scope. As far as we're concerned, they're different variables! The two methods use another `find_free` method which we add to `type_mgr`: -{{< codelines "C++" "compiler/12/type.cpp" 206 213 >}} +{{< codelines "C++" "compiler/12/type.cpp" 206 219 >}} + +This one is a bit of a hack. Typically, while running `find_free`, a +`type_mgr` will resolve any type variables. However, variables from the +`forall` quantifier of a type scheme should not be resolved, since they +are explicitly generic. To prevent the type manager from erroneously resolving +such type variables, we create a new type manager that does not have +these variables bound to anything, and thus marks them as free. We then +filter these variables out of the final list of free variables. Finally, `generalize` makes sure not to use variables that it finds free: