Resolved circular depependancy between macro expander and binder

Src/AST/Myc.Ast.Compiler.Macros.pas

@@ -20,9 +20,14 @@ type
         function Execute(const RootNode: IAstNode): IAstNode;
     end;
 
-    TEvaluateProc = reference to function(const Node: IAstNode): TDataValue;
+    // Callback used by TMacroExpander to request full compilation and execution of macro arguments.
+    // This abstracts away the specific Binder, TypeChecker, and Evaluator implementations.
+    TMacroEvaluatorProc = reference to function(const Scope: IExecutionScope; const Node: IAstNode): TDataValue;
 
+    // Handles the expansion of the macro body template (Quasiquotes/Unquotes).
     TExpansionVisitor = class(TAstTransformer)
+        type
+            TEvaluateProc = reference to function(const Node: IAstNode): TDataValue;
     private
         FEvaluate: TEvaluateProc;
         FMacroScope: IExecutionScope;
@@ -49,11 +54,12 @@ type
         class function Expand(const MacroScope: IExecutionScope; const RootNode: IAstNode; const AEvaluate: TEvaluateProc): IAstNode;
     end;
 
+    // Handles the expansion of macro calls within the AST.
     TMacroExpander = class(TAstTransformer, IAstMacroExpander)
     private
         FInitialScope: IExecutionScope;
         FCurrentMacroRegistry: IMacroRegistry;
-        FEvaluatorFactory: TEvaluatorFactory;
+        FMacroEvaluator: TMacroEvaluatorProc;
 
         procedure EnterMacroScope;
         procedure ExitMacroScope;
@@ -73,7 +79,7 @@ type
         constructor Create(
             const ARootRegistry: IMacroRegistry;
             const AInitialScope: IExecutionScope;
-            const AEvaluatorFactory: TEvaluatorFactory
+            const AMacroEvaluator: TMacroEvaluatorProc
         );
         destructor Destroy; override;
 
@@ -83,10 +89,11 @@ type
             const ARootRegistry: IMacroRegistry;
             const AInitialScope: IExecutionScope;
             const RootNode: IAstNode;
-            const EvaluatorFactory: TEvaluatorFactory
+            const MacroEvaluator: TMacroEvaluatorProc
         ): IAstNode; static;
     end;
 
+    // Implementation of the macro registry interface defined in Myc.Ast.Environment.
     TMacroRegistry = class(TInterfacedObject, IMacroRegistry)
     private
         FParent: IMacroRegistry;
@@ -105,8 +112,6 @@ implementation
 uses
     System.Generics.Defaults,
     System.SyncObjs,
-    Myc.Ast.Compiler.Binder,
-    Myc.Ast.Evaluator,
     Myc.Data.Keyword;
 
 { TExpansionVisitor }
@@ -127,6 +132,8 @@ end;
 
 function TExpansionVisitor.Gensym(const ABaseName: string): string;
 begin
+    // Hygienic macros: Generate unique names for identifiers introduced by the macro
+    // that were not captured from the arguments.
     if not FRenameMap.TryGetValue(ABaseName, Result) then
     begin
         var count := TInterlocked.Add(FGensymCounter, 1);
@@ -169,6 +176,7 @@ begin
                 if (not evaluatedSpliceValue.IsVoid) and (evaluatedSpliceValue.Kind = vkInterface) then
                 begin
                     nodeToSplice := evaluatedSpliceValue.AsIntf<IAstNode>;
+                    // If the result is a block, flatten it into the current list
                     if nodeToSplice.Kind = akBlockExpression then
                         newList.AddRange(nodeToSplice.AsBlockExpression.Expressions)
                     else
@@ -176,6 +184,7 @@ begin
                 end
                 else if (not evaluatedSpliceValue.IsVoid) then
                 begin
+                    // Treat scalar values as constants in the AST
                     newList.Add(TAst.Constant(evaluatedSpliceValue));
                 end;
             end
@@ -196,6 +205,7 @@ function TExpansionVisitor.VisitIdentifier(const Node: IIdentifierNode): IAstNod
 var
     newName: string;
 begin
+    // Apply hygienic renaming
     if FRenameMap.TryGetValue(Node.Name, newName) then
     begin
         Result := TAst.Identifier(newName, TTypes.Unknown);
@@ -219,7 +229,8 @@ begin
     end
     else
     begin
-        newTarget := Accept(Node.Target); // Recursively expand unquotes in target position!
+        // Recursively expand unquotes in target position (advanced usage)
+        newTarget := Accept(Node.Target);
     end;
 
     Result := TAst.VarDecl(newTarget, newInit, TTypes.Unknown);
@@ -232,6 +243,7 @@ var
     newName: string;
     i: Integer;
 begin
+    // Parameters in a macro body must also be renamed hygienically
     SetLength(newParams, Length(Node.Parameters));
     for i := 0 to High(Node.Parameters) do
     begin
@@ -252,6 +264,8 @@ var
 begin
     expr := Node.Expression;
 
+    // Optimization: If unquoting a simple identifier that exists in the macro scope,
+    // try to resolve it directly to avoid unnecessary evaluation overhead.
     if expr.Kind = akIdentifier then
     begin
         addr := FMacroScope.Resolve(expr.AsIdentifier.Name);
@@ -267,14 +281,17 @@ begin
         end;
     end;
 
+    // Evaluate the expression at compile time
     value := FEvaluate(expr);
 
     if value.Kind = vkInterface then
     begin
+        // It returned an AST node -> Inject it
         Result := value.AsIntf<IAstNode>;
         exit;
     end;
 
+    // It returned a value -> Wrap as Constant
     if value.Kind in [vkScalar, vkText, vkVoid] then
         Result := TAst.Constant(value)
     else
@@ -283,6 +300,7 @@ end;
 
 function TExpansionVisitor.VisitUnquoteSplicing(const Node: IUnquoteSplicingNode): IAstNode;
 begin
+    // ~@ can only be handled within a list context (TransformAndSpliceNodes)
     raise Exception.Create('Unquote-splicing (`~@`) can only appear inside a list form.');
 end;
 
@@ -292,6 +310,7 @@ var
     transformedCallee: IAstNode;
 begin
     transformedCallee := Self.Accept(Node.Callee);
+    // Use special splicing logic for argument lists
     newArgs := TransformAndSpliceNodes(Node.Arguments);
     Result := TAst.FunctionCall(transformedCallee, newArgs);
 end;
@@ -300,12 +319,14 @@ function TExpansionVisitor.VisitBlockExpression(const Node: IBlockExpressionNode
 var
     newExprs: TArray<IAstNode>;
 begin
+    // Use special splicing logic for block expressions
     newExprs := TransformAndSpliceNodes(Node.Expressions);
     Result := TAst.Block(newExprs);
 end;
 
 function TExpansionVisitor.VisitRecordLiteral(const Node: IRecordLiteralNode): IAstNode;
 begin
+    // Standard recursion for records
     Result := inherited VisitRecordLiteral(Node);
 end;
 
@@ -314,12 +335,13 @@ end;
 constructor TMacroExpander.Create(
     const ARootRegistry: IMacroRegistry;
     const AInitialScope: IExecutionScope;
-    const AEvaluatorFactory: TEvaluatorFactory
+    const AMacroEvaluator: TMacroEvaluatorProc
 );
 begin
     inherited Create;
     FInitialScope := AInitialScope;
-    FEvaluatorFactory := AEvaluatorFactory;
+    FMacroEvaluator := AMacroEvaluator;
+    // Create a local child registry to handle scoped macro definitions (defmacro inside do)
     FCurrentMacroRegistry := ARootRegistry.CreateChildRegistry;
 end;
 
@@ -342,10 +364,10 @@ class function TMacroExpander.ExpandMacros(
     const ARootRegistry: IMacroRegistry;
     const AInitialScope: IExecutionScope;
     const RootNode: IAstNode;
-    const EvaluatorFactory: TEvaluatorFactory
+    const MacroEvaluator: TMacroEvaluatorProc
 ): IAstNode;
 begin
-    var expander := TMacroExpander.Create(ARootRegistry, AInitialScope, EvaluatorFactory) as IAstMacroExpander;
+    var expander := TMacroExpander.Create(ARootRegistry, AInitialScope, MacroEvaluator) as IAstMacroExpander;
     Result := expander.Execute(RootNode);
 end;
 
@@ -358,6 +380,7 @@ end;
 
 function TMacroExpander.VisitBlockExpression(const Node: IBlockExpressionNode): IAstNode;
 begin
+    // Macros defined inside a block should only be visible within that block
     EnterMacroScope;
     try
         Result := inherited VisitBlockExpression(Node);
@@ -378,7 +401,11 @@ end;
 
 function TMacroExpander.VisitMacroDefinition(const Node: IMacroDefinitionNode): IAstNode;
 begin
+    // Register the macro in the current scope
     FCurrentMacroRegistry.Define(Node);
+    // Remove the definition from the runtime AST (it's compile-time only)
+    // However, for now we return the node so it can be serialized/inspected,
+    // but the Evaluator usually ignores it (returns Void).
     Result := Node;
 end;
 
@@ -388,6 +415,7 @@ var
     macroDef: IMacroDefinitionNode;
     i: Integer;
 begin
+    // Check if it is a macro call
     if Node.Callee.Kind <> akIdentifier then
         exit(inherited VisitFunctionCall(Node));
 
@@ -396,62 +424,49 @@ begin
     if macroDef = nil then
         exit(inherited VisitFunctionCall(Node));
 
-    // --- Macro Expansion ---
+    // --- Macro Expansion Logic ---
 
     // 1. Create temporary scope for macro arguments (parented to Initial/Global Scope)
+    // This ensures macro execution is hygienic regarding the environment it runs in.
     var expansionScope := TScope.CreateScope(FInitialScope, nil, nil);
 
     var params := macroDef.Parameters;
     if Length(Node.Arguments) <> Length(params) then
         raise Exception.CreateFmt('Macro %s expects %d arguments.', [calleeIdentifier.Name, Length(params)]);
 
-    // 2. Populate scope
+    // 2. Populate scope with UN-EVALUATED AST nodes (Code as Data)
     for i := 0 to High(params) do
         expansionScope.Define(params[i].Name, TDataValue.FromIntf<IAstNode>(Node.Arguments[i]));
 
-    // 3. Create Evaluator Callback
-    var evaluatorProc: TEvaluateProc;
-    evaluatorProc :=
+    // 3. Create simple callback for TExpansionVisitor
+    var evaluatorProc: TExpansionVisitor.TEvaluateProc :=
         function(const ANodeToEvaluate: IAstNode): TDataValue
-        var
-            tmpLayout: IScopeLayout;
-            scratchScope: IExecutionScope;
-            evaluator: IEvaluatorVisitor;
-            boundSubAst: IAstNode;
         begin
-            // A. BINDING
-            // The Binder creates a virtual child scope builder.
-            // Symbols in expansionScope will have ScopeDepth = 1.
-            // We access the layout via the descriptor (which for dynamic scopes is created on demand/proxy).
-            // Note: expansionScope is dynamic, so its Descriptor.Layout reflects current variables.
-            boundSubAst := TAstBinder.Bind(expansionScope.Descriptor.Layout, ANodeToEvaluate, tmpLayout);
-
-            // B. SCOPE MATCHING (The Fix)
-            // We must execute in a child scope so that ScopeDepth = 1 correctly points to expansionScope.
-            scratchScope := TScope.CreateScope(expansionScope, nil, nil);
-
-            // C. EXECUTION
-            evaluator := FEvaluatorFactory(scratchScope);
-            Result := evaluator.Execute(boundSubAst);
+            // Delegate complex binding/execution logic to the environment via FMacroEvaluator
+            Result := FMacroEvaluator(expansionScope, ANodeToEvaluate);
         end;
 
-    // 4. Expand Body
+    // 4. Expand the Macro Body
+    // We assume the macro body is a Quasiquote. We expand it using the visitor.
     var expandedBody := TExpansionVisitor.Expand(expansionScope, macroDef.Body.AsQuasiquote.Expression, evaluatorProc);
 
-    // 5. Wrap result
+    // 5. Create a MacroExpansionNode to preserve source mapping
     var macroNode := TAst.MacroExpansionNode(Node, expandedBody);
 
-    // 6. Re-expand result
+    // 6. Recursively expand the result (the output of a macro might contain more macro calls)
     Result := Self.Accept(macroNode);
 end;
 
 function TMacroExpander.VisitQuasiquote(const Node: IQuasiquoteNode): IAstNode;
 begin
+    // Quasiquotes in user code are not expanded by the MacroExpander,
+    // they are literals.
     Result := Node;
 end;
 
 function TMacroExpander.VisitUnquote(const Node: IUnquoteNode): IAstNode;
 begin
+    // Unquotes outside of a macro definition/expansion phase are invalid syntax
     raise Exception.Create('Unquote (`~`) can only be used inside a quasiquote.');
 end;
 

Src/AST/Myc.Ast.Environment.pas

@@ -560,12 +560,36 @@ end;
 function TEnvironment.ExpandMacros(const Node: IAstNode): IAstNode;
 begin
     var cExecutionStrategy := FExecutionStrategy;
+
+    // The Glue: This callback performs the full compilation pipeline for a macro argument expression.
+    // It binds the node to the layout of the temporary macro scope and then executes it.
+    var macroEvaluator: TMacroEvaluatorProc :=
+        function(const Scope: IExecutionScope; const ANode: IAstNode): TDataValue
+        var
+            tmpLayout: IScopeLayout;
+            evaluator: IEvaluatorVisitor;
+            boundSubAst: IAstNode;
+            scratchScope: IExecutionScope;
+        begin
+            // 1. Binding
+            // Note: Scope is dynamic (from TMacroExpander), so Descriptor.Layout describes current variables.
+            boundSubAst := TAstBinder.Bind(Scope.Descriptor.Layout, ANode, tmpLayout);
+
+            // 2. Scope Matching
+            // Create a child scope to ensure correct depth resolution (Binder sees Layout at depth 0 relative to itself)
+            scratchScope := TScope.CreateScope(Scope, nil, nil);
+
+            // 3. Execution
+            evaluator := cExecutionStrategy.CreateVisitor(scratchScope);
+            Result := evaluator.Execute(boundSubAst);
+        end;
+
     Result :=
         TMacroExpander.ExpandMacros(
             FMacroRegistry,
             FRootScope,
             Node,
-            function(const Scope: IExecutionScope): IEvaluatorVisitor begin Result := cExecutionStrategy.CreateVisitor(Scope); end
+            macroEvaluator // Injected single dependency
         );
 end;