From 4f454581caeb7454e551054383d0cfe6a4158237 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Mon, 2 Dec 2024 18:39:28 +0100 Subject: [PATCH] feat(name resolution): somewhat better handling of closures and captures --- include/Ark/VM/Value/Closure.hpp | 9 +++++++++ src/arkreactor/VM/VM.cpp | 17 ++++++++++++++++- src/arkreactor/VM/Value/Closure.cpp | 12 ++++++++++++ .../runtime/closure_field_wrong_fqn.ark | 1 + .../runtime/closure_field_wrong_fqn.expected | 1 + .../runtime/closure_field_wrong_fqn/b.ark | 10 ++++++++++ .../resources/LangSuite/list-tests.ark | 10 +++++----- .../unittests/resources/LangSuite/vm-tests.ark | 4 ++-- 8 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.ark create mode 100644 tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.expected create mode 100644 tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn/b.ark diff --git a/include/Ark/VM/Value/Closure.hpp b/include/Ark/VM/Value/Closure.hpp index 994bf0d2d..e8ac77a79 100644 --- a/include/Ark/VM/Value/Closure.hpp +++ b/include/Ark/VM/Value/Closure.hpp @@ -61,6 +61,15 @@ namespace Ark::internal */ [[nodiscard]] PageAddr_t pageAddr() const { return m_page_addr; } + /** + * @brief Used when generating error messages in the VM, to see if a symbol might have been wrongly fully qualified + * + * @param end + * @param vm + * @return true if the closure has a field which is the end of 'end' + */ + [[nodiscard]] bool hasFieldEndingWith(const std::string& end, VM& vm) const; + /** * @brief Print the closure to a string * diff --git a/src/arkreactor/VM/VM.cpp b/src/arkreactor/VM/VM.cpp index baebf5726..d3d11a48e 100644 --- a/src/arkreactor/VM/VM.cpp +++ b/src/arkreactor/VM/VM.cpp @@ -619,7 +619,22 @@ namespace Ark push(field, context); } else - throwVMError(ErrorKind::Scope, fmt::format("`{}' isn't in the closure environment: {}", m_state.m_symbols[arg], var->refClosure().toString(*this))); + { + if (!var->refClosure().hasFieldEndingWith(m_state.m_symbols[arg], *this)) + throwVMError( + ErrorKind::Scope, + fmt::format( + "`{0}' isn't in the closure environment: {1}", + m_state.m_symbols[arg], + var->refClosure().toString(*this))); + throwVMError( + ErrorKind::Scope, + fmt::format( + "`{0}' isn't in the closure environment: {1}. A variable in the package might have the same name as '{0}', " + "and name resolution tried to fully qualify it. Rename either the variable or the capture to solve this", + m_state.m_symbols[arg], + var->refClosure().toString(*this))); + } DISPATCH(); } diff --git a/src/arkreactor/VM/Value/Closure.cpp b/src/arkreactor/VM/Value/Closure.cpp index c8cc8caec..0631eac8b 100644 --- a/src/arkreactor/VM/Value/Closure.cpp +++ b/src/arkreactor/VM/Value/Closure.cpp @@ -4,6 +4,8 @@ #include #include +#include + namespace Ark::internal { Closure::Closure(const Scope& scope, const PageAddr_t pa) noexcept : @@ -16,6 +18,16 @@ namespace Ark::internal m_page_addr(pa) {} + bool Closure::hasFieldEndingWith(const std::string& end, VM& vm) const + { + for (const auto id : std::ranges::views::keys(m_scope->m_data)) + { + if (end.ends_with(":" + vm.m_state.m_symbols[id])) + return true; + } + return false; + } + std::string Closure::toString(VM& vm) const noexcept { std::string out = "("; diff --git a/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.ark b/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.ark new file mode 100644 index 000000000..2de7cb2bb --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.ark @@ -0,0 +1 @@ +(import closure_field_wrong_fqn.b) diff --git a/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.expected b/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.expected new file mode 100644 index 000000000..c4922e9fc --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn.expected @@ -0,0 +1 @@ +ScopeError: `b:b' isn't in the closure environment: (.a=hello .b=1). A variable in the package might have the same name as 'b:b', and name resolution tried to fully qualify it. Rename either the variable or the capture to solve this diff --git a/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn/b.ark b/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn/b.ark new file mode 100644 index 000000000..a0a1d40de --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/runtime/closure_field_wrong_fqn/b.ark @@ -0,0 +1,10 @@ +(let tests 0) +(let closure (fun (&tests) ())) + +(let a 1) +(let b 2) + +(let make (fun (a b) + (fun (&a &b) ()))) +(let foo (make "hello" 1)) +(let c [foo.a foo.b]) diff --git a/tests/unittests/resources/LangSuite/list-tests.ark b/tests/unittests/resources/LangSuite/list-tests.ark index 0da2ea6ff..e22ff7b65 100644 --- a/tests/unittests/resources/LangSuite/list-tests.ark +++ b/tests/unittests/resources/LangSuite/list-tests.ark @@ -4,14 +4,14 @@ (let b [4 5 6]) (test:suite list { - (let make (fun (a b) - (fun (&a &b) ()))) + (let make (fun (c d) + (fun (&c &d) ()))) (let foo (make "hello" 1)) # if this is failing, this is most likely to be a compiler problem - (test:eq ["hello" 1] [foo.a foo.b]) - (test:eq 2 (len [foo.a foo.b])) - (test:eq ["hello"] (append [] foo.a)) + (test:eq ["hello" 1] [foo.c foo.d]) + (test:eq 2 (len [foo.c foo.d])) + (test:eq ["hello"] (append [] foo.c)) (test:case "append and return a new list" { (test:eq (append a 4) [1 2 3 4]) diff --git a/tests/unittests/resources/LangSuite/vm-tests.ark b/tests/unittests/resources/LangSuite/vm-tests.ark index 2f6a689f8..12af3af6c 100644 --- a/tests/unittests/resources/LangSuite/vm-tests.ark +++ b/tests/unittests/resources/LangSuite/vm-tests.ark @@ -126,11 +126,11 @@ (test:eq (type nil) "Nil") (test:eq (type true) "Bool") (test:eq (type false) "Bool") - (test:expect (hasField closure "tests")) + (test:expect (hasField closure "vm-tests:tests")) (test:expect (not (hasField closure "12"))) }) (test:case "closures" { - (test:eq (toString closure) "(.tests=0)") + (test:eq (toString closure) "(.vm-tests:tests=0)") (test:eq closure_1 closure_1_bis) (test:eq closure_1 closure_2) (test:neq closure_1 closure_4)