refactor(macro processor): removing the need for a double apply macro…

… + recursive apply in MacroProcessor::processNode
ArkScript-lang · Nov 23, 2024 · 66b5650 · 66b5650
1 parent 108c941
commit 66b5650
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 21 deletions.
diff --git a/include/Ark/Compiler/Macros/Executor.hpp b/include/Ark/Compiler/Macros/Executor.hpp
@@ -73,6 +73,15 @@ namespace Ark::internal
          */
         [[nodiscard]] const Node* findNearestMacro(const std::string& name) const;
 
+        /**
+         * @brief Apply a macro on a given node
+         * @details Proxy function for MacroProcessor::applyMacro
+         *
+         * @param node
+         * @param depth
+         */
+        void applyMacroProxy(Node& node, unsigned depth);
+
         /**
          * @brief Registers macros based on their type, expand conditional macros
          * @details Validate macros and register them by their name

diff --git a/include/Ark/Compiler/Macros/Processor.hpp b/include/Ark/Compiler/Macros/Processor.hpp
@@ -84,13 +84,6 @@ namespace Ark::internal
          */
         void deleteNearestMacro(const std::string& name);
 
-        /**
-         * @brief Recursively apply macros on a given node
-         *
-         * @param node
-         */
-        void recurApply(Node& node);
-
         /**
          * @brief Check if a given node is a list node, and starts with a Begin
          *

diff --git a/src/arkreactor/Compiler/Macros/Executor.cpp b/src/arkreactor/Compiler/Macros/Executor.cpp
@@ -14,6 +14,11 @@ namespace Ark::internal
         return m_processor->findNearestMacro(name);
     }
 
+    void MacroExecutor::applyMacroProxy(Node& node, unsigned depth)
+    {
+        m_processor->applyMacro(node, depth);
+    }
+
     void MacroExecutor::handleMacroNode(Node& node) const
     {
         m_processor->handleMacroNode(node);

diff --git a/src/arkreactor/Compiler/Macros/Executors/Function.cpp b/src/arkreactor/Compiler/Macros/Executors/Function.cpp
@@ -74,6 +74,7 @@ namespace Ark::internal
                 unify(args_applied, temp_body, nullptr);
 
             node.updateValueAndType(evaluate(temp_body, depth + 1, false));
+            applyMacroProxy(node, depth + 1);
             return true;
         }
 

diff --git a/src/arkreactor/Compiler/Macros/Executors/Symbol.cpp b/src/arkreactor/Compiler/Macros/Executors/Symbol.cpp
@@ -16,6 +16,7 @@ namespace Ark::internal
             {
                 node.updateValueAndType(macro->constList()[1]);
                 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
@@ -132,11 +132,17 @@ namespace Ark::internal
                 else  // running on non-macros
                 {
                     applyMacro(node.list()[pos], 0);
-                    recurApply(node.list()[pos]);  // todo remove it
                     added_begin = isBeginNode(node.list()[pos]) && !had_begin;
 
                     if (node.list()[pos].nodeType() == NodeType::Unused)
                         node.list().erase(node.constList().begin() + static_cast<std::vector<Node>::difference_type>(pos));
+                    else
+                        // 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
+                        // that happens, we can't increment i, otherwise we delete a node,
+                        // advance, resulting in a node being skipped!
+                        ++i;
 
                     // process subnodes if any
                     if (node.nodeType() == NodeType::List && pos < node.constList().size())
@@ -145,10 +151,6 @@ namespace Ark::internal
                         // needed if we created a function node from a macro
                         registerFuncDef(node.list()[pos]);
                     }
-
-                    // go forward only if it isn't a macro, because we delete macros
-                    // while running on the AST
-                    ++i;
                 }
 
                 if (pos < node.constList().size())
@@ -584,15 +586,6 @@ namespace Ark::internal
         }
     }
 
-    void MacroProcessor::recurApply(Node& node)
-    {
-        if (applyMacro(node, 0) && node.isListLike())
-        {
-            for (auto& child : node.list())
-                recurApply(child);
-        }
-    }
-
     bool MacroProcessor::isBeginNode(const Node& node)
     {
         return node.nodeType() == NodeType::List &&