From 97eb4b6e3ea42e78495e673c2cc4c38f3776f837 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Fri, 18 Sep 2020 12:02:37 -0700 Subject: [PATCH] Fix silent error in set_mangled_name --- code/compiler/13/type_env.cpp | 11 ++++++----- content/blog/13_compiler_cleanup/index.md | 10 +++------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/code/compiler/13/type_env.cpp b/code/compiler/13/type_env.cpp index 6f4fe6a..0c0ddef 100644 --- a/code/compiler/13/type_env.cpp +++ b/code/compiler/13/type_env.cpp @@ -34,12 +34,13 @@ 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); - it->second.mangled_name = mangled; - } + // 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; } const std::string& type_env::get_mangled_name(const std::string& name) const { diff --git a/content/blog/13_compiler_cleanup/index.md b/content/blog/13_compiler_cleanup/index.md index 72d56a8..ee28252 100644 --- a/content/blog/13_compiler_cleanup/index.md +++ b/content/blog/13_compiler_cleanup/index.md @@ -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" 76 77 >}} +{{< codelines "C++" "compiler/13/type_env.cpp" 77 78 >}} 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" 35 43 >}} +{{< codelines "C++" "compiler/13/type_env.cpp" 35 44 >}} Furthermore, we'll _require_ that a global definition has a mangled name. This way, we can be more confident @@ -635,7 +635,7 @@ 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" 45 51 >}} +{{< codelines "C++" "compiler/13/type_env.cpp" 46 52 >}} Since looking up a mangled name for non-global variable will now result in an assertion failure, we have to change @@ -866,7 +866,3 @@ 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 >}}