From 63ac30ce882cce41a1a1a2185e9b399186c55fb7 Mon Sep 17 00:00:00 2001
From: Bill Hails <billhails2014@gmail.com>
Date: Sun, 27 Oct 2024 12:22:26 +0000
Subject: [PATCH] macros as lazy functions in a happy place now

---
 docs/OPERATORS.md        | 45 ++++++++++++++++++++++++++++++++++++++++
 src/macro_substitution.c | 23 ++++++++++++--------
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/docs/OPERATORS.md b/docs/OPERATORS.md
index 4572148..c748fbb 100644
--- a/docs/OPERATORS.md
+++ b/docs/OPERATORS.md
@@ -67,3 +67,48 @@ local scoping rules, since `AND` is now a normal function then free variables in
 body are evaluated in the context of the function definition, and variables in the
 argument expressions are evaluated in the calling context.
 
+Got that working, and we're also handling local shadowing of arguments so they don't
+get wrapped in an invocation unless they are the same lexical variable.
+
+One little unnecssary inefficiency needs to be addressed. If one macro calls another,
+for example
+
+```
+macro NAND(a, b) { NOT(AND(a, b)) }
+```
+
+This first gets rewritten, by `lambda_conversion.c` to
+
+```
+fn NAND(a, b) { NOT(AND(fn () {a}, fn () {b})) }
+```
+
+and then subsequently by `macro_substitution.c` to
+
+```
+fn NAND(a, b) { NOT(AND(fn () {a()}, fn () {b()})) }
+```
+
+While correct, the expression `fn () {a()}` is just `a` so we'll need
+a pass to optimise away this unnecessary wrapping and unwrapping,
+essentially restoring
+
+```
+fn NAND(a, b) { NOT(AND(a, b)) }
+```
+
+Two approaches:
+
+1. Macro specific, have a special type for macro argument application
+   and another for macro argument wrapping, and detect the explicit
+   combination of the two.
+2. Generic pass that would detect this wherever it occurs and optimize it.
+
+In either case we need to be a little bit careful that we allow the
+pattern if the argument is being modified, for example if a macro
+called another with it's argument modified in some way then the pattern
+i.e. `fn() { a() + 1 }` would be necessary.
+
+Got option 1 working, but no need for extra types, just inspect the
+thunk during macro conversion, if it has no arguments and just contains
+a symbol that would otherwise be invoked then return the symbol.
diff --git a/src/macro_substitution.c b/src/macro_substitution.c
index c5e8663..f2f92e2 100644
--- a/src/macro_substitution.c
+++ b/src/macro_substitution.c
@@ -30,15 +30,21 @@
 #  include "debugging_off.h"
 #endif
 
-static LamLam *performLamSubstitutions(LamLam *lam, LamMacroArgsTable *symbols) {
+static bool isReplacementSymbol(HashSymbol *var, LamMacroArgsTable *symbols) {
+    return getLamMacroArgsTable(symbols, var, NULL);
+}
+
+static LamExp *performLamSubstitutions(LamLam *lam, LamMacroArgsTable *symbols) {
     ENTER(performLamSubstitutions);
+    // fn () { a() } == a
+    if (   lam->args == NULL
+        && lam->exp->type == LAMEXP_TYPE_VAR
+        && isReplacementSymbol(lam->exp->val.var, symbols)) {
+        return lam->exp;
+    }
     lam->exp = lamPerformMacroSubstitutions(lam->exp, symbols);
     LEAVE(performLamSubstitutions);
-    return lam;
-}
-
-static bool isReplacementSymbol(HashSymbol *var, LamMacroArgsTable *symbols) {
-    return getLamMacroArgsTable(symbols, var, NULL);
+    return newLamExp_Lam(CPI(lam), lam);
 }
 
 static bool containsReplacementSymbols(LamVarList *vars, LamMacroArgsTable *symbols) {
@@ -374,8 +380,7 @@ LamExp *lamPerformMacroSubstitutions(LamExp *exp, LamMacroArgsTable *symbols) {
             case LAMEXP_TYPE_CONSTRUCTOR:
                 break;
             case LAMEXP_TYPE_LAM:
-                exp->val.lam =
-                    performLamSubstitutions(exp->val.lam, symbols);
+                exp = performLamSubstitutions(exp->val.lam, symbols);
                 break;
             case LAMEXP_TYPE_VAR: {
                 LamExp *rep = performVarSubstitutions(CPI(exp), exp->val.var, symbols);
@@ -445,7 +450,7 @@ LamExp *lamPerformMacroSubstitutions(LamExp *exp, LamMacroArgsTable *symbols) {
                 exp->val.lookup = performLookupSubstitutions(exp->val.lookup, symbols);
                 break;
             default:
-                cant_happen("unrecognized LamExp type %s", lamExpTypeName(exp->type));
+                cant_happen("unrecognized %s", lamExpTypeName(exp->type));
         }
     }
     LEAVE(lamPerformMacroSubstitutions);