From f82dab70d36e7749bde3d871a39ddf0b1a2cefa9 Mon Sep 17 00:00:00 2001
From: Josh Davies <jodavies@users.noreply.github.com>
Date: Fri, 9 Oct 2026 14:36:47 +0100
Subject: [PATCH] fix: regression: dollar var arguments and TestSub

The is a regression due to 13f03ab. Since we have removed intermediate calls of
Normalize, SUBEXPRESSIONs can survive in the terms later, when we sort the
argument, for example when dollar variables are replaced. Allow such terms,
and move the dirty check after this normalize and sort.
---
 check/fixes.frm  | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 sources/proces.c | 40 ++++++++++++++++++++--------------------
 2 files changed, 67 insertions(+), 20 deletions(-)

diff --git a/check/fixes.frm b/check/fixes.frm
index 94c7198c..232ba4e2 100644
--- a/check/fixes.frm
+++ b/check/fixes.frm
@@ -5245,6 +5245,53 @@ Print;
 assert succeeded?
 assert result("test") =~ expr("4+4*x")
 *--#] Issue882_6 :
+*--#[ Issue916_1 :
+Vector l1;
+CFunction den;
+
+#$k = -l1;
+
+Local expr = 1;
+Local match = den(l1,1);
+Identify den(-$k,1) = 1;
+
+Print;
+.end
+assert succeeded?
+assert result("expr") =~ expr("1")
+assert result("match") =~ expr("1")
+*--#] Issue916_1 :
+*--#[ Issue916_2 :
+Symbol x,y,z;
+CFunction den,g;
+
+#$s = -x-y;
+
+Local rhs1 = den(-$s);
+Local rhs2 = den(g(-$s),z);
+Local cancel = den(-$s-x-y);
+Local match1 = den(x+y+z);
+Local match2 = den(g(x+y+z),z);
+Local match3 = den(x^2+x*y+y);
+Local match4 = den(x+y+z+g(x+y));
+.sort
+
+Identify den(-$s+z) = 1;
+Identify den(g(-$s+z),z) = 2;
+Identify den(-$s*x+y) = 3;
+Identify den(-$s+z+g(-$s)) = 4;
+
+Print;
+.end
+assert succeeded?
+assert result("rhs1") =~ expr("den(y+x)")
+assert result("rhs2") =~ expr("den(g(y+x),z)")
+assert result("cancel") =~ expr("den(0)")
+assert result("match1") =~ expr("1")
+assert result("match2") =~ expr("2")
+assert result("match3") =~ expr("3")
+assert result("match4") =~ expr("4")
+*--#] Issue916_2 :
 *--#[ PullReq535 :
 * This test requires more than the specified 50K workspace.
 #:maxtermsize 200
diff --git a/sources/proces.c b/sources/proces.c
index b0fc8f74..cf1106e7 100644
--- a/sources/proces.c
+++ b/sources/proces.c
@@ -1687,30 +1687,15 @@ doesnotwork:;
 								AN.ncmod = oldncmod;
 								DONE(retvalue)
 							}
-							else {
-								// For a long time, Normalize was called in-place here. This was
-								// very dangerous, since Normalize can increase the size of the
-								// term, and overwrite term data at outer nesting levels. We
-								// can instead just rely on the Normalize call (which uses the
-								// WorkSpace) just before calling StoreTerm later.
-								WORD *tend = t + *t, *tt = t+1;
-								stilldirty = 0;
-								tend -= ABS(tend[-1]);
-								while ( tt < tend ) {
-									if ( *tt == SUBEXPRESSION || *tt == EXPRESSION ) {
-										stilldirty = 1; break;
-									}
-									tt += tt[1];
-								}
-							}
 							AN.subsubveto = 0;
 							AT.RecFlag--;
 							t += *t;
 						}
 						AT.NestPoin--;
 /*
-						Argument contains no subexpressions.
-						It should be normalized and sorted.
+						Normalize and sort the terms of the argument. Normalize may possibly
+						introduce new SUBEXPRESSIONs, for example due to dollar-var inserts.
+						Check after sorting if the argument is clean, or needs further work.
 						The main problem is the storage: we need to Normalize a copy of the
 						term data in the WorkSpace, since Normalize can increase the size
 						of the term, which would overwrite the data which follows!
@@ -1732,7 +1717,8 @@ doesnotwork:;
 							do { *m++ = *t++; } while ( t < r );
 							r = AT.WorkPointer;
 							AT.WorkPointer = r + *r;
-							if ( Normalize(BHEAD r) ) {
+							/* Allow positive return values: there may still be SUBEXPRESSIONs to process */
+							if ( Normalize(BHEAD r) < 0 ) {
 								if ( *t1 == AR.PolyFun && AR.PolyFunType == 2 ) {
 									AR.SortType = oldsorttype;
 									AR.CompareRoutine = (COMPAREDUMMY)oldcompareroutine;
@@ -1767,7 +1753,21 @@ doesnotwork:;
 							AR.CompareRoutine = (COMPAREDUMMY)oldcompareroutine;
 							t1[2] |= MUSTCLEANPRF;
 						}
-						while ( *m ) m += *m;
+						/* Check the normalized, sorted result for remaining processing */
+						stilldirty = 0;
+						while ( *m ) {
+							if ( !stilldirty ) {
+								WORD *tend = m + *m, *tt = m+1;
+								tend -= ABS(tend[-1]);
+								while ( tt < tend ) {
+									if ( *tt == SUBEXPRESSION || *tt == EXPRESSION ) {
+										stilldirty = 1; break;
+									}
+									tt += tt[1];
+								}
+							}
+							m += *m;
+						}
 						i = WORDDIF(m,AT.WorkPointer);
 						*AT.WorkPointer = i;
 						AT.WorkPointer[1] = stilldirty;