code/compiler/13/ast.cpp

@@ -47,7 +47,7 @@ type_ptr ast_lid::typecheck(type_mgr& mgr, type_env_ptr& env) {
     this->env = env;
     type_scheme_ptr lid_type = env->lookup(id);
     if(!lid_type)
-        throw type_error("unknown identifier " + id, loc);
+        throw type_error(std::string("unknown identifier ") + id, loc);
     return lid_type->instantiate(mgr);
 }
 
@@ -75,7 +75,7 @@ type_ptr ast_uid::typecheck(type_mgr& mgr, type_env_ptr& env) {
     this->env = env;
     type_scheme_ptr uid_type = env->lookup(id);
     if(!uid_type)
-        throw type_error("unknown constructor " + id, loc);
+        throw type_error(std::string("unknown constructor ") + id, loc);
     return uid_type->instantiate(mgr);
 }
 
@@ -105,7 +105,7 @@ type_ptr ast_binop::typecheck(type_mgr& mgr, type_env_ptr& env) {
     type_ptr ltype = left->typecheck(mgr, env);
     type_ptr rtype = right->typecheck(mgr, env);
     type_ptr ftype = env->lookup(op_name(op))->instantiate(mgr);
-    if(!ftype) throw type_error("unknown binary operator " + op_name(op), loc);
+    if(!ftype) throw type_error(std::string("unknown binary operator ") + op_name(op), loc);
 
     // For better type errors, we first require binary function,
     // and only then unify each argument. This way, we can
@@ -438,7 +438,7 @@ void pattern_constr::find_variables(std::set<std::string>& into) const {
 void pattern_constr::typecheck(type_ptr t, type_mgr& mgr, type_env_ptr& env) const {
     type_scheme_ptr constructor_type_scheme = env->lookup(constr);
     if(!constructor_type_scheme) {
-        throw type_error("pattern using unknown constructor " + constr, loc);
+        throw type_error(std::string("pattern using unknown constructor ") + constr, loc);
     }
     type_ptr constructor_type = constructor_type_scheme->instantiate(mgr);
 

code/compiler/13/parsed_type.cpp

@@ -9,10 +9,10 @@ type_ptr parsed_type_app::to_type(
         const type_env& e) const {
     auto parent_type = e.lookup_type(name);
     if(parent_type == nullptr)
-        throw type_error("no such type or type constructor " + name);
+        throw type_error(std::string("no such type or type constructor ") + name);
     type_base* base_type;
     if(!(base_type = dynamic_cast<type_base*>(parent_type.get())))
-        throw type_error("invalid type " + name);
+        throw type_error(std::string("invalid type ") + name);
     if(base_type->arity != arguments.size()) {
         std::ostringstream error_stream;
         error_stream << "invalid application of type ";
@@ -34,7 +34,7 @@ type_ptr parsed_type_var::to_type(
         const std::set<std::string>& vars,
         const type_env& e) const {
     if(vars.find(var) == vars.end())
-        throw type_error("the type variable " + var + " was not explicitly declared.");
+        throw type_error(std::string("the type variable ") + var + std::string(" was not explicitly declared."));
     return type_ptr(new type_var(var));
 }
 

code/compiler/13/type_env.cpp

@@ -1,7 +1,6 @@
 #include "type_env.hpp"
 #include "type.hpp"
 #include "error.hpp"
-#include <cassert>
 
 void type_env::find_free(const type_mgr& mgr, std::set<std::string>& into) const {
     if(parent != nullptr) parent->find_free(mgr, into);
@@ -35,21 +34,17 @@ bool type_env::is_global(const std::string& name) const {
 
 void type_env::set_mangled_name(const std::string& name, const std::string& mangled) {
     auto it = names.find(name);
+    if(it != names.end()) {
+        // Local names shouldn't need mangling.
+        assert(it->second.vis == visibility::global);
 
-    // Can't set mangled name for non-existent variable.
-    assert(it != names.end());
-    // Local names shouldn't need mangling.
-    assert(it->second.vis == visibility::global);
-
-    it->second.mangled_name = mangled;
+        it->second.mangled_name = mangled;
+    }
 }
 
 const std::string& type_env::get_mangled_name(const std::string& name) const {
     auto it = names.find(name);
-    if(it != names.end()) {
-        assert(it->second.mangled_name);
-        return *it->second.mangled_name;
-    }
+    if(it != names.end()) return it->second.mangled_name;
     assert(parent != nullptr);
     return parent->get_mangled_name(name);
 }
@@ -63,7 +58,7 @@ type_ptr type_env::lookup_type(const std::string& name) const {
 
 void type_env::bind(const std::string& name, type_ptr t, visibility v) {
     type_scheme_ptr new_scheme(new type_scheme(std::move(t)));
-    names[name] = variable_data(std::move(new_scheme), v, std::nullopt);
+    names[name] = variable_data(std::move(new_scheme), v, "");
 }
 
 void type_env::bind(const std::string& name, type_scheme_ptr t, visibility v) {

code/compiler/13/type_env.hpp

@@ -2,7 +2,6 @@
 #include <map>
 #include <string>
 #include <set>
-#include <optional>
 #include "graph.hpp"
 #include "type.hpp"
 
@@ -16,11 +15,11 @@ class type_env {
         struct variable_data {
             type_scheme_ptr type;
             visibility vis;
-            std::optional<std::string> mangled_name;
+            std::string mangled_name;
 
             variable_data()
-                : variable_data(nullptr, visibility::local, std::nullopt) {}
-            variable_data(type_scheme_ptr t, visibility v, std::optional<std::string> n)
+                : variable_data(nullptr, visibility::local, "") {}
+            variable_data(type_scheme_ptr t, visibility v, std::string n)
                 : type(std::move(t)), vis(v), mangled_name(std::move(n)) {}
         };
 

content/blog/13_compiler_cleanup/index.md

@@ -10,7 +10,7 @@ and lambda expressions to our compiler. At the end of that post, I mentioned
 that before we move on to bigger and better things, I wanted to take a 
 step back and clean up the compiler. Now is the time to do that.
 
-In particular, I identified four things that could be improved
+In particular, I identified three things that could be improved
 or cleaned up:
 
 * __Error handling__. We need to stop using `throw 0` and start
@@ -535,7 +535,7 @@ In general, this change is also rather mechanical, but, to
 maintain a balance between exceptions and assertions, here
 are a couple more assertions from `type_env`:
 
-{{< codelines "C++" "compiler/13/type_env.cpp" 81 82 >}}
+{{< codelines "C++" "compiler/13/type_env.cpp" 76 77 >}}
 
 Once again, it should not be possible for the compiler
 to try generalize the type of a variable that doesn't
@@ -621,7 +621,7 @@ Now that we've started using assertions, I also think it's worth
 to put our new invariant -- "only global definitions have mangled
 names" -- into code:
 
-{{< codelines "C++" "compiler/13/type_env.cpp" 36 45 >}}
+{{< codelines "C++" "compiler/13/type_env.cpp" 35 43 >}}
 
 Furthermore, we'll _require_ that a global definition
 has a mangled name. This way, we can be more confident
@@ -631,24 +631,11 @@ this, we change `get_mangled_name` to stop
 returning the input string if a mangled name was not
 found; now that we _must_ have a mangled name, doing
 so is effectively obscuring the error. Instead,
-we add two assertions. First, if an environment scope doesn't
-contain a variable, then it _must_ have a parent. 
-If it does contain variable, that variable _must_ have
-a mangled name. We end up with the following:
+we add another assertion: if an environment scope doesn't
+contain a mangled name for a variable, then it _must_
+have a parent. We end up with the following:
 
-{{< codelines "C++" "compiler/13/type_env.cpp" 47 55 >}}
-
-For this to work, we make one more change. Now that we've
-enabled C++17, we have access to `std::optional`. We
-can thus represent the presence or absence of mangled
-names using an optional field, rather than with the empty string `""`.
-I hear that C++ compilers have pretty good
-[empty string optimizations](https://www.youtube.com/watch?v=kPR8h4-qZdk),
-but nonetheless, I think it makes more sense semantically
-to use "absent" (`nullopt`) instead of "empty" (`""`).
-Here's the definition of `type_env::variable_data` now:
-
-{{< codelines "C++" "compiler/13/type_env.hpp" 16 25 >}}
+{{< codelines "C++" "compiler/13/type_env.cpp" 45 51 >}}
 
 Since looking up a mangled name for non-global variable
 will now result in an assertion failure, we have to change
@@ -879,3 +866,7 @@ name with `f_`, much like `create_custom_function`:
 I think that's enough. If we chose to turn more compiler
 data structures into classes, I think we would've quickly drowned
 in one-line getter and setter methods.
+
+{{< todo >}}
+Assertion failure on `set_mangled_name` on non-existent function,
+too. {{< /todo >}}