From fe9968d43cba259dc73b7b72bff89bca82bb3ec1 Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Sun, 9 Aug 2026 09:26:36 +0000 Subject: [PATCH] Return null from the limit descent's default, not the node (#829, #833) ComputeLimitDivideEtImpera defaulted to new Limitf(this, x, dist, side). That is the cycle AGENTS.md names, with this exact expression as its example: the caller evaluates the returned node to compare it, evaluating a Limitf computes the limit, and computing arrives back at the default. It overflows the stack, which kills the process rather than raising anything a caller can catch. So a node crashed by inheriting the default -- by not being mentioned anywhere. Seven did: limit(floor(x), x, 0) stack overflow, process dies limit(ceil(x), x, 0) stack overflow limit(round(x), x, 0) stack overflow limit(min(x, 1), x, 0) stack overflow limit(max(x, 1), x, 0) stack overflow limit(gcd(x, 2), x, 0) stack overflow limit(phi(x), x, 0) stack overflow The first six arrived with #827 and #828, which added the nodes without the ComputeLimitDivideEtImpera override that AddingNode.cs lists at step 3d. phi predates both and is #833, so this is not only my regression -- the default has been a landmine for every node that ever inherited it, which is what #704 was. Returning null is what AGENTS.md prescribes and what every caller already expects: each of the eleven call sites tests the result with `is { }` or switches on it, and the signature was already Entity?. All seven now come back as an unevaluated limit node, which is the honest "I could not settle this", and nothing else moves -- 5928 tests passed before this change and after it. LimitTerminatesOnEveryNodeTest enumerates the node types by reflection rather than listing them, because the defect is in what a node inherits by not being mentioned, and a per-node test cannot catch that: 45 types, each over three destinations and three sides, asserting only that the call returns. Checked against the unfixed code, where the run aborts. This does not replace #831, which gives floor and ceil real limit answers and fixes the OverflowException of #830. That adds capability; this removes the way to crash. They compose, and the order does not matter. Tests: 5973 passing, 0 failed, 14 skipped; F# 130. Co-Authored-By: Claude Opus 5 (1M context) --- .../Continuous/Limits/Limit.Definition.cs | 27 ++++- .../LimitTerminatesOnEveryNodeTest.cs | 112 ++++++++++++++++++ 2 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 Sources/Tests/UnitTests/Calculus/LimitTerminatesOnEveryNodeTest.cs diff --git a/Sources/AngouriMath/Functions/Continuous/Limits/Limit.Definition.cs b/Sources/AngouriMath/Functions/Continuous/Limits/Limit.Definition.cs index 1754e1d61..1c3ffb1da 100644 --- a/Sources/AngouriMath/Functions/Continuous/Limits/Limit.Definition.cs +++ b/Sources/AngouriMath/Functions/Continuous/Limits/Limit.Definition.cs @@ -140,12 +140,29 @@ public Entity Limit(Variable x, Entity destination) /// the same or related type, until these become simple enough to be solved /// directly. The solutions to the sub-problems are then combined to give a /// solution to the original problem. + /// + /// Here we try to compute the limit for each child and then merge them into the limit + /// of the whole expression. Theoretically, for cases such as limit (x -> -1) 1 / (x + 1) + /// this method will return NaN, but thanks to replacement of x by a non-definite + /// expression, it is somehow compensated. /// - // here we try to compute limit for each children and then merge them into limit of whole expression - // theoretically, for cases such limit (x -> -1) 1 / (x + 1) - // this method will return NaN, but thanks to replacement of x to an non-definite expression, - // it is somehow compensated + /// + /// Null, not . This used to hand back + /// new Limitf(this, ...), which reads as the honest "I could not settle this" + /// and is in fact a cycle: the caller evaluates the node to compare it, evaluating a + /// computes the limit, and computing arrives back here. That + /// overflows the stack, which kills the process rather than raising anything a caller + /// can catch — the exact failure AGENTS.md names, with this exact expression as its + /// example. + /// + /// It made every node without an override a landmine, since inheriting the default + /// was enough to crash: floor, ceil, round, min, max, gcd and phi all did. + /// Returning null reads the same to every caller here — they all test with + /// is { } — and terminates. + /// https://github.com/asc-community/AngouriMath/issues/829 + /// https://github.com/asc-community/AngouriMath/issues/833 + /// internal virtual Entity? ComputeLimitDivideEtImpera(Variable x, Entity dist, ApproachFrom side) - => new Limitf(this, x, dist, side); + => null; } } \ No newline at end of file diff --git a/Sources/Tests/UnitTests/Calculus/LimitTerminatesOnEveryNodeTest.cs b/Sources/Tests/UnitTests/Calculus/LimitTerminatesOnEveryNodeTest.cs new file mode 100644 index 000000000..3ff00033c --- /dev/null +++ b/Sources/Tests/UnitTests/Calculus/LimitTerminatesOnEveryNodeTest.cs @@ -0,0 +1,112 @@ +// +// Copyright (c) 2019-2026 Angouri. +// AngouriMath is licensed under MIT. +// Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md. +// Website: https://am.angouri.org. +// + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Threading.Tasks; +using AngouriMath; +using AngouriMath.Core; +using AngouriMath.Extensions; +using Xunit; + +namespace AngouriMath.Tests.Calculus +{ + /// + /// A limit over any node terminates. + /// + /// + /// + /// used to default to + /// new Limitf(this, ...), so a node that did not override it crashed the process: + /// the caller evaluates the returned node to compare it, evaluating a Limitf + /// computes the limit, and computing arrives back at the default. Seven node types + /// inherited it and every one of them overflowed the stack — + /// floor, ceil, round, min, max, gcd and + /// phi. + /// + /// + /// The per-node regression tests could not have caught it, because the defect is in what + /// a node inherits by not being mentioned. So this enumerates the node types + /// instead of listing them: a node added tomorrow is covered on the day it is added, + /// which is the only shape of test that fixes this class of bug rather than its + /// instances. + /// + /// + /// Termination is the assertion, not the answer. Most of these have no limit this + /// library can compute and come back unevaluated, which is the honest result; what is + /// forbidden is not returning at all. + /// + /// https://github.com/asc-community/AngouriMath/issues/829 + /// https://github.com/asc-community/AngouriMath/issues/833 + /// + [Trait("Area", "Calculus")] + public sealed class LimitTerminatesOnEveryNodeTest + { + /// + /// One instance of every concrete node type under that can be + /// built from a variable, so that the limit has something to descend into. + /// + public static IEnumerable EveryNodeType() + { + var x = MathS.Var("x"); + var built = new List(); + + foreach (var type in typeof(Entity).Assembly + .GetTypes() + .Where(t => t.IsSealed && !t.IsAbstract && typeof(Entity).IsAssignableFrom(t)) + .OrderBy(t => t.FullName, StringComparer.Ordinal)) + { + // The one-argument constructor over Entity is the common shape; where a node + // takes more, fill every parameter it will accept with the variable. + var constructor = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance) + .Where(c => c.GetParameters().Length > 0 + && c.GetParameters().All(p => p.ParameterType == typeof(Entity))) + .OrderBy(c => c.GetParameters().Length) + .FirstOrDefault(); + if (constructor is null) + continue; + + Entity node; + try + { + node = (Entity)constructor.Invoke( + constructor.GetParameters().Select(_ => (object)x).ToArray()); + } + catch (Exception) + { + // A node whose constructor rejects a plain variable is not the subject + // here; the point is the ones that accept one and then recurse. + continue; + } + built.Add(node); + } + + Assert.NotEmpty(built); + return built.Select(n => new object[] { n }); + } + + [Theory] + [MemberData(nameof(EveryNodeType))] + public void ALimitOverAnyNodeTerminates(Entity node) + { + // In a task with a timeout rather than measured for speed: a regression here is + // non-termination, and the run must fail rather than hang the suite. A stack + // overflow cannot be caught at all, so the guard is that the assembly gets to + // run its remaining tests. + var finished = Task.Run(() => + { + foreach (var destination in new Entity[] { 0, "+oo".ToEntity(), "-oo".ToEntity() }) + foreach (var side in new[] { ApproachFrom.BothSides, ApproachFrom.Left, ApproachFrom.Right }) + node.Limit(MathS.Var("x"), destination, side); + }).Wait(TimeSpan.FromSeconds(30)); + + Assert.True(finished, $"a limit over {node.Stringize()} did not terminate"); + } + } +}