Skip to content

Read a rule switch that takes a second parameter (#825) - #971

Merged
Rafael-SOWNet merged 1 commit into
feat/addressable-single-rulesfrom
feat/addressable-parameterised-switch
Aug 16, 2026
Merged

Rafael-SOWNet merged 1 commit into
feat/addressable-single-rulesfrom
feat/addressable-parameterised-switch

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Member

Stacked on #970. Base is feat/addressable-single-rules. If #970 merges first, retarget this PR to master before merging it — I stacked #969 the same way, #968 merged first, and merging #969 delivered it to its base branch instead of master. That is what #970 exists to undo. I will retarget this myself as soon as #970 lands.

The generator found the expression being rewritten by counting parameters: one meant that one, anything else meant it could not read the method. So a switch over the expression that also takes a sort level was refused for its parameter list rather than for its switch.

It now asks the switch which parameter it governs, and treats every other parameter as captured. That is the same thing the factory shape already needed, so the two cases collapse into one: captured parameters — not the presence of a lambda — decide whether the arms can be a static readonly field and whether the apply lambda can be static.

That reads the three CommonDenominator sets: one switch, three sort levels.

CollapseMultipleFractions comes in with them and needed nothing — it is an ordinary one-parameter expr switch that was simply never marked. It is here because #970's second commit found the previous list of "what is left" was wrong about it.

before after
addressable sets 22 26
addressable rules 371 388
armless sets 8 4

PerfectSquare is deliberately not among them, and the reason is measured

It is a single is pattern — the shape #970 added — and it works: marked and wired, it generates one rule that agrees with its switch everywhere, once the corpus is given a surd. (The grammar makes no fractional exponent, so without one the set never fires and the replay compares nothing. CollapseToPerfectSquare wants u + 2·√u·√v + v, not a polynomial trinomial.)

What stopped it is cost:

duration
replaying PerfectSquare alone 5m10s
the other four sets here, together 9s
full suite without it 5m33s
full suite with it 10m15s

Deciding whether the cross term matches goes through Simplify on every node of the corpus, so verifying that one rule costs as much as the entire rest of the suite. Bundling it here would force one answer to two questions. Worth doing; worth deciding on its own.

What is left, and it really is one kind of thing now

RationalizeDenominator, ExpandFactorialDivisions, FactorizeFactorialMultiplications — methods with statement bodies, branches and locals, no arms to read — plus PerfectSquare, left out on cost rather than shape, which the test comment says.

Tests

The four join the replay test, each against the switch built at its own level where there is one. The armless example moves for the third time, to RationalizeDenominator — and 1 / sqrt(2) does not fire it, its denominator being neither a sum nor a difference, so the case is 1 / (3 - sqrt(5)). The assertion that the example set really has no rules has now caught three sets being addressed out from under it, so it stays.

AddressableRulesTest 33 passed in 11s. Full suite 7308 passed, 0 failed, 14 skipped, 5m33s.

No answer changes: ApplyOnce is handed the same Func; only the optional rule list beside it is new.

The generator found the expression being rewritten by counting parameters: one
meant that one, anything else meant it could not read the method. So a switch
over the expression that also takes a sort level was refused for the parameter
rather than for the switch.

It now asks the switch which parameter it governs, and treats every other
parameter as captured -- which is the same thing the factory shape already
needed, so the two cases collapse into one. Captured parameters, not the
presence of a lambda, now decide whether the arms can be a static field and
whether the apply lambda can be static.

That reads the three CommonDenominator sets, one switch over three sort levels.

CollapseMultipleFractions comes in with them and needed nothing: it is an
ordinary one-parameter `expr switch` that was simply never marked. It is here
because the previous commit's list of what was left was wrong about it, and a
list is worth less than the thing it describes.

  addressable sets   22 -> 26
  addressable rules  371 -> 388
  armless sets        8 -> 4

**PerfectSquare is deliberately not among them, and the reason is measured.**
It is a single `is` pattern, the shape read since the last commit, and it works:
marked and wired it generates one rule that agrees with its switch everywhere,
once the corpus is given a surd to work with -- the grammar makes no fractional
exponent, so without one the set never fires and the replay compares nothing.

What stopped it is cost. Replaying that one rule takes **5m10s**, because
deciding whether the cross term matches goes through Simplify on every node of
the corpus. The suite goes from 5m21s to 10m15s for it. The other four sets
here cost eight seconds between them, so bundling the two would force one answer
to two questions. Worth doing; worth deciding on its own.

The four join the replay test, each against the switch built at its own level
where there is one. The armless example moves for the third time, to
RationalizeDenominator, since CollapseMultipleFractions is no longer armless --
and `1 / sqrt(2)` does not fire it, its denominator being neither a sum nor a
difference, so the case is `1 / (3 - sqrt(5))`.

Measured: AddressableRulesTest 33 passed in 9s; full suite green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Rafael-SOWNet

Copy link
Copy Markdown
Member Author

Same as #969: this merged into its base branch, feat/addressable-single-rules, rather than into master.

$ git show origin/master:…/Core/Transformations/RewriteRules.cs | grep -c FractionCommonDenominatorRulesArms
0
$ git branch -r --contains e579bfea
  origin/feat/addressable-single-rules

My fault twice over — I wrote in the description above that I would retarget this before it merged, and did not get there first. #973 re-delivers it, based on master, so the shape cannot recur.

Nothing needs doing here; this is only so the record does not read as though the work is in master.

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