From 54218458a1a11a29d144ce34ebfaae686c746bef Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Sun, 16 Aug 2026 16:41:04 +0000 Subject: [PATCH 1/2] Read a rule set that is a single rule (#825, #746 item 50) 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 --- .../RuleRegistryGenerator.cs | 209 +++++++++++++++--- .../Core/Transformations/RewriteRules.cs | 9 +- .../Simplification/Patterns/Patterns.Power.cs | 1 + .../Simplification/Patterns/Patterns.cs | 2 + .../Transformations/AddressableRulesTest.cs | 28 +-- 5 files changed, 198 insertions(+), 51 deletions(-) diff --git a/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs b/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs index ff227d092..cb8e833b7 100644 --- a/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs +++ b/Sources/Analyzers/RuleRegistryGenerator/RuleRegistryGenerator.cs @@ -130,7 +130,8 @@ internal GeneratedFile(string hintName, string source) // severity setting, which matters because the failure mode being guarded against // is a rule set silently having no rules rather than a rule being wrong. text.AppendLine($"{indent}#error AddressableRules: '{method.Identifier.Text}' must be an " - + "expression-bodied method whose body is `parameter switch { ... }`."); + + "expression-bodied method whose body is `x switch { ... }`, `x => x switch { ... }`, " + + "or `x is pattern [&& guard] ? replacement : x`."); } else text.Append(body); @@ -166,35 +167,32 @@ private static string Sanitize(string name) private static string? Body(MethodDeclarationSyntax method, string indent) { - // Two shapes are read. A rule set written directly as a switch over its expression, + // Three shapes are read. A rule set written directly as a switch over its expression, // static Entity CommonRules(Entity x) => x switch { ... } - // and a *factory* for one, which is how a set parameterised by something that is not - // the expression is written, + // a *factory* for one, which is how a set parameterised by something that is not the + // expression is written, // static Func SortRules(SortLevel level) => x => x switch { ... } + // and a set that is a single rule, which needs no switch to say so, + // static Entity InvertNegativePowers(Entity x) => x is Powf(...) ? ... : x; // The arms of a factory close over its parameters, so they cannot be a static field; - // they are emitted as a method taking the same parameters instead. Everything after - // this point is common to both, which is the point of unwrapping here. + // they are emitted as a method taking the same parameters instead. var lambda = method.ExpressionBody?.Expression as SimpleLambdaExpressionSyntax; - if ((lambda?.Body ?? method.ExpressionBody?.Expression) is not SwitchExpressionSyntax dispatch) - return null; - if (dispatch.GoverningExpression is not IdentifierNameSyntax governing) - return null; + var inner = lambda?.Body as ExpressionSyntax ?? method.ExpressionBody?.Expression; ParameterSyntax parameter; - if (lambda is null) - { - if (method.ParameterList.Parameters.Count != 1) - return null; + if (lambda is not null) + parameter = lambda.Parameter; + else if (method.ParameterList.Parameters.Count == 1) parameter = method.ParameterList.Parameters[0]; - } else - parameter = lambda.Parameter; - if (parameter.Identifier.Text != governing.Identifier.Text) return null; + var subject = parameter.Identifier.Text; // 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"; - var arms = dispatch.Arms.Where(arm => arm.Pattern is not DiscardPatternSyntax).ToList(); + var arms = ArmsOf(inner, subject, indent, method.GetLeadingTrivia()); + if (arms is null) + return null; var names = new Dictionary(StringComparer.Ordinal); var text = new StringBuilder(); @@ -219,22 +217,22 @@ private static string Sanitize(string name) { var arm = arms[index]; var pattern = Flatten(arm.Pattern.ToString()); - var guard = arm.WhenClause is null ? null : Flatten(arm.WhenClause.Condition.ToString()); - var replacement = Flatten(arm.Expression.ToString()); + var guard = arm.Guard is null ? null : Flatten(arm.Guard); + var replacement = Flatten(arm.Replacement.ToString()); var key = guard is null ? pattern : pattern + " when " + guard; names.TryGetValue(key, out var seen); names[key] = seen + 1; var name = seen == 0 ? key : $"{key} #{seen + 1}"; - var line = arm.GetLocation().GetLineSpan().StartLinePosition.Line + 1; + var line = arm.Source.GetLocation().GetLineSpan().StartLinePosition.Line + 1; var nodeTypes = RootTypesOf(arm.Pattern); - var growth = Growth(arm.Pattern, arm.Expression); + var growth = Growth(arm.Pattern, arm.Replacement); text.AppendLine($"{indent} new global::AngouriMath.Core.Transformations.RewriteRule("); text.AppendLine($"{indent} source: \"{method.Identifier.Text}\","); text.AppendLine($"{indent} index: {index},"); text.AppendLine($"{indent} name: {Literal(name)},"); - text.AppendLine($"{indent} description: {Literal(Description(arm))},"); + text.AppendLine($"{indent} description: {Literal(arm.Description)},"); text.AppendLine($"{indent} nodeTypes: new global::System.Type[] {{ " + string.Join(", ", nodeTypes.Select(type => $"typeof({type})")) + " },"); text.AppendLine($"{indent} patternSource: {Literal(pattern)},"); @@ -244,28 +242,169 @@ private static string Sanitize(string name) 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? " - + $"({parameterType} {parameter.Identifier.Text}) => {parameter.Identifier.Text} switch"); - text.AppendLine($"{indent} {{"); - text.AppendLine($"{indent} {arm.Pattern}{(arm.WhenClause is null ? "" : " " + arm.WhenClause)} => {arm.Expression},"); - text.AppendLine($"{indent} _ => null"); - text.AppendLine($"{indent} }}),"); + + $"({parameterType} {subject}) => {arm.Apply}),"); } text.AppendLine($"{indent} }};"); return text.ToString(); } - /// The comment written above an arm, which is the rule stated as mathematics. - private static string? Description(SwitchExpressionArmSyntax arm) + /// One rule, whatever shape the method that holds it was written in. + private sealed class Arm + { + internal Arm(PatternSyntax pattern, string? guard, ExpressionSyntax replacement, + SyntaxNode source, string? description, string apply) + => (Pattern, Guard, Replacement, Source, Description, Apply) + = (pattern, guard, replacement, source, description, apply); + + internal PatternSyntax Pattern { get; } + internal string? Guard { get; } + internal ExpressionSyntax Replacement { get; } + /// What the rule was read from, for its line number. + internal SyntaxNode Source { get; } + internal string? Description { get; } + /// The body of the generated apply, copied from the source it came from. + internal string Apply { get; } + } + + /// + /// The rules a method body holds, or where it is not a shape this + /// can read. is the method's own leading trivia, which describes + /// the rule when the method is one. + /// + private static List? ArmsOf(ExpressionSyntax? body, string subject, string indent, SyntaxTriviaList ownDoc) + { + switch (body) + { + case SwitchExpressionSyntax dispatch + when dispatch.GoverningExpression is IdentifierNameSyntax governing + && governing.Identifier.Text == subject: + return dispatch.Arms + .Where(arm => arm.Pattern is not DiscardPatternSyntax) + .Select(arm => new Arm( + arm.Pattern, + arm.WhenClause?.Condition.ToString(), + arm.Expression, + arm, + Description(arm.GetLeadingTrivia()), + // A one-armed switch over the same subject, so the pattern binds its + // variables exactly as it does where it is written. + $"{subject} switch\n" + + $"{indent} {{\n" + + $"{indent} {arm.Pattern}" + + $"{(arm.WhenClause is null ? "" : " " + arm.WhenClause)} => {arm.Expression},\n" + + $"{indent} _ => null\n" + + $"{indent} }}")) + .ToList(); + + // `subject is pattern [&& guard] ? replacement : subject` — a set that is a single + // rule and needs no switch to say so. The false branch has to be the subject + // itself; anything else is a second rewrite this would silently drop. + case ConditionalExpressionSyntax conditional + when conditional.WhenFalse is IdentifierNameSyntax unchanged + && unchanged.Identifier.Text == subject: + { + var conjuncts = Conjuncts(conditional.Condition); + if (conjuncts[0] is not IsPatternExpressionSyntax test + || test.Expression is not IdentifierNameSyntax tested + || tested.Identifier.Text != subject) + return null; + return new List + { + new Arm( + test.Pattern, + conjuncts.Count == 1 ? null : string.Join(" && ", conjuncts.Skip(1)), + conditional.WhenTrue, + conditional, + Description(ownDoc), + // The condition is kept whole rather than rebuilt from the pattern and + // the guard: a later conjunct may declare what the replacement reads, + // which is how the polynomial rules are written. + $"{conditional.Condition} ? {conditional.WhenTrue} : null") + }; + } + + default: + return null; + } + } + + /// The value of one attribute of an XML tag, or null where it carries none. + private static string? AttributeOf(string tag, string name) + { + var at = tag.IndexOf(name + "=\"", StringComparison.Ordinal); + if (at < 0) + return null; + var from = at + name.Length + 2; + var to = tag.IndexOf('"', from); + return to < 0 ? null : tag.Substring(from, to - from); + } + + /// The operands of a chain of &&, left to right. + private static List Conjuncts(ExpressionSyntax expression) { - var lines = arm.GetLeadingTrivia() - .Where(trivia => trivia.IsKind(SyntaxKind.SingleLineCommentTrivia)) - .Select(trivia => trivia.ToString().TrimStart('/').Trim()) - .Where(line => line.Length > 0) - .ToList(); + if (expression is not BinaryExpressionSyntax binary || !binary.IsKind(SyntaxKind.LogicalAndExpression)) + return new List { expression }; + var operands = Conjuncts(binary.Left); + operands.AddRange(Conjuncts(binary.Right)); + return operands; + } + + /// The comment written above a rule, which is the rule stated as mathematics. + private static string? Description(SyntaxTriviaList trivia) + { + // A `//` comment is prose and is taken as written: mathematics is full of `<` and `>`, + // and reading it as XML would eat `a ^ (-1) => 1 / a` down to `a ^ (-1) 1 / a`. Only a + // `///` comment is XML, and only there are the tags stripped. + var lines = new List(); + foreach (var item in trivia) + { + var line = + item.IsKind(SyntaxKind.SingleLineCommentTrivia) ? item.ToString().TrimStart('/').Trim() + : item.IsKind(SyntaxKind.SingleLineDocumentationCommentTrivia) ? WithoutTags(item.ToString()) + : ""; + if (line.Length > 0) + lines.Add(line); + } return lines.Count == 0 ? null : string.Join(" ", lines); } + /// + /// A comment's text without its slashes or its XML, so that a <summary> above a + /// single-rule method reads the same way a // above an arm does. + /// + private static string WithoutTags(string comment) + { + var text = new StringBuilder(comment.Length); + for (var i = 0; i < comment.Length; i++) + { + if (comment[i] != '<') + { + text.Append(comment[i]); + continue; + } + var end = comment.IndexOf('>', i); + if (end < 0) + break; + // A `see` carries its meaning in the attribute and has nothing between its tags, so + // dropping it wholesale leaves a hole: "See for what is and is not attempted". + var cref = AttributeOf(comment.Substring(i + 1, end - i - 1), "cref"); + if (cref is not null) + { + var head = cref.Substring(cref.IndexOf(':') + 1); + var arguments = head.IndexOf('('); + if (arguments >= 0) + head = head.Substring(0, arguments); + text.Append(head.Substring(head.LastIndexOf('.') + 1)); + } + i = end; + } + return string.Join(" ", text.ToString() + .Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries) + .Select(line => line.TrimStart().TrimStart('/').Trim()) + .Where(line => line.Length > 0)); + } + /// /// The node types the pattern admits at its root. Usually one; two where the arm is an /// or of node types, and none where the constraint cannot be read off the syntax. diff --git a/Sources/AngouriMath/Core/Transformations/RewriteRules.cs b/Sources/AngouriMath/Core/Transformations/RewriteRules.cs index 69741bdc1..a98a9f7e9 100644 --- a/Sources/AngouriMath/Core/Transformations/RewriteRules.cs +++ b/Sources/AngouriMath/Core/Transformations/RewriteRules.cs @@ -85,7 +85,8 @@ public static class RewriteRules "Rewrites negative powers as quotients.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - Patterns.InvertNegativePowers); + Patterns.InvertNegativePowers, + Patterns.InvertNegativePowersArms); /// /// Brings a negative numeric factor out in front of the term it multiplies. @@ -245,7 +246,8 @@ public static class RewriteRules "Divides a polynomial by a polynomial, giving the quotient plus the remainder.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - Patterns.PolynomialLongDivision); + Patterns.PolynomialLongDivision, + Patterns.PolynomialLongDivisionArms); /// /// Puts a quotient of polynomials into lowest terms. @@ -255,7 +257,8 @@ public static class RewriteRules "Cancels the greatest common divisor of a polynomial quotient's numerator and denominator.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - Patterns.PolynomialGcdCancellation); + Patterns.PolynomialGcdCancellation, + Patterns.PolynomialGcdCancellationArms); #endregion diff --git a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs index 9dcae3108..21819bf84 100644 --- a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs +++ b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs @@ -13,6 +13,7 @@ namespace AngouriMath.Functions partial class Patterns { /// a ^ (-1) => 1 / a + [AddressableRules] internal static Entity InvertNegativePowers(Entity expr) => expr is Powf(var @base, Integer { IsNegative: true } pow) ? 1 / MathS.Pow(@base, -1 * pow) diff --git a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.cs b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.cs index 767d9d491..0bbdae5e6 100644 --- a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.cs +++ b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.cs @@ -46,6 +46,7 @@ private static Entity SortAndGroup(IEnumerable children, TreeAnalyzer.So SortAndGroup(Xorf.LinearChildren(x), level, (a, b) => a ^ b), _ => x, }; + [AddressableRules] internal static Entity PolynomialLongDivision(Entity x) => x is Divf(var num, var denom) && TreeAnalyzer.PolynomialLongDivision(num, denom) is var (divided, remainder) @@ -58,6 +59,7 @@ x is Divf(var num, var denom) /// See for what is and is not attempted, and /// #55. /// + [AddressableRules] internal static Entity PolynomialGcdCancellation(Entity x) => x is Divf(var num, var denom) && PolynomialGcd.TryCancel(num, denom, out var cancelled) diff --git a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs index 0ade7ce92..b90afae1a 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs @@ -63,6 +63,10 @@ public sealed class AddressableRulesTest yield return (RewriteRules.CanonicalOrder, Patterns.SortRules(TreeAnalyzer.SortLevel.HIGH_LEVEL)); yield return (RewriteRules.CanonicalOrderCountingConstants, Patterns.SortRules(TreeAnalyzer.SortLevel.MIDDLE_LEVEL)); yield return (RewriteRules.CanonicalOrderExact, Patterns.SortRules(TreeAnalyzer.SortLevel.LOW_LEVEL)); + // Sets that are a single rule: the method is the rule, so it is its own switch here. + yield return (RewriteRules.InvertNegativePowers, Patterns.InvertNegativePowers); + yield return (RewriteRules.PolynomialLongDivision, Patterns.PolynomialLongDivision); + yield return (RewriteRules.PolynomialGcdCancellation, Patterns.PolynomialGcdCancellation); } public static IEnumerable AddressableSets() @@ -258,10 +262,9 @@ public void TheRegistryIsAddressableAsFarAsItSaysItIs() var without = RewriteRules.All.Where(set => set.Rules.Count == 0) .Select(set => set.Name).OrderBy(name => name, StringComparer.Ordinal).ToList(); - // The three CanonicalOrder sets are not here: their rules are written as a switch - // inside a factory parameterised by the sort level, which the generator now reads. - // What is left is genuinely armless -- a polynomial division, a method with branches - // and locals, a single `is` pattern that is one rule and has nothing to split. + // What is left is armless because it is a method with branches and locals, which has + // no arms to read. Everything written as a switch -- directly, or inside a factory -- + // and everything that is a single `is` pattern is now addressable. Assert.Equal(new[] { "CollapseMultipleFractions", @@ -270,15 +273,12 @@ public void TheRegistryIsAddressableAsFarAsItSaysItIs() "CommonDenominatorExact", "ExpandFactorialDivisions", "FactorizeFactorialMultiplications", - "InvertNegativePowers", "PerfectSquare", - "PolynomialGcdCancellation", - "PolynomialLongDivision", "RationalizeDenominator", }, without); - Assert.Equal(19, withRules.Count); - Assert.Equal(368, withRules.Sum(set => set.Rules.Count)); + Assert.Equal(22, withRules.Count); + Assert.Equal(371, withRules.Sum(set => set.Rules.Count)); } [Fact] @@ -352,15 +352,17 @@ public void ARecordedStepNamesTheRuleAndNotOnlyTheSet() public void ASetWithNoAddressableRulesStillRecordsItsStep() { // Stated rather than assumed: if this set ever becomes addressable the test would - // otherwise keep passing while testing nothing at all. - Assert.Empty(RewriteRules.InvertNegativePowers.Rules); + // 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); using var recording = RewriteRecording.Start(); - RewriteRules.InvertNegativePowers.ApplyOnce("x ^ (-2)".ToEntity()); + RewriteRules.CollapseMultipleFractions.ApplyOnce("x / y / z".ToEntity()); recording.Dispose(); var step = Assert.Single(recording.Steps); - Assert.Equal(RewriteRules.InvertNegativePowers, step.RuleSet); + Assert.Equal(RewriteRules.CollapseMultipleFractions, step.RuleSet); Assert.Null(step.Rule); } } From de8b94c6a4fe56a9195d30cd8180fb4d182bd003 Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Sun, 16 Aug 2026 16:54:28 +0000 Subject: [PATCH 2/2] Say what the remaining eight sets actually are, having checked 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 --- .../Core/Transformations/AddressableRulesTest.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs index b90afae1a..19c4cc7a3 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs @@ -262,9 +262,13 @@ 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 armless because it is a method with branches and locals, which has - // no arms to read. Everything written as a switch -- directly, or inside a factory -- - // and everything that is a single `is` pattern is now addressable. + // 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. Assert.Equal(new[] { "CollapseMultipleFractions",