Skip to content

Commit 4413824

Browse files
committed
2 parents 98bf699 + 82b5f10 commit 4413824

7 files changed

Lines changed: 268 additions & 61 deletions

File tree

CHANGELOG.md

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
## Upcoming changes
2+
3+
### Added
4+
5+
- **ReplaceOptions.direction** — replacement traversal can now be controlled
6+
from left-to-right or right-to-left.
7+
8+
### Fixed
9+
10+
- **ReplaceOptions form handling**`Expression.replace()` now accepts the new
11+
`form` option, while keeping deprecated `canonical` as a backward- compatible
12+
alias for one release.
13+
- **Replacement form propagation** — recursive replacements preserve and
14+
propagate the requested form upward when child operands already share it.
15+
16+
### Changes
17+
18+
- **ReplaceOptions.canonical is deprecated** — prefer `form`; specifying both
19+
`form` and `canonical` now raises an error.
120
### [Unreleased]
221

322
- **Numeric correctness fixes** — several arithmetic operations produced
@@ -1315,8 +1334,8 @@ GPU compile).
13151334
`oklab(L a b / alpha)` syntax, matching the existing `oklch()` support.
13161335
- **GPU compilation**: `ColorMix`, `ColorContrast`, `ContrastingColor`,
13171336
`ColorToColorspace`, and `ColorFromColorspace` now compile to GLSL and WGSL.
1318-
Preamble functions provide sRGB ↔ OKLab ↔ OKLCh conversion, color mixing with
1319-
shorter-arc hue interpolation, and APCA contrast on the GPU.
1337+
Preamble functions provide sRGB ↔ OKLab ↔ OKLCh conversion, color mixing
1338+
with shorter-arc hue interpolation, and APCA contrast on the GPU.
13201339
- Added `rgbToHsl()` conversion function. Exported `hslToRgb()` (previously
13211340
private).
13221341

src/compute-engine/boxed-expression/rules.ts

Lines changed: 183 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {
1515
Expression,
1616
ReplaceOptions,
1717
ExpressionInput,
18+
FormOption,
1819
} from '../global-types';
1920

2021
import {
@@ -657,16 +658,9 @@ function boxRule(
657658
);
658659
}
659660

660-
// Push a clean scope that only inherits from the system scope (index 0),
661-
// not from the global scope or user-defined scopes. This prevents user-defined
662-
// symbols (like `x` used as a function name in `x(y+z)`) from interfering with
663-
// rule parsing. The system scope contains all built-in definitions.
664-
const systemScope = ce.contextStack[0]?.lexicalScope;
665-
if (systemScope) {
666-
ce.pushScope({ parent: systemScope, bindings: new Map() });
667-
} else {
668-
ce.pushScope();
669-
}
661+
// Ensure a clean scope (that only inherits from the system scope) before boxing or parsing:
662+
// preventing wildcards & user-defined from inheriting definitions in rules.
663+
pushSafeScope(ce);
670664

671665
let matchExpr: Expression | undefined;
672666
let replaceExpr: Expression | RuleReplaceFunction | RuleFunction | undefined;
@@ -742,6 +736,25 @@ function boxRule(
742736
};
743737
}
744738

739+
/**
740+
* Push a clean scope - safe for the boxing of rules - that only inherits from the system scope
741+
* (index 0), not from the global scope or user-defined scopes. This prevents user-defined symbols
742+
* (like `x` used as a function name in `x(y+z)`) from interfering with rule parsing. The system
743+
* scope contains all built-in definitions.
744+
*
745+
* This also crucially prevents wildcards from being given definitions where captured & bound.
746+
*
747+
* @param ce
748+
*/
749+
function pushSafeScope(ce: ComputeEngine) {
750+
const systemScope = ce.contextStack[0]?.lexicalScope;
751+
if (systemScope) {
752+
ce.pushScope({ parent: systemScope, bindings: new Map() });
753+
} else {
754+
ce.pushScope();
755+
}
756+
}
757+
745758
/**
746759
* Create a boxed rule set from a collection of non-boxed rules
747760
*/
@@ -774,6 +787,23 @@ export function boxRules(
774787
return { rules };
775788
}
776789

790+
function normalizeReplaceForm(
791+
options?: Readonly<Partial<ReplaceOptions>>
792+
): FormOption | undefined {
793+
if (options?.canonical !== undefined && options?.form !== undefined)
794+
throw new Error(
795+
'replace(): options.canonical and options.form are mutually exclusive'
796+
);
797+
798+
if (options?.canonical !== undefined) {
799+
if (options.canonical === true) return 'canonical';
800+
if (options.canonical === false) return 'raw';
801+
return options.canonical;
802+
}
803+
804+
return options?.form;
805+
}
806+
777807
/**
778808
* Apply a rule to an expression, assuming an incoming substitution
779809
* @param rule the rule to apply
@@ -789,15 +819,20 @@ export function applyRule(
789819
options?: Readonly<Partial<ReplaceOptions>>
790820
): RuleStep | null {
791821
if (!rule) return null;
792-
let canonical = options?.canonical ?? (expr.isCanonical || expr.isStructural);
822+
const requestedForm = normalizeReplaceForm(options);
793823

794824
// eslint-disable-next-line prefer-const
795825
let { match, replace, condition, id, onMatch, onBeforeMatch } = rule;
796826
const because = id ?? '';
797827

798828
const ce = expr.engine;
799829

800-
if (canonical && match) {
830+
const canonicalRequested =
831+
requestedForm !== undefined &&
832+
requestedForm !== 'raw' &&
833+
requestedForm !== 'structural';
834+
835+
if ((canonicalRequested || expr.isCanonical) && match) {
801836
const awc = getWildcards(match);
802837
const canonicalMatch = match.canonical;
803838
const bwc = getWildcards(canonicalMatch);
@@ -809,29 +844,39 @@ export function applyRule(
809844
let operandsMatched = false;
810845

811846
if (isFunction(expr) && options?.recursive) {
847+
const direction = options?.direction ?? 'left-right';
848+
let newOps =
849+
direction === 'left-right' ? expr.ops : [...expr.ops].reverse();
850+
812851
// Apply the rule to the operands of the expression
813-
const newOps = expr.ops.map((op) => {
852+
newOps = newOps.map((op) => {
814853
const subExpr = applyRule(rule, op, {}, options);
815854
if (!subExpr) return op;
816855
operandsMatched = true;
817856
return subExpr.value;
818857
});
819858

859+
if (direction === 'right-left') (newOps as Expression[]).reverse();
860+
820861
// At least one operand (directly or recursively) matched: but continue onwards to match against
821862
// the top-level expr., test against any 'condition', et cetera.
822863
if (operandsMatched) {
823-
// If new/replaced operands are all canonical, and options do not explicitly specify canonical
824-
// status, then should be safe to mark as fully-canonical
825-
if (
826-
!canonical &&
827-
options?.canonical === undefined &&
828-
newOps.every((x) => x.isCanonical)
829-
)
830-
canonical = true;
831-
832-
expr = ce.function(expr.operator, newOps, {
833-
form: canonical ? 'canonical' : 'raw',
834-
});
864+
// (note: so not consult the input-expr 'form' because, assuming that replaced operands assume
865+
// the same form, this will be upcast in the subsequent branches.
866+
// ^Another reason to avoid this, is if the form of replacements differ from the input expr.,
867+
// then likely it is not the intention to preserve the form of the parent)
868+
let form: FormOption = 'raw';
869+
// The current policy for applying a form according to 'options.form' is for this to apply to
870+
// *replacements only* (this ultimately allowing for finer control of replacement operations).
871+
// ...However, if all child operands bear the same form, 'eagerly' assume this form for the
872+
// present expression (if this present expression also later matches, form may be updated
873+
// according to 'options.form'.)
874+
//(@note: check 'canonical' first, because numbers may be jointly marked as structural and
875+
//canonical).
876+
if (newOps.every((x) => x.isCanonical)) form = 'canonical';
877+
else if (newOps.every((x) => x.isStructural)) form = 'structural';
878+
879+
expr = ce.function(expr.operator, newOps, { form });
835880
}
836881
}
837882

@@ -881,41 +926,81 @@ export function applyRule(
881926
}
882927
}
883928

929+
/** The computed form value to be assumed by the *directly replaced* expression: assuming either an
930+
'enforced' value (options), or consultation to the form of the input expression */
931+
let formValue =
932+
requestedForm ??
933+
(expr.isStructural ? 'structural' : expr.isCanonical ? 'canonical' : 'raw');
934+
935+
// If `true`, then the form is not 'enforced' (via options) and therefore, the prior computed
936+
// form only applies wherein the initially-produced replacement expression has a 'raw' form
937+
// (else retaining whichever form of the replacement)
938+
const dynamicForm = requestedForm === undefined;
939+
940+
/** Get the overall form type from *formValue* (raw/structural/canonical), accounting for
941+
* 'canonical' potentially assuming multiple values. */
942+
const getFormType = () =>
943+
formValue === 'structural'
944+
? 'structural'
945+
: formValue === 'raw'
946+
? 'raw'
947+
: 'canonical';
948+
884949
// Have a (direct) match: in this case, consider the canonical-status of the replacement, too.
885950
if (
886-
!canonical &&
887-
options?.canonical === undefined &&
951+
formValue === 'raw' &&
952+
dynamicForm &&
888953
replace instanceof _BoxedExpression &&
889-
replace.isCanonical
954+
(replace.isCanonical || replace.isStructural)
890955
)
891-
canonical = true;
956+
formValue = replace.isCanonical ? 'canonical' : 'structural';
892957

893958
//@note: '.subs()' acts like an expr. 'clone' here (in case of an empty substitution)
894959
const result =
895960
typeof replace === 'function'
896961
? replace(expr, sub)
897-
: replace.subs(sub, { canonical });
962+
: // @todo: 'expr.subs()' to eventually also assume a 'form' option
963+
// : replace.subs(sub, { form: dynamicForm ? undefined : formValue });
964+
replace.subs(sub, { canonical: getFormType() === 'canonical' });
898965

899-
if (!result)
900-
return operandsMatched
901-
? { value: canonical ? expr.canonical : expr, because }
902-
: null;
966+
if (!result) return operandsMatched ? { value: expr, because } : null;
903967

904968
// To aid in debugging, invoke onMatch when the rule matches
905969
onMatch?.(rule, expr, result);
906970

971+
/** Return the final *expression* with the correctly computed form. */
972+
const computeValue = (result: Expression) => {
973+
// If 'raw', leave the expression as-is
974+
// (note that if result has produced a 'non-raw' form, this may not be 'undone'...)
975+
if (formValue === 'raw') return result;
976+
// Non option-enforced form; let replacement/result expression form override
977+
if (dynamicForm === true && (result.isStructural || result.isCanonical))
978+
return result;
979+
// Enforced form
980+
return getFormType() === 'canonical'
981+
? result.isCanonical
982+
? result
983+
: ce.expr(result, { form: formValue }) //Re-box (instead of 'x.canonical'), case of 'CanonicalForm'
984+
: result.structural;
985+
};
986+
987+
// (Need to request a 'form' variant (canonical/structural) to account for case of a custom
988+
// replace: which may not have returned the same 'form' calculated here)
907989
if (isRuleStep(result))
908-
return canonical ? { ...result, value: result.value.canonical } : result;
990+
return getFormType() === 'raw'
991+
? result
992+
: { ...result, value: computeValue(result.value) };
909993

910994
if (!isExpression(result)) {
911995
throw new Error(
912996
'Invalid rule replacement result: expected a Expression or RuleStep'
913997
);
914998
}
915999

916-
// (Need to request the canonical variant to account for case of a custom replace: which may not
917-
// have returned canonical.)
918-
return { value: canonical ? result.canonical : result, because };
1000+
return {
1001+
value: computeValue(result),
1002+
because,
1003+
};
9191004
}
9201005

9211006
/**
@@ -936,6 +1021,7 @@ export function replace(
9361021
const iterationLimit = options?.iterationLimit ?? 1;
9371022
let iterationCount = 0;
9381023
const once = options?.once ?? false;
1024+
normalizeReplaceForm(options);
9391025

9401026
// Normalize the ruleset
9411027
let ruleSet: ReadonlyArray<BoxedRule>;
@@ -956,7 +1042,7 @@ export function replace(
9561042
if (
9571043
result !== null &&
9581044
result.value !== expr &&
959-
!result.value.isSame(expr)
1045+
(!result.value.isSame(expr) || varyingForm(expr, result.value))
9601046
) {
9611047
// If `once` flag is set, bail on first matching rule
9621048
if (once) return [result];
@@ -977,6 +1063,64 @@ export function replace(
9771063
iterationCount += 1;
9781064
}
9791065
return steps;
1066+
1067+
/*
1068+
* Local f.
1069+
*/
1070+
/**
1071+
* Assuming *x* and *x2* are **structurally (symbolically) equivalent**, and considering
1072+
* expression forms 'structural' and 'canonical':
1073+
*
1074+
* - If option 'recursive' equals `true` or `'functions-only'` (**default** = `'functions-only'`),
1075+
* then, if either 'x' or 'x2', or one of the matching sub-expression pairs of these has a
1076+
* differing 'structural' or 'canonical' status, then return `true`.
1077+
* (if 'functions-only', then only function-expression operands are considered)
1078+
*
1079+
* - If 'recursive' === `false`, then this status comparison applies only to/between `x` and `x2`
1080+
* directly.
1081+
*
1082+
* For both cases, if neither `x` nor `x2` (nor compared sub-expressions if recursive) is
1083+
* structural or canonical, then return `false`.
1084+
*
1085+
* **Warning**: will throw an error if it is determined, in case of `recursive !== false`, that
1086+
* `x` and `x2` are not structurally equivalent/have an identical tree/branching structure.
1087+
* (It is therefore the responsibility of the caller to ensure this beforehand)
1088+
*/
1089+
function varyingForm(
1090+
x: Expression,
1091+
x2: Expression,
1092+
{
1093+
recursive = 'functions-only',
1094+
}: { recursive?: boolean | 'functions-only' } = {}
1095+
): boolean {
1096+
if (varies(x, x2)) return true;
1097+
1098+
if (recursive === false) return false;
1099+
1100+
if (isFunction(x) && isFunction(x2)) {
1101+
if (x.ops.length !== x2.ops.length)
1102+
throw new Error(
1103+
`'x' and 'x2' detected to not be structurally equivalent`
1104+
);
1105+
if (x.nops === 0) return false;
1106+
1107+
return x.ops.some((op, index) =>
1108+
recursive === true || (!isFunction(op) && !isFunction(x2.ops[index]))
1109+
? false
1110+
: varyingForm(op, x2.ops[index], { recursive })
1111+
);
1112+
} else if (isFunction(x) || isFunction(x2)) return true;
1113+
1114+
return false;
1115+
1116+
function varies(x: Expression, x2: Expression): boolean {
1117+
if (x.isStructural || x.isCanonical) {
1118+
if (x.isStructural) return !x2.isStructural;
1119+
return !x2.isCanonical;
1120+
}
1121+
return x2.isStructural || x2.isCanonical ? true : false;
1122+
}
1123+
}
9801124
}
9811125

9821126
/**

src/compute-engine/boxed-expression/simplify.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ function simplifyExpression(
341341
if (isSymbol(expr)) {
342342
const result = replace(expr, rules, {
343343
recursive: false,
344-
canonical: true,
344+
form: 'canonical',
345345
useVariations: false,
346346
});
347347
if (result.length > 0) return [...steps, ...result];
@@ -394,7 +394,7 @@ function simplifyNonCommutativeFunction(
394394
): RuleSteps {
395395
const result = replace(expr, rules, {
396396
recursive: false,
397-
canonical: true,
397+
form: 'canonical',
398398
useVariations: options.useVariations ?? false,
399399
});
400400

0 commit comments

Comments
 (0)