From 7cd196a0e870d2a6036dcd64ac4fb9b37bc2a0a1 Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Sun, 16 Aug 2026 17:42:41 +0000 Subject: [PATCH] Read a rule switch that takes a second parameter (#825, #746 item 50) 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 --- .../RuleRegistryGenerator.cs | 28 +++++++----- .../Core/Transformations/RewriteRules.cs | 14 +++--- .../Patterns/Patterns.Rational.cs | 2 + .../Transformations/AddressableRulesTest.cs | 44 +++++++++++-------- 4 files changed, 54 insertions(+), 34 deletions(-) diff --git a/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs b/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs index cb8e833b7..345192376 100644 --- a/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs +++ b/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs @@ -178,14 +178,22 @@ private static string Sanitize(string name) // they are emitted as a method taking the same parameters instead. var lambda = method.ExpressionBody?.Expression as SimpleLambdaExpressionSyntax; var inner = lambda?.Body as ExpressionSyntax ?? method.ExpressionBody?.Expression; - ParameterSyntax parameter; - if (lambda is not null) - parameter = lambda.Parameter; - else if (method.ParameterList.Parameters.Count == 1) - parameter = method.ParameterList.Parameters[0]; - else + // Which parameter is the expression being rewritten. A factory says so by being a + // lambda; a switch says so by naming it; and a method with one parameter has only the + // one to offer. A switch that takes a second parameter — a sort level, say — is read + // by asking the switch rather than by counting, which is the whole of this. + ParameterSyntax? parameter = + lambda is not null ? lambda.Parameter + : inner is SwitchExpressionSyntax { GoverningExpression: IdentifierNameSyntax named } + ? method.ParameterList.Parameters.FirstOrDefault(p => p.Identifier.Text == named.Identifier.Text) + : method.ParameterList.Parameters.Count == 1 ? method.ParameterList.Parameters[0] + : null; + if (parameter is null) return null; var subject = parameter.Identifier.Text; + // Everything else the method takes is closed over by the arms, so it decides whether + // they can be a field at all. + var captured = method.ParameterList.Parameters.Where(p => p != parameter).ToList(); // A lambda parameter is written without its type, and the arms are copied into a // context where nothing infers it, so it is named rather than echoed. var parameterType = parameter.Type?.ToString() ?? "global::AngouriMath.Entity"; @@ -200,7 +208,7 @@ private static string Sanitize(string name) text.AppendLine($"{indent}/// The arms of , each one addressable on its own."); text.AppendLine($"{indent}/// Generated from the switch itself, so the two cannot disagree."); text.AppendLine($"{indent}/// "); - if (lambda is null) + if (captured.Count == 0) { text.AppendLine($"{indent}[global::AngouriMath.Core.ConstantField]"); text.AppendLine($"{indent}internal static readonly global::System.Collections.Generic.IReadOnlyList" @@ -209,7 +217,7 @@ private static string Sanitize(string name) else text.AppendLine($"{indent}internal static global::System.Collections.Generic.IReadOnlyList" + $" {method.Identifier.Text}Arms" - + $"{method.ParameterList} =>"); + + $"({string.Join(", ", captured)}) =>"); text.AppendLine($"{indent} new global::AngouriMath.Core.Transformations.RewriteRule[]"); text.AppendLine($"{indent} {{"); @@ -240,8 +248,8 @@ private static string Sanitize(string name) text.AppendLine($"{indent} replacementSource: {Literal(replacement)},"); text.AppendLine($"{indent} growth: global::AngouriMath.Core.Transformations.RewriteRuleGrowth.{growth},"); text.AppendLine($"{indent} sourceLine: {line},"); - // A factory's arm reads the factory's parameters, so its lambda cannot be static. - text.AppendLine($"{indent} apply: {(lambda is null ? "static " : "")}global::AngouriMath.Entity? " + // An arm that reads a captured parameter cannot be a static lambda. + text.AppendLine($"{indent} apply: {(captured.Count == 0 ? "static " : "")}global::AngouriMath.Entity? " + $"({parameterType} {subject}) => {arm.Apply}),"); } diff --git a/Sources/AngouriMath/Core/Transformations/RewriteRules.cs b/Sources/AngouriMath/Core/Transformations/RewriteRules.cs index a98a9f7e9..e801db050 100644 --- a/Sources/AngouriMath/Core/Transformations/RewriteRules.cs +++ b/Sources/AngouriMath/Core/Transformations/RewriteRules.cs @@ -1,4 +1,4 @@ -// +// // Copyright (c) 2019-2026 Angouri. // AngouriMath is licensed under MIT. // Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md. @@ -205,7 +205,8 @@ public static class RewriteRules "Collapses nested quotients into a single numerator over a single denominator.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - Patterns.CollapseMultipleFractions); + Patterns.CollapseMultipleFractions, + Patterns.CollapseMultipleFractionsArms); /// /// Puts a sum of quotients over one denominator, grouping the terms by variables and @@ -216,7 +217,8 @@ public static class RewriteRules "Adds quotients by putting them over a common denominator.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.HIGH_LEVEL)); + expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.HIGH_LEVEL), + Patterns.FractionCommonDenominatorRulesArms(TreeAnalyzer.SortLevel.HIGH_LEVEL)); /// /// , counting constants when it groups terms. @@ -226,7 +228,8 @@ public static class RewriteRules "Adds quotients over a common denominator, distinguishing terms by their constants too.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.MIDDLE_LEVEL)); + expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.MIDDLE_LEVEL), + Patterns.FractionCommonDenominatorRulesArms(TreeAnalyzer.SortLevel.MIDDLE_LEVEL)); /// /// , grouping terms by the whole subtree. @@ -236,7 +239,8 @@ public static class RewriteRules "Adds quotients over a common denominator, grouping terms by the whole subtree.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.LOW_LEVEL)); + expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.LOW_LEVEL), + Patterns.FractionCommonDenominatorRulesArms(TreeAnalyzer.SortLevel.LOW_LEVEL)); /// /// Divides one polynomial by another, leaving a quotient plus a remainder. diff --git a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Rational.cs b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Rational.cs index 4798f882d..af3027d87 100644 --- a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Rational.cs +++ b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Rational.cs @@ -56,6 +56,7 @@ private static IEnumerable PairwiseGrouping(Entity num, Entity den, Tree return factors.Values; } + [AddressableRules] internal static Entity FractionCommonDenominatorRules(Entity expr, TreeAnalyzer.SortLevel level) => expr switch { @@ -160,6 +161,7 @@ internal static Entity RationalizeDenominator(Entity expr) return ((num * conjugate) / divisor).InnerSimplified; } + [AddressableRules] internal static Entity CollapseMultipleFractions(Entity expr) => expr switch { diff --git a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs index 19c4cc7a3..6b61d62f4 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs @@ -1,4 +1,4 @@ -// +// // Copyright (c) 2019-2026 Angouri. // AngouriMath is licensed under MIT. // Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md. @@ -67,6 +67,16 @@ public sealed class AddressableRulesTest yield return (RewriteRules.InvertNegativePowers, Patterns.InvertNegativePowers); yield return (RewriteRules.PolynomialLongDivision, Patterns.PolynomialLongDivision); yield return (RewriteRules.PolynomialGcdCancellation, Patterns.PolynomialGcdCancellation); + // A switch that takes a second parameter: one switch, three sets, differing only in + // the sort level it is closed over. + yield return (RewriteRules.CommonDenominator, + expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.HIGH_LEVEL)); + yield return (RewriteRules.CommonDenominatorCountingConstants, + expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.MIDDLE_LEVEL)); + yield return (RewriteRules.CommonDenominatorExact, + expr => Patterns.FractionCommonDenominatorRules(expr, TreeAnalyzer.SortLevel.LOW_LEVEL)); + // A shape that was already readable and simply not marked. + yield return (RewriteRules.CollapseMultipleFractions, Patterns.CollapseMultipleFractions); } public static IEnumerable AddressableSets() @@ -262,27 +272,23 @@ public void TheRegistryIsAddressableAsFarAsItSaysItIs() var without = RewriteRules.All.Where(set => set.Rules.Count == 0) .Select(set => set.Name).OrderBy(name => name, StringComparer.Ordinal).ToList(); - // What is left is not one kind of thing, and saying so would be wrong: of these eight, - // only RationalizeDenominator, ExpandFactorialDivisions and - // FactorizeFactorialMultiplications are methods with branches and locals. The three - // CommonDenominator sets are a switch that takes a second parameter, which the - // generator does not read yet; CollapseMultipleFractions is an ordinary one-parameter - // switch and PerfectSquare a single `is` pattern, both of which it reads today and - // neither of which is marked. So this list is a list, not a category. + // Three of these four are a method with a statement body, branches and locals, and no + // arms to read. PerfectSquare is not: it is a single `is` pattern that the generator + // reads perfectly well, and it is left out on cost rather than on shape. Replaying one + // rule against its switch over this corpus takes 5m10s, because deciding whether the + // cross term matches goes through Simplify -- as long as the entire rest of the suite. + // Worth doing, worth deciding on its own, and not worth smuggling in beside four sets + // that cost eight seconds between them. Assert.Equal(new[] { - "CollapseMultipleFractions", - "CommonDenominator", - "CommonDenominatorCountingConstants", - "CommonDenominatorExact", "ExpandFactorialDivisions", "FactorizeFactorialMultiplications", "PerfectSquare", "RationalizeDenominator", }, without); - Assert.Equal(22, withRules.Count); - Assert.Equal(371, withRules.Sum(set => set.Rules.Count)); + Assert.Equal(26, withRules.Count); + Assert.Equal(388, withRules.Sum(set => set.Rules.Count)); } [Fact] @@ -357,16 +363,16 @@ public void ASetWithNoAddressableRulesStillRecordsItsStep() { // Stated rather than assumed: if this set ever becomes addressable the test would // otherwise keep passing while testing nothing at all. It has already happened twice - // -- CanonicalOrderExact stood here, then InvertNegativePowers -- so the assertion is - // load-bearing rather than decorative. - Assert.Empty(RewriteRules.CollapseMultipleFractions.Rules); + // -- CanonicalOrderExact, then InvertNegativePowers, then CollapseMultipleFractions -- + // so the assertion is load-bearing rather than decorative. + Assert.Empty(RewriteRules.RationalizeDenominator.Rules); using var recording = RewriteRecording.Start(); - RewriteRules.CollapseMultipleFractions.ApplyOnce("x / y / z".ToEntity()); + RewriteRules.RationalizeDenominator.ApplyOnce("1 / (3 - sqrt(5))".ToEntity()); recording.Dispose(); var step = Assert.Single(recording.Steps); - Assert.Equal(RewriteRules.CollapseMultipleFractions, step.RuleSet); + Assert.Equal(RewriteRules.RationalizeDenominator, step.RuleSet); Assert.Null(step.Rule); } }