From bbcbd6864670fbb4d7f34c1cee1bca3148a1075a Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Wed, 18 Dec 2024 21:05:00 +0100 Subject: [PATCH 1/3] feat(parser): allow defining non-function macros with a macro call inside --- src/arkreactor/Compiler/AST/Parser.cpp | 21 ++++++++++--------- .../compileTime/bad_macro_arg_list.ark | 2 -- .../compileTime/bad_macro_arg_list.expected | 7 ------- 3 files changed, 11 insertions(+), 19 deletions(-) delete mode 100644 tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.ark delete mode 100644 tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.expected diff --git a/src/arkreactor/Compiler/AST/Parser.cpp b/src/arkreactor/Compiler/AST/Parser.cpp index baebb0ed1..7ba4d896a 100644 --- a/src/arkreactor/Compiler/AST/Parser.cpp +++ b/src/arkreactor/Compiler/AST/Parser.cpp @@ -724,19 +724,20 @@ namespace Ark::internal if (value.has_value()) leaf->push_back(value.value()); + else if (leaf->list().size() == 2) + { + setNodePosAndFilename(leaf->list().back()); + comment.clear(); + if (newlineOrComment(&comment)) + leaf->list().back().attachCommentAfter(comment); + + expect(IsChar(')')); + return leaf; + } else { backtrack(position); - - if (leaf->list().size() == 2) - errorWithNextToken( - fmt::format( - "Expected a body while defined macro `{0}'. If you were trying to define a macro based on a function call, try wrapping it inside a begin node: ($ {0} {{ {1} }})." - "\nWithout the begin node, '{1}' is seen as an argument list.", - symbol, - leaf->list().back().repr())); - else - errorWithNextToken(fmt::format("Expected a value while defining macro `{}'", symbol)); + errorWithNextToken(fmt::format("Expected a value while defining macro `{}'", symbol)); } setNodePosAndFilename(leaf->list().back()); diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.ark b/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.ark deleted file mode 100644 index 9c93a9ae2..000000000 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.ark +++ /dev/null @@ -1,2 +0,0 @@ -($ b ($symcat c)) -(print b) diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.expected deleted file mode 100644 index 3b84bc37f..000000000 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.expected +++ /dev/null @@ -1,7 +0,0 @@ -At ( @ 1:6 - 1 | ($ b ($symcat c)) - | ^ - 2 | (print b) - 3 | - Expected a body while defined macro `b'. If you were trying to define a macro based on a function call, try wrapping it inside a begin node: ($ b { ($symcat c) }). - Without the begin node, '($symcat c)' is seen as an argument list. From b5b58587edf5dfc2b22c38acc5ed699e9090a294 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Wed, 18 Dec 2024 21:11:26 +0100 Subject: [PATCH 2/3] feat(macro processor): improve error messages, evaluate condition result and pre-compute macro values --- include/Ark/Compiler/Macros/Processor.hpp | 12 +- .../Compiler/Macros/Executors/Conditional.cpp | 3 + .../Compiler/Macros/Executors/Symbol.cpp | 2 +- src/arkreactor/Compiler/Macros/Processor.cpp | 112 ++++++++++++++---- .../macro_empty_arity_error.expected | 5 +- .../macro_head_arity_error.expected | 4 +- .../macro_len_arity_error.expected | 4 +- .../macro_paste_arity_error.expected | 5 +- .../macro_symcat_arg_type_error.expected | 2 +- .../macro_symcat_type_error.expected | 7 +- .../macro_tail_arity_error.expected | 4 +- 11 files changed, 115 insertions(+), 45 deletions(-) diff --git a/include/Ark/Compiler/Macros/Processor.hpp b/include/Ark/Compiler/Macros/Processor.hpp index 0eee108c5..1c1daf0af 100644 --- a/include/Ark/Compiler/Macros/Processor.hpp +++ b/include/Ark/Compiler/Macros/Processor.hpp @@ -152,7 +152,17 @@ namespace Ark::internal * @param name the name of the macro being applied * @param kind the macro kind, empty by default (eg "operator", "condition") */ - void checkMacroArgCount(const Node& node, std::size_t expected, const std::string& name, const std::string& kind = ""); + static void checkMacroArgCountEq(const Node& node, std::size_t expected, const std::string& name, const std::string& kind = ""); + + /** + * @brief Check if the given node has at least the provided argument count, otherwise throws an error + * + * @param node a list node with a macro application, eg (= a b) + * @param expected expected argument count, not counting the macro + * @param name the name of the macro being applied + * @param kind the macro kind, empty by default (eg "operator", "condition") + */ + static void checkMacroArgCountGe(const Node& node, std::size_t expected, const std::string& name, const std::string& kind = ""); /** * @brief Evaluate only the macros diff --git a/src/arkreactor/Compiler/Macros/Executors/Conditional.cpp b/src/arkreactor/Compiler/Macros/Executors/Conditional.cpp index b25cc443e..353ed3607 100644 --- a/src/arkreactor/Compiler/Macros/Executors/Conditional.cpp +++ b/src/arkreactor/Compiler/Macros/Executors/Conditional.cpp @@ -22,7 +22,10 @@ namespace Ark::internal } if (node.nodeType() == NodeType::Macro) + { handleMacroNode(node); + applyMacroProxy(node, depth + 1); + } return true; } diff --git a/src/arkreactor/Compiler/Macros/Executors/Symbol.cpp b/src/arkreactor/Compiler/Macros/Executors/Symbol.cpp index 7f8cc4da5..658a6ee00 100644 --- a/src/arkreactor/Compiler/Macros/Executors/Symbol.cpp +++ b/src/arkreactor/Compiler/Macros/Executors/Symbol.cpp @@ -15,7 +15,7 @@ namespace Ark::internal if (macro->constList().size() == 2) { node.updateValueAndType(macro->constList()[1]); - evaluate(node, depth + 1, false); + node = evaluate(node, depth + 1, false); applyMacroProxy(node, depth + 1); return true; } diff --git a/src/arkreactor/Compiler/Macros/Processor.cpp b/src/arkreactor/Compiler/Macros/Processor.cpp index f83a1074f..3bd5c20f0 100644 --- a/src/arkreactor/Compiler/Macros/Processor.cpp +++ b/src/arkreactor/Compiler/Macros/Processor.cpp @@ -59,6 +59,8 @@ namespace Ark::internal if (node.constList().size() == 2) { assert(first_node.nodeType() == NodeType::Symbol && "Can not define a macro without a symbol"); + applyMacro(node.list()[1], 0); + node.list()[1] = evaluate(node.list()[1], 0, true); m_macros.back().add(first_node.string(), node); } // ($ name (args) body) @@ -139,7 +141,7 @@ namespace Ark::internal if (child.nodeType() == NodeType::Unused) node.list().erase(node.constList().begin() + static_cast::difference_type>(pos)); - else + else if (!added_begin) // Go forward only if it isn't a macro, because we delete macros // while running on the AST. Also, applying a macro can result in // nodes being marked unused, and delete them immediately. When @@ -202,7 +204,7 @@ namespace Ark::internal return false; } - void MacroProcessor::checkMacroArgCount(const Node& node, std::size_t expected, const std::string& name, const std::string& kind) + void MacroProcessor::checkMacroArgCountEq(const Node& node, std::size_t expected, const std::string& name, const std::string& kind) { const std::size_t argcount = node.constList().size(); if (argcount != expected + 1) @@ -216,6 +218,20 @@ namespace Ark::internal node); } + void MacroProcessor::checkMacroArgCountGe(const Node& node, std::size_t expected, const std::string& name, const std::string& kind) + { + const std::size_t argcount = node.constList().size(); + if (argcount < expected + 1) + throwMacroProcessingError( + fmt::format( + "Interpreting a `{}'{} with {} arguments, expected at least {}.", + name, + kind.empty() ? kind : " " + kind, + argcount, + expected), + node); + } + Node MacroProcessor::evaluate(Node& node, const unsigned depth, const bool is_not_body) { if (node.nodeType() == NodeType::Symbol) @@ -238,77 +254,109 @@ namespace Ark::internal } else if (name == "=" && is_not_body) { - checkMacroArgCount(node, 2, "=", "condition"); + checkMacroArgCountEq(node, 2, "=", "condition"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); const Node two = evaluate(node.list()[2], depth + 1, is_not_body); return (one == two) ? getTrueNode() : getFalseNode(); } else if (name == "!=" && is_not_body) { - checkMacroArgCount(node, 2, "!=", "condition"); + checkMacroArgCountEq(node, 2, "!=", "condition"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); const Node two = evaluate(node.list()[2], depth + 1, is_not_body); return (one != two) ? getTrueNode() : getFalseNode(); } else if (name == "<" && is_not_body) { - checkMacroArgCount(node, 2, "<", "condition"); + checkMacroArgCountEq(node, 2, "<", "condition"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); const Node two = evaluate(node.list()[2], depth + 1, is_not_body); return (one < two) ? getTrueNode() : getFalseNode(); } else if (name == ">" && is_not_body) { - checkMacroArgCount(node, 2, ">", "condition"); + checkMacroArgCountEq(node, 2, ">", "condition"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); const Node two = evaluate(node.list()[2], depth + 1, is_not_body); return !(one < two) && (one != two) ? getTrueNode() : getFalseNode(); } else if (name == "<=" && is_not_body) { - checkMacroArgCount(node, 2, "<=", "condition"); + checkMacroArgCountEq(node, 2, "<=", "condition"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); const Node two = evaluate(node.list()[2], depth + 1, is_not_body); return one < two || one == two ? getTrueNode() : getFalseNode(); } else if (name == ">=" && is_not_body) { - checkMacroArgCount(node, 2, ">=", "condition"); + checkMacroArgCountEq(node, 2, ">=", "condition"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); const Node two = evaluate(node.list()[2], depth + 1, is_not_body); return !(one < two) ? getTrueNode() : getFalseNode(); } else if (name == "+" && is_not_body) { - checkMacroArgCount(node, 2, "+", "operator"); - const Node one = evaluate(node.list()[1], depth + 1, is_not_body); - const Node two = evaluate(node.list()[2], depth + 1, is_not_body); - return (one.nodeType() == two.nodeType() && two.nodeType() == NodeType::Number) ? Node(one.number() + two.number()) : node; + checkMacroArgCountGe(node, 2, "+", "operator"); + double v = 0.0; + for (auto& child : node.list() | std::ranges::views::drop(1)) + { + Node ev = evaluate(child, depth + 1, is_not_body); + if (ev.nodeType() != NodeType::Number) + return node; + v += ev.number(); + } + return Node(v); } else if (name == "-" && is_not_body) { - checkMacroArgCount(node, 2, "-", "operator"); + checkMacroArgCountGe(node, 2, "-", "operator"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); - const Node two = evaluate(node.list()[2], depth + 1, is_not_body); - return (one.nodeType() == two.nodeType() && two.nodeType() == NodeType::Number) ? Node(one.number() - two.number()) : node; + if (one.nodeType() != NodeType::Number) + return node; + + double v = one.number(); + for (auto& child : node.list() | std::ranges::views::drop(2)) + { + Node ev = evaluate(child, depth + 1, is_not_body); + if (ev.nodeType() != NodeType::Number) + return node; + v -= ev.number(); + } + return Node(v); } else if (name == "*" && is_not_body) { - checkMacroArgCount(node, 2, "*", "operator"); - const Node one = evaluate(node.list()[1], depth + 1, is_not_body); - const Node two = evaluate(node.list()[2], depth + 1, is_not_body); - return (one.nodeType() == two.nodeType() && two.nodeType() == NodeType::Number) ? Node(one.number() * two.number()) : node; + checkMacroArgCountGe(node, 2, "*", "operator"); + double v = 1.0; + for (auto& child : node.list() | std::ranges::views::drop(1)) + { + Node ev = evaluate(child, depth + 1, is_not_body); + if (ev.nodeType() != NodeType::Number) + return node; + v *= ev.number(); + } + return Node(v); } else if (name == "/" && is_not_body) { - checkMacroArgCount(node, 2, "/", "operator"); + checkMacroArgCountGe(node, 2, "/", "operator"); const Node one = evaluate(node.list()[1], depth + 1, is_not_body); - const Node two = evaluate(node.list()[2], depth + 1, is_not_body); - return (one.nodeType() == two.nodeType() && two.nodeType() == NodeType::Number) ? Node(one.number() / two.number()) : node; + if (one.nodeType() != NodeType::Number) + return node; + + double v = one.number(); + for (auto& child : node.list() | std::ranges::views::drop(2)) + { + Node ev = evaluate(child, depth + 1, is_not_body); + if (ev.nodeType() != NodeType::Number) + return node; + v /= ev.number(); + } + return Node(v); } else if (name == "not" && is_not_body) { - checkMacroArgCount(node, 1, "not", "condition"); + checkMacroArgCountEq(node, 1, "not", "condition"); return (!isTruthy(evaluate(node.list()[1], depth + 1, is_not_body))) ? getTrueNode() : getFalseNode(); } else if (name == Language::And && is_not_body) @@ -367,7 +415,7 @@ namespace Ark::internal } else if (name == "@") { - checkMacroArgCount(node, 2, "@"); + checkMacroArgCountEq(node, 2, "@"); Node sublist = evaluate(node.list()[1], depth + 1, is_not_body); const Node idx = evaluate(node.list()[2], depth + 1, is_not_body); @@ -460,7 +508,13 @@ namespace Ark::internal if (node.list().size() <= 2) throwMacroProcessingError(fmt::format("When expanding `{}', expected at least 2 arguments, got {} arguments", Language::Symcat, argcount), node); if (node.list()[1].nodeType() != NodeType::Symbol) - throwMacroProcessingError(fmt::format("When expanding `{}', expected the first argument to be a Symbol, got a {}", Language::Symcat, typeToString(node.list()[1])), node); + throwMacroProcessingError( + fmt::format( + "When expanding `{}', expected the first argument to be a Symbol, got a {}: {}", + Language::Symcat, + typeToString(node.list()[1]), + node.list()[1].repr()), + node); std::string sym = node.list()[1].string(); @@ -481,7 +535,13 @@ namespace Ark::internal break; default: - throwMacroProcessingError(fmt::format("When expanding `{}', expected either a Number, String or Symbol, got a {}", Language::Symcat, typeToString(ev)), ev); + throwMacroProcessingError( + fmt::format( + "When expanding `{}', expected either a Number, String or Symbol, got a {}: {}", + Language::Symcat, + typeToString(ev), + ev.repr()), + ev); } } diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_empty_arity_error.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_empty_arity_error.expected index 95a2b48b8..a50836ec8 100644 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_empty_arity_error.expected +++ b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_empty_arity_error.expected @@ -1,7 +1,6 @@ -At (empty? 1 2) @ 2:8 +At (empty? 1 2) @ 1:7 1 | ($ a (empty? 1 2)) + | ^~~~~~~~~~~~~~~~~~ 2 | (print a) - | ^~~~~~~~~ 3 | - | ^ When expanding `empty?' inside a macro, got 2 arguments, expected 1 diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_head_arity_error.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_head_arity_error.expected index d2a22e2d4..6bea0b6c2 100644 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_head_arity_error.expected +++ b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_head_arity_error.expected @@ -1,6 +1,6 @@ -At (head 1 2) @ 2:8 +At (head 1 2) @ 1:7 1 | ($ a (head 1 2)) + | ^~~~~~~~~~~~~~~~ 2 | (print a) - | ^~~~~~~~~ 3 | When expanding `head' inside a macro, got 2 arguments, expected 1 diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_len_arity_error.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_len_arity_error.expected index f906aa3fe..2d16fea97 100644 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_len_arity_error.expected +++ b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_len_arity_error.expected @@ -1,6 +1,6 @@ -At (len 1 2) @ 2:8 +At (len 1 2) @ 1:7 1 | ($ a (len 1 2)) + | ^~~~~~~~~~~~~~~ 2 | (print a) - | ^~~~~~~~~ 3 | When expanding `len' inside a macro, got 2 arguments, expected 1 diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_paste_arity_error.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_paste_arity_error.expected index 2063ceea7..cbd6f97b1 100644 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_paste_arity_error.expected +++ b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_paste_arity_error.expected @@ -1,7 +1,6 @@ -At ($paste b (list)) @ 2:8 +At ($paste b (list)) @ 1:7 1 | ($ a ($paste b [])) + | ^~~~~~~~~~~~~~~~~~~ 2 | (print a) - | ^~~~~~~~~ 3 | - | ^ When expanding `$paste', expected one argument, got 2 arguments diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_arg_type_error.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_arg_type_error.expected index 0f8e069fc..369d68cbb 100644 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_arg_type_error.expected +++ b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_arg_type_error.expected @@ -3,4 +3,4 @@ At (list) @ 1:17 | ^~~~~~~~~~~~~~~~~~~~ 2 | (print a) 3 | - When expanding `$symcat', expected either a Number, String or Symbol, got a List + When expanding `$symcat', expected either a Number, String or Symbol, got a List: (list) diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_type_error.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_type_error.expected index 8d8f43f3a..6460fe57c 100644 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_type_error.expected +++ b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_symcat_type_error.expected @@ -1,7 +1,6 @@ -At ($symcat 5 2) @ 2:8 +At ($symcat 5 2) @ 1:7 1 | ($ a ($symcat 5 2)) + | ^~~~~~~~~~~~~~~~~~~ 2 | (print a) - | ^~~~~~~~~ 3 | - | ^ - When expanding `$symcat', expected the first argument to be a Symbol, got a Number + When expanding `$symcat', expected the first argument to be a Symbol, got a Number: 5 diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_tail_arity_error.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_tail_arity_error.expected index eb08ce872..3646993cc 100644 --- a/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_tail_arity_error.expected +++ b/tests/unittests/resources/DiagnosticsSuite/compileTime/macro_tail_arity_error.expected @@ -1,6 +1,6 @@ -At (tail 1 2) @ 2:8 +At (tail 1 2) @ 1:7 1 | ($ a (tail 1 2)) + | ^~~~~~~~~~~~~~~~ 2 | (print a) - | ^~~~~~~~~ 3 | When expanding `tail' inside a macro, got 2 arguments, expected 1 From 146a27bea668db0360cbe94b071e2f484af09da5 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Mon, 23 Dec 2024 14:21:17 +0100 Subject: [PATCH 3/3] feat(tests): adding more macros tests --- tests/unittests/resources/LangSuite/macro-tests.ark | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/unittests/resources/LangSuite/macro-tests.ark b/tests/unittests/resources/LangSuite/macro-tests.ark index faef3ddf5..5ad8a839a 100644 --- a/tests/unittests/resources/LangSuite/macro-tests.ark +++ b/tests/unittests/resources/LangSuite/macro-tests.ark @@ -31,17 +31,28 @@ (test:eq (c 4 10) 40) ($ d (a b) (/ a b)) - (test:eq (d 10 2) 5) }) + (test:eq (d 10 2) 5) + + ($ e (+ 1 2 3)) + ($ f (* 2 2 3)) + ($ g (- 1 2 3)) + ($ h (/ 1 2 3)) + (test:eq e 6) + (test:eq f 12) + (test:eq g -4) + (test:eq h (/ 1 6)) }) (test:case "node manipulation" { ($ node_tail () (tail (begin 1 2 3))) ($ length () (len (fun () 5))) ($ not_empty_node () (empty? (fun () ()))) ($ empty_node () (empty? ())) + ($ empty_node_bis (empty? ())) (test:eq (length) 3) (test:eq (not_empty_node) false) (test:eq (empty_node) true) + (test:eq empty_node_bis true) # because it removes the "begin" (test:eq (node_tail) [1 2 3]) })