Skip to content

Read the arms of a rule switch that lives inside a factory (#825) - #968

Merged
Rafael-SOWNet merged 1 commit into
masterfrom
feat/addressable-rule-factories
Aug 16, 2026
Merged

Rafael-SOWNet merged 1 commit into
masterfrom
feat/addressable-rule-factories

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Member

Fourteen of the thirty rewrite rule sets had no addressable rules. Three of those fourteen had arms all along — the canonical-order sets, whose rules are one switch over the node type, seven arms, each dispatching to SortAndGroup.

What stopped the generator reading them was the shape around the switch, not anything about the rules:

static Entity CommonRules(Entity x)                 => x switch { ... }      // read
static Func<Entity, Entity> SortRules(SortLevel l)  => x => x switch { ... } // not read

A set parameterised by something that is not the expression is written as a factory. So there is a lambda in the way, and the arms close over the factory's parameter. That is the whole change:

  • the switch is found one level down, through the lambda, and the governing identifier is checked against the lambda's parameter rather than the method's;
  • the arms cannot be a static readonly field, because they capture the parameter — they are emitted as a method taking the factory's own parameter list, and the apply lambda loses its static.

A lambda parameter carries no type and the arms are copied into a context that infers nothing, so the parameter type is named rather than echoed.

before after
addressable sets 16 19
addressable rules 347 368
armless sets 14 11

What this does not do

SortRules is still the method the simplifier calls and still the thing a human edits. The arms are still copied as syntax, so a rule still cannot say something its arm does not. ApplyOnce calls the same Func it called before — this adds reporting granularity and changes no answer.

It also does not chase the remaining eleven, and they are armless for a reason rather than for want of a generator: a polynomial long division, a GCD cancellation, methods with branches and locals, and three single is patterns that are one rule each and have nothing to split. RewriteRuleSet.Rules offered "a sort" as an example of the first kind — that was the wrong example, and the doc now says so.

The new tests bind

The three sets join the replay test, each against the switch built at its own level. That per-level pairing is not decoration — it is the mistake this shape newly makes possible, so I tried to make it:

Patterns.SortRulesArms(SortLevel.HIGH_LEVEL)   // while the switch stays at LOW_LEVEL

  Failed AddressableRulesTest.EveryArmIsTheArmItWasGeneratedFrom(setName: "CanonicalOrderExact")
  Failed: 1, Passed: 25

Reverted, of course; 26/26 pass on the branch.

ASetWithNoAddressableRulesStillRecordsItsStep used CanonicalOrderExact as its armless example, which it no longer is. It now uses InvertNegativePowers and asserts up front that the set really has no rules — otherwise the test would keep passing while testing nothing the day that set becomes addressable too.

Measured

  • Suite 7301 passed, 0 failed, 14 skipped.
  • canoncheck on this branch is identical to master, line for line: InnerSimplified idempotence 0/834 and order 2024/2738; CanonicalOrderExact 18/834 and 0/2738; Normalise+Order 0 and 0; Simplify 0/120 and 8/72. I measured master myself rather than reading the checked-in report, which says 21 for the second of those — it is stale against master independently of this PR.

No BREAKING-CHANGES.md entry: RewriteRuleSet.Rules does not exist in 2.2.0, so there is nothing here to break against.

…item 50)

Fourteen of the thirty rewrite rule sets had no addressable rules. Three of
those fourteen had arms all along -- the three canonical-order sets, whose
rules are one switch over the node type, seven arms, dispatching to
SortAndGroup. What stopped the generator reading it was the shape around the
switch rather than anything about the rules:

    static Entity  CommonRules(Entity x)              => x switch { ... }   // read
    static Func<Entity, Entity> SortRules(SortLevel l) => x => x switch {...} // not

A set parameterised by something that is not the expression is written as a
factory, so there is a lambda in the way and the arms close over the
factory's parameter. Two consequences, and they are the whole change:

- the switch is found one level down, through the lambda, and the governing
  identifier checked against the lambda's parameter rather than the method's;
- the arms cannot be a static field, because they capture the parameter. They
  are emitted as a method with the factory's own parameter list, and the
  apply lambda loses its `static`.

A lambda parameter carries no type, and the arms are copied into a context
that infers nothing, so the parameter type is named rather than echoed.

Nothing else moves. SortRules is still the method the simplifier calls, still
the thing a human edits, and the arms are still copied as syntax -- so a rule
still cannot say something its arm does not.

  addressable sets   16 -> 19
  addressable rules  347 -> 368   (seven arms, read at three sort levels)
  armless sets       14 -> 11

What is left armless is armless for a reason rather than for want of a
generator: a polynomial long division, a GCD cancellation, methods with
branches and locals, and three single `is` patterns that are one rule each
and have nothing to split. RewriteRuleSet.Rules said "a sort" was an example
of the first kind; it was the wrong example and now says so.

The three sets are added to the replay test, each against the switch built at
its own level. That test binds: pointing CanonicalOrderExact's arms at
HIGH_LEVEL while its switch stays at LOW_LEVEL fails it by name, which is the
mistake this shape makes possible and the reason the arms are replayed per
level rather than once.

ASetWithNoAddressableRulesStillRecordsItsStep used CanonicalOrderExact as its
armless example and now uses InvertNegativePowers, with an assertion that the
set really has no rules -- otherwise the test would keep passing while
testing nothing.

Measured: suite 7301 passed, 0 failed, 14 skipped. canoncheck is identical to
master line for line -- InnerSimplified idempotence 0/834 and order
2024/2738, CanonicalOrderExact 18/834 and 0/2738, Normalise+Order 0 and 0,
Simplify 0/120 and 8/72 -- which is the point, since ApplyOnce calls the same
Func it did before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant