From eb30fc88b0982c37a16f23d0545b93016c665e7b Mon Sep 17 00:00:00 2001 From: water Date: Sun, 22 Nov 2020 20:10:33 -0500 Subject: [PATCH] fix compiler bugs --- common/versions.h | 2 +- doc/changelog.md | 7 ++- doc/goal_doc.md | 5 +- goal_src/kernel/gcommon.gc | 2 +- goalc/compiler/Env.cpp | 5 ++ goalc/compiler/Env.h | 1 + goalc/compiler/compilation/Block.cpp | 16 +++--- goalc/compiler/compilation/Function.cpp | 50 ++++++++++++++++--- goalc/compiler/compilation/Type.cpp | 1 - .../function-returning-none.static.gc | 6 +++ .../functions/inline-with-block-1.static.gc | 11 ++++ .../functions/inline-with-block-2.static.gc | 11 ++++ .../functions/inline-with-block-3.static.gc | 10 ++++ .../functions/inline-with-block-4.static.gc | 14 ++++++ .../functions/return-from-trick.static.gc | 8 +++ test/goalc/test_functions.cpp | 24 +++++++++ 16 files changed, 154 insertions(+), 19 deletions(-) create mode 100644 test/goalc/source_templates/functions/function-returning-none.static.gc create mode 100644 test/goalc/source_templates/functions/inline-with-block-1.static.gc create mode 100644 test/goalc/source_templates/functions/inline-with-block-2.static.gc create mode 100644 test/goalc/source_templates/functions/inline-with-block-3.static.gc create mode 100644 test/goalc/source_templates/functions/inline-with-block-4.static.gc create mode 100644 test/goalc/source_templates/functions/return-from-trick.static.gc diff --git a/common/versions.h b/common/versions.h index 82caa57f9..89267cc0d 100644 --- a/common/versions.h +++ b/common/versions.h @@ -13,7 +13,7 @@ namespace versions { // language version constexpr s32 GOAL_VERSION_MAJOR = 0; -constexpr s32 GOAL_VERSION_MINOR = 1; +constexpr s32 GOAL_VERSION_MINOR = 2; constexpr u32 ART_FILE_VERSION = 6; constexpr u32 LEVEL_FILE_VERSION = 30; constexpr u32 DGO_FILE_VERSION = 1; diff --git a/doc/changelog.md b/doc/changelog.md index 799671ebc..53378171c 100644 --- a/doc/changelog.md +++ b/doc/changelog.md @@ -46,4 +46,9 @@ - Static fields can now contain floating point values - Fixed a bug where loading a float from an object and immediately using it math would cause a compiler crash -- Arrays of value types can be created on the stack with `new`. \ No newline at end of file +- Arrays of value types can be created on the stack with `new`. + +## V0.2 +- Breaking change: return type of a function using `return-from #f` to return a value from the entire function is now the lowest common ancestor of all possible return values. +- Fixed bug where `return-from` could reach outside of an inlined function. +- Fixed bug where `return-from` might not behave correctly when returning from inside a let inside an inlined function. \ No newline at end of file diff --git a/doc/goal_doc.md b/doc/goal_doc.md index 3e855068b..1c32e28d5 100644 --- a/doc/goal_doc.md +++ b/doc/goal_doc.md @@ -66,7 +66,7 @@ Exit a `block` or function early. (return-from block-name value) ``` -Looks up the block and exits from it with the value. You can exit out nested blocks. If you are enclosed in multiple blocks with the same name, exits from the inner-most one with a matching name. Everything in a function is wrapped in a block named `#f`, so you can use `(return-from #f x)` to return early from a function with `x`. Unlike returning from a block, using `return-from` to exit a function currently does _not_ modify the return type of your function and does _not_ check the type of what you return. +Looks up the block and exits from it with the value. You can exit out nested blocks. If you are enclosed in multiple blocks with the same name, exits from the inner-most one with a matching name. Everything in a function is wrapped in a block named `#f`, so you can use `(return-from #f x)` to return early from a function with `x`. When using this form, it may change the return type of the function or block. The return type will be the lowest common ancestor type of all written return paths. If there is an unreachable return path, it will still be considered. Example ```lisp @@ -109,6 +109,9 @@ Example: The `goto` form used very rarely outside of macros and inline assembly. Try to avoid using `goto`. +## `top-level` +This form is reserved by the compiler. Internally all forms in a file are grouped under a `top-level` form, so you may see this in error messages. Do not name anything `top-level`. + # Compiler Forms - Compiler Control These forms are used to control the GOAL compiler, and are usually entered at the GOAL REPL, or as part of a macro that's executed at the GOAL REPL. These shouldn't really be used in GOAL source code. diff --git a/goal_src/kernel/gcommon.gc b/goal_src/kernel/gcommon.gc index a560f8930..4331a8d4c 100644 --- a/goal_src/kernel/gcommon.gc +++ b/goal_src/kernel/gcommon.gc @@ -477,7 +477,7 @@ "Delete the first occurance of item from a list and return the list. Does nothing if the item isn't in the list." (if (eq? (car lst) item) - (return-from #f (cdr lst)) + (return-from #f (the pair (cdr lst))) ) (let ((iter (cdr lst)) diff --git a/goalc/compiler/Env.cpp b/goalc/compiler/Env.cpp index 43db177cf..3534b13c0 100644 --- a/goalc/compiler/Env.cpp +++ b/goalc/compiler/Env.cpp @@ -248,6 +248,11 @@ std::unordered_map& LabelEnv::get_label_map() { return m_labels; } +BlockEnv* LabelEnv::find_block(const std::string& name) { + (void)name; + return nullptr; +} + RegVal* FunctionEnv::lexical_lookup(goos::Object sym) { if (!sym.is_symbol()) { throw std::runtime_error("invalid symbol in lexical_lookup"); diff --git a/goalc/compiler/Env.h b/goalc/compiler/Env.h index e49800815..4cec30189 100644 --- a/goalc/compiler/Env.h +++ b/goalc/compiler/Env.h @@ -234,6 +234,7 @@ class LabelEnv : public Env { explicit LabelEnv(Env* parent) : Env(parent) {} std::string print() override { return "labelenv"; } std::unordered_map& get_label_map() override; + BlockEnv* find_block(const std::string& name) override; protected: std::unordered_map m_labels; diff --git a/goalc/compiler/compilation/Block.cpp b/goalc/compiler/compilation/Block.cpp index 60f68beb1..1be182bc3 100644 --- a/goalc/compiler/compilation/Block.cpp +++ b/goalc/compiler/compilation/Block.cpp @@ -84,14 +84,16 @@ Val* Compiler::compile_block(const goos::Object& form, const goos::Object& _rest auto return_type = m_ts.lowest_common_ancestor(return_types); block_env->return_value->set_type(coerce_to_reg_type(return_type)); - // an IR to move the result of the block into the block's return register (if no return-from's are - // taken) - auto ir_move_rv = std::make_unique(block_env->return_value, result->to_gpr(fe)); + if (!dynamic_cast(result)) { + // an IR to move the result of the block into the block's return register (if no return-from's + // are taken) + auto ir_move_rv = std::make_unique(block_env->return_value, result->to_gpr(fe)); - // note - one drawback of doing this single pass is that a block always evaluates to a gpr. - // so we may have an unneeded xmm -> gpr move that could have been an xmm -> xmm that could have - // been eliminated. - env->emit(std::move(ir_move_rv)); + // note - one drawback of doing this single pass is that a block always evaluates to a gpr. + // so we may have an unneeded xmm -> gpr move that could have been an xmm -> xmm that could have + // been eliminated. + env->emit(std::move(ir_move_rv)); + } // now we know the end of the block, so we set the label index to be on whatever comes after the // return move. functions always end with a "null" IR and "null" instruction, so this is safe. diff --git a/goalc/compiler/compilation/Function.cpp b/goalc/compiler/compilation/Function.cpp index c82d3824b..1807f865d 100644 --- a/goalc/compiler/compilation/Function.cpp +++ b/goalc/compiler/compilation/Function.cpp @@ -182,13 +182,16 @@ Val* Compiler::compile_lambda(const goos::Object& form, const goos::Object& rest new_func_env->settings.is_set = true; } }); - if (result) { + if (result && !dynamic_cast(result)) { // got a result, so to_gpr it and return it. auto final_result = result->to_gpr(new_func_env.get()); new_func_env->emit(std::make_unique(return_reg, final_result)); - lambda_ts.add_arg(final_result->type()); + func_block_env->return_types.push_back(final_result->type()); + auto return_type = m_ts.lowest_common_ancestor(func_block_env->return_types); + lambda_ts.add_arg(return_type); + // lambda_ts.add_arg(final_result->type()); } else { - // empty body, return none + // empty body or returning none, return none lambda_ts.add_arg(m_ts.make_typespec("none")); } // put null instruction at the end so jumps to the end have somewhere to go. @@ -316,13 +319,13 @@ Val* Compiler::compile_function_or_method_call(const goos::Object& form, Env* en // construct a lexical environment auto lexical_env = fe->alloc_env(env); - Env* compile_env = lexical_env; + Env* inlined_compile_env = lexical_env; // if need to, create a label env. // we don't want a separate label env with lets, but we do for inlined functions. // either inlined through the auto-inliner, or through an explicit (inline x) form. if (auto_inline || got_inlined_lambda) { - compile_env = fe->alloc_env(lexical_env); + inlined_compile_env = fe->alloc_env(lexical_env); } // check arg types @@ -350,13 +353,25 @@ Val* Compiler::compile_function_or_method_call(const goos::Object& form, Env* en lexical_env->vars[head_as_lambda->lambda.params.at(i).name] = copy; } + // setup env + BlockEnv* inlined_block_env = nullptr; + RegVal* result_reg_if_return_from = nullptr; + if (auto_inline || got_inlined_lambda) { + inlined_block_env = fe->alloc_env(inlined_compile_env, "#f"); + result_reg_if_return_from = + inlined_compile_env->make_ireg(get_none()->type(), emitter::RegKind::GPR); + inlined_block_env->return_value = result_reg_if_return_from; + inlined_block_env->end_label = Label(fe); + inlined_compile_env = inlined_block_env; + } + // compile inline! bool first_thing = true; Val* result = get_none(); for_each_in_list(head_as_lambda->lambda.body, [&](const goos::Object& o) { - result = compile_error_guard(o, compile_env); + result = compile_error_guard(o, inlined_compile_env); if (!dynamic_cast(result)) { - result = result->to_reg(compile_env); + result = result->to_reg(inlined_compile_env); } if (first_thing) { first_thing = false; @@ -366,6 +381,27 @@ Val* Compiler::compile_function_or_method_call(const goos::Object& form, Env* en // ignore the user specified return type and return the most specific type. // todo - does this make sense for an inline function? Should we check the return type? + + if (inlined_block_env && !inlined_block_env->return_types.empty()) { + // there were return froms used in the function, so we fall back to using the separate + // return gpr. + if (!dynamic_cast(result)) { + auto final_result = result->to_gpr(inlined_compile_env); + inlined_compile_env->emit( + std::make_unique(result_reg_if_return_from, final_result)); + inlined_block_env->return_types.push_back(final_result->type()); + auto return_type = m_ts.lowest_common_ancestor(inlined_block_env->return_types); + inlined_block_env->return_value->set_type(return_type); + } else { + inlined_block_env->return_value->set_type(get_none()->type()); + } + + inlined_compile_env->emit(std::make_unique()); + inlined_block_env->end_label.idx = inlined_block_env->end_label.func->code().size(); + return inlined_block_env->return_value; + } + + inlined_compile_env->emit(std::make_unique()); return result; } else { // not an inlined/immediate, it's a real function call. diff --git a/goalc/compiler/compilation/Type.cpp b/goalc/compiler/compilation/Type.cpp index aabbf9642..4bd5855a4 100644 --- a/goalc/compiler/compilation/Type.cpp +++ b/goalc/compiler/compilation/Type.cpp @@ -654,7 +654,6 @@ Val* Compiler::compile_new(const goos::Object& form, const goos::Object& _rest, } else if (allocation == "stack") { auto type_of_object = m_ts.make_typespec(type_as_string); - printf("type as string is %s\n", type_as_string.c_str()); if (type_as_string == "inline-array" || type_as_string == "array") { bool is_inline = type_as_string == "inline-array"; auto elt_type = quoted_sym_as_string(pair_car(*rest)); diff --git a/test/goalc/source_templates/functions/function-returning-none.static.gc b/test/goalc/source_templates/functions/function-returning-none.static.gc new file mode 100644 index 000000000..3e1369e1a --- /dev/null +++ b/test/goalc/source_templates/functions/function-returning-none.static.gc @@ -0,0 +1,6 @@ +(defun function-returning-none () + (none) + ) + +(function-returning-none) +1 \ No newline at end of file diff --git a/test/goalc/source_templates/functions/inline-with-block-1.static.gc b/test/goalc/source_templates/functions/inline-with-block-1.static.gc new file mode 100644 index 000000000..5c799a952 --- /dev/null +++ b/test/goalc/source_templates/functions/inline-with-block-1.static.gc @@ -0,0 +1,11 @@ + +; this is testing that return-from's work correctly inside of inlined functions. +(defun inline-with-block-1 () + (declare (inline)) + (if (= 123 (block my-block 1 (return-from my-block 123) 2)) + 1 + 0 + ) + ) + +(inline-with-block-1) \ No newline at end of file diff --git a/test/goalc/source_templates/functions/inline-with-block-2.static.gc b/test/goalc/source_templates/functions/inline-with-block-2.static.gc new file mode 100644 index 000000000..ca8140865 --- /dev/null +++ b/test/goalc/source_templates/functions/inline-with-block-2.static.gc @@ -0,0 +1,11 @@ +;; testing that we can have a block's final statement be a none. + +(defun inline-with-block-2 () + (declare (inline)) + (block a-block + (return-from a-block 3) + ) + ) + +;; this is a little hacky, this technically returns none. +(inline-with-block-2) \ No newline at end of file diff --git a/test/goalc/source_templates/functions/inline-with-block-3.static.gc b/test/goalc/source_templates/functions/inline-with-block-3.static.gc new file mode 100644 index 000000000..6cfb7a69e --- /dev/null +++ b/test/goalc/source_templates/functions/inline-with-block-3.static.gc @@ -0,0 +1,10 @@ +(defun inline-with-block-3 () + (declare (inline)) + (block a-block + (return-from a-block 4) + ) + (none) + ) + +(inline-with-block-3) +4 \ No newline at end of file diff --git a/test/goalc/source_templates/functions/inline-with-block-4.static.gc b/test/goalc/source_templates/functions/inline-with-block-4.static.gc new file mode 100644 index 000000000..a95342b0f --- /dev/null +++ b/test/goalc/source_templates/functions/inline-with-block-4.static.gc @@ -0,0 +1,14 @@ +(define format _format) + +(defun even-odd-to-1-2 ((in int)) + (declare (inline)) + (if (= 0 (logand 1 in)) + (let ((x 1.0)) + (return-from #f 1.0) + ) + 123123.23 + ) + 2.0 + ) + +(format #t "~f~%" (+ (even-odd-to-1-2 11) (even-odd-to-1-2 12))) diff --git a/test/goalc/source_templates/functions/return-from-trick.static.gc b/test/goalc/source_templates/functions/return-from-trick.static.gc new file mode 100644 index 000000000..b18ed9b14 --- /dev/null +++ b/test/goalc/source_templates/functions/return-from-trick.static.gc @@ -0,0 +1,8 @@ +(defun return-from-let () + (let ((x 1)) + (return-from #f x) + ) + 3 + ) + +(return-from-let) \ No newline at end of file diff --git a/test/goalc/test_functions.cpp b/test/goalc/test_functions.cpp index 7dac93e89..dc93dd8b9 100644 --- a/test/goalc/test_functions.cpp +++ b/test/goalc/test_functions.cpp @@ -114,3 +114,27 @@ TEST_F(FunctionTests, AllowInline) { EXPECT_EQ(got_mult, 1); EXPECT_EQ(got_call, 1); } + +TEST_F(FunctionTests, ReturnNone) { + runner.run_static_test(env, testCategory, "function-returning-none.static.gc", {"1\n"}); +} + +TEST_F(FunctionTests, InlineBlock1) { + runner.run_static_test(env, testCategory, "inline-with-block-1.static.gc", {"1\n"}); +} + +TEST_F(FunctionTests, InlineBlock2) { + runner.run_static_test(env, testCategory, "inline-with-block-2.static.gc", {"3\n"}); +} + +TEST_F(FunctionTests, InlineBlock3) { + runner.run_static_test(env, testCategory, "inline-with-block-3.static.gc", {"4\n"}); +} + +TEST_F(FunctionTests, InlineBlock4) { + runner.run_static_test(env, testCategory, "inline-with-block-4.static.gc", {"3.0000\n0\n"}); +} + +TEST_F(FunctionTests, ReturnFromTrick) { + runner.run_static_test(env, testCategory, "return-from-trick.static.gc", {"1\n"}); +} \ No newline at end of file