Skip to content

Read a rule set that is a single rule (#825) — re-delivering #969 to master - #970

Merged
Rafael-SOWNet merged 2 commits into
masterfrom
feat/addressable-single-rules
Aug 16, 2026
Merged

Rafael-SOWNet merged 2 commits into
masterfrom
feat/addressable-single-rules

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Member

This re-delivers #969, which merged but did not reach master. My mistake, and worth stating plainly: I opened #969 stacked on feat/addressable-rule-factories (#968). #968 merged to master first, so when #969 was merged it went into its base branch rather than master — the squash commit a38d455a sits on feat/addressable-rule-factories, and master has none of it:

$ git show origin/master:…/RewriteRules.cs | grep -c 'InvertNegativePowersArms\|PolynomialLongDivisionArms'
0

I should have retargeted #969's base the moment #968 landed. Nothing is wrong with the code — it is the same two commits, cherry-picked onto current master (upstream squash-merges, so a rebase would have replayed #968's commits against themselves).

The content, unchanged from #969:

Three rule sets are one rule each, written the way a single rule is written when there is no need for a switch:

x is pattern [&& guard] ? replacement : x

InvertNegativePowers, PolynomialLongDivision, PolynomialGcdCancellation. The generator reads that shape and emits exactly one rule — pattern from the is, guard from whatever conjuncts follow it, replacement from the true branch.

  • The false branch has to be the subject itself. Anything else is a second rewrite, and emitting one rule for it would silently drop the other.
  • The condition is kept whole rather than rebuilt from pattern + guard, because a later conjunct may declare what the replacement reads: TreeAnalyzer.PolynomialLongDivision(num, denom) is var (divided, remainder) is exactly that, and divided + remainder needs it.
before after
addressable sets 19 22
addressable rules 368 371
armless sets 11 8

The second commit corrects a claim I made in the first: that what remains is armless "because it is a method with branches and locals". Only three of the eight are. CollapseMultipleFractions is an ordinary one-parameter expr switch, PerfectSquare is this PR's own shape, and the three CommonDenominator sets are a switch with a second parameter — so the list is a list, not a category. A follow-up takes four of those five.

The tests bind

RewriteRules.PolynomialGcdCancellation  ->  Patterns.PolynomialLongDivisionArms

  Failed AddressableRulesTest.EveryArmIsTheArmItWasGeneratedFrom(setName: "PolynomialGcdCancellation")
  Failed: 1, Passed: 28

Reverted; 29/29 pass. Two defects found by reading the generated output rather than assuming it: <see cref="X"/> left See for what is and is not attempted with a hole in it, and tag-stripping applied to // comments ate a ^ (-1) => 1 / a down to a ^ (-1) 1 / a. Only /// is XML now — verified across all 371 rules, 92 described, 8 containing < or >, all 8 intact.

Suite 7304 passed, 0 failed, 14 skipped, corpus gate included. No answer changes: ApplyOnce is handed the same Func, only the optional rule list beside it is new.

Rafael-SOWNet and others added 2 commits August 16, 2026 17:42
Three of the eleven sets still without addressable rules are one rule each,
written the way one rule is written when there is no need for a switch:

    x is pattern [&& guard] ? replacement : x

InvertNegativePowers, PolynomialLongDivision and PolynomialGcdCancellation.
There is nothing in them to split, so the generator now reads the shape and
emits exactly one rule -- the pattern from the `is`, the guard from whatever
conjuncts follow it, the replacement from the true branch.

The false branch has to be the subject itself. Anything else is a second
rewrite, and emitting one rule for it would silently drop the other.

The condition is kept whole in the generated `apply` rather than rebuilt from
the pattern and the guard, because a later conjunct may declare what the
replacement reads -- `TreeAnalyzer.PolynomialLongDivision(num, denom) is var
(divided, remainder)` is exactly that, and `divided + remainder` needs it.

  addressable sets   19 -> 22
  addressable rules  368 -> 371
  armless sets       11 -> 8

What is left is armless because it is a method with branches and locals,
which has no arms to read. That is now the only reason left, so the pinned
list says so.

The three join the replay test, each against itself, since a set that is one
rule is its own switch. It binds: wiring PolynomialGcdCancellation's arms to
PolynomialLongDivisionArms fails it by name.

ASetWithNoAddressableRulesStillRecordsItsStep moves again, to
CollapseMultipleFractions. Its example set has now been addressed out from
under it twice, so the assertion that the set really has no rules is
load-bearing rather than decorative, and says so.

Two defects found by reading the generated output rather than assuming it:

- A `///` summary has its meaning in the attribute, so stripping tags left
  "See  for what is and is not attempted" with a hole in it. A `see cref` now
  contributes the name it points at.
- Tag-stripping must not touch a `//` comment. Mathematics is full of `<` and
  `>`, and reading prose as XML ate `a ^ (-1) => 1 / a` down to `a ^ (-1) 1 /
  a`. Only `///` is XML. Verified across all 371 rules: 92 carry a
  description, 8 of those contain `<` or `>`, and all 8 are intact -- `=> b -
  a`, `|n| >= 2`, `ab < 1`, `(-pi/2, pi/2]`, `any1 > 0`.

Environment.NewLine is banned in an analyzer (RS1035) and rightly so, since
generator output has to be identical on every platform; the emitted newline
is a literal.

Measured: suite 7304 passed, 0 failed, 14 skipped, corpus gate included. No
answer changes -- ApplyOnce is handed the same Func it was before, and only
the optional list of rules alongside it is new.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment added with the previous commit claimed the sets still without
addressable rules are armless "because it is a method with branches and
locals", and that this is now the only reason left. Reading them says
otherwise, and only three of the eight are that:

  RationalizeDenominator            statement body, branches and locals
  ExpandFactorialDivisions          statement body, with a local function
  FactorizeFactorialMultiplications statement body

  CommonDenominator                 expr switch, plus a SortLevel parameter
  CommonDenominatorCountingConstants  ditto
  CommonDenominatorExact              ditto

  CollapseMultipleFractions         an ordinary one-parameter expr switch
  PerfectSquare                     x is Sumf or Minusf && ... ? square : x

The last two are shapes the generator reads today. They are not addressable
because nothing marks them, which is a different fact from the one the comment
asserted, and a smaller one.

The claim was written from the three sets this branch had just looked at and
generalised to the rest without opening them -- the same mistake as a
measurement that stops one step early. Corrected here rather than in the
follow-up that acts on it, so the comment is not false in between.

No behaviour change: a comment.

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