From 88bdc624ec5b7f53ed2ef5131e4bc007f10740aa Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:19:25 -0600 Subject: [PATCH 01/16] fix(collections): hash sets independently of element order Equal sets such as {1,2} and {2,1} used to get different hash codes, because GetHashCode hashed ToString(), which lists elements in the order they were written. Any set of sets could then miss a lookup: {{1,2}}.Contains({2,1}) was False and {{1,2},{2,1}} kept both. Sets now combine their element hashes with an order-independent sum, lists hash their elements in order, and values hash their text. Equals now also checks that both sides are the same kind (value, list or set). Before, an empty set compared equal to an empty list and to any plain value. Adds a test project (tests/SPADE.Tests, xUnit) with tests for both. Closes #9 --- DiscreteParser.csproj | 2 + Documentation/development.md | 9 +++ UtilCollection.cs | 29 ++++++++- tests/SPADE.Tests/SPADE.Tests.csproj | 25 ++++++++ .../UtilCollectionEqualityTests.cs | 61 +++++++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 tests/SPADE.Tests/SPADE.Tests.csproj create mode 100644 tests/SPADE.Tests/UtilCollectionEqualityTests.cs diff --git a/DiscreteParser.csproj b/DiscreteParser.csproj index a83145c..32a344b 100644 --- a/DiscreteParser.csproj +++ b/DiscreteParser.csproj @@ -9,6 +9,8 @@ Russell Phillips false false + + $(DefaultItemExcludes);tests/** diff --git a/Documentation/development.md b/Documentation/development.md index f936fcb..78728f6 100644 --- a/Documentation/development.md +++ b/Documentation/development.md @@ -1,6 +1,15 @@ # Development Feel free to contribute! Also feel free to open any issues on [github](https://github.com/Jetison333/SPADE) with bugs or feature requests. +## Tests +The tests live in tests/SPADE.Tests and use xUnit. Run them from the repository root with + +```bash +dotnet test tests/SPADE.Tests +``` + +The test project targets .NET 8, so you need the .NET 8 SDK or newer. The library itself still targets .NET 6. + ## antlr If you need to edit the grammar inside of coll.g4 file, use the following commands (you will need to install antlr4): diff --git a/UtilCollection.cs b/UtilCollection.cs index f63d236..ea3ebcb 100644 --- a/UtilCollection.cs +++ b/UtilCollection.cs @@ -273,6 +273,9 @@ public override bool Equals(object? obj) UtilCollection other = (UtilCollection)obj; + // A value, a list and a set are never equal to each other, even when empty. + if (isValue != other.isValue || isOrdered != other.isOrdered) return false; + if (isValue) { return value == other.value; @@ -296,7 +299,31 @@ public bool NotEquals(object? obj) public override int GetHashCode() { - return ToString().GetHashCode(); + if (isValue) + { + return value!.GetHashCode(); + } + + if (isOrdered) + { + HashCode hash = new HashCode(); + hash.Add(isOrdered); + foreach (UtilCollection item in list) + { + hash.Add(item); + } + return hash.ToHashCode(); + } + + // Sets are equal regardless of order, so combine element hashes with + // an order-independent sum rather than hashing ToString(), which + // follows insertion order. + int sum = 0; + foreach (UtilCollection item in set) + { + sum = unchecked(sum + item.GetHashCode()); + } + return HashCode.Combine(isOrdered, set.Count, sum); } public List ToList() diff --git a/tests/SPADE.Tests/SPADE.Tests.csproj b/tests/SPADE.Tests/SPADE.Tests.csproj new file mode 100644 index 0000000..1ff3916 --- /dev/null +++ b/tests/SPADE.Tests/SPADE.Tests.csproj @@ -0,0 +1,25 @@ + + + + net8.0 + enable + enable + false + + Exe + + + + + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + diff --git a/tests/SPADE.Tests/UtilCollectionEqualityTests.cs b/tests/SPADE.Tests/UtilCollectionEqualityTests.cs new file mode 100644 index 0000000..1dc1a52 --- /dev/null +++ b/tests/SPADE.Tests/UtilCollectionEqualityTests.cs @@ -0,0 +1,61 @@ +using SPADE; +using Xunit; + +namespace SPADE.Tests; + +// Equal collections must hash the same, or sets of sets lose lookups (#9). +public class UtilCollectionEqualityTests +{ + private static UtilCollection C(string s) => new UtilCollection(s); + + [Theory] + [InlineData("{1,2}", "{2,1}")] + [InlineData("{1,2,3}", "{3,1,2}")] + [InlineData("{{1,2},{3}}", "{{3},{2,1}}")] + [InlineData("{(1,2),(3,4)}", "{(3,4),(1,2)}")] + [InlineData("({1,2},4)", "({2,1},4)")] + [InlineData("{}", "{}")] + public void EqualCollectionsHaveEqualHashCodes(string a, string b) + { + Assert.Equal(C(a), C(b)); + Assert.Equal(C(a).GetHashCode(), C(b).GetHashCode()); + } + + [Fact] + public void SetOfSetsContainsReorderedMember() + { + Assert.True(C("{{1,2}}").Contains(C("{2,1}"))); + } + + [Fact] + public void SetOfSetsDropsReorderedDuplicate() + { + Assert.Equal(1, C("{{1,2},{2,1}}").Count()); + } + + [Fact] + public void IntersectMatchesReorderedSets() + { + Assert.Equal(1, C("{{1,2}}").Intersect(C("{{2,1}}")).Count()); + } + + [Fact] + public void UnionMergesReorderedSets() + { + Assert.Equal(1, C("{{1,2}}").Union(C("{{2,1}}")).Count()); + } + + [Theory] + [InlineData("(1,2)", "(2,1)")] + [InlineData("{1,2}", "(1,2)")] + [InlineData("{}", "()")] + [InlineData("{}", "x")] + [InlineData("()", "x")] + [InlineData("{1}", "1")] + [InlineData("{1,2}", "{1,2,3}")] + public void DifferentCollectionsAreNotEqual(string a, string b) + { + Assert.NotEqual(C(a), C(b)); + Assert.NotEqual(C(b), C(a)); + } +} From 0f38e6fad2358a4399ea1a442c3c1e08dee910be Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:26:40 -0600 Subject: [PATCH 02/16] fix(grammar): accept set edges in unorderedcross again Since d7e982e, an unorderedcross membership check rejected every normal undirected edge such as {a,b}. The added line checked whether each node inside the edge was a list, and plain nodes never are. The check now looks at the edge itself: a plain value is not an edge, so it fails the constraint instead of crashing while being enumerated. List edges such as (a,b) are still accepted, as in the released 0.1.2, because existing grammars for directed graphs rely on it. Closes #12 --- grammar/desciptors/SetDescriptor.cs | 5 +- tests/SPADE.Tests/UnorderedCrossTests.cs | 69 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 tests/SPADE.Tests/UnorderedCrossTests.cs diff --git a/grammar/desciptors/SetDescriptor.cs b/grammar/desciptors/SetDescriptor.cs index 79432a4..4157cac 100644 --- a/grammar/desciptors/SetDescriptor.cs +++ b/grammar/desciptors/SetDescriptor.cs @@ -84,9 +84,12 @@ public SetDescriptor UnorderedCross(SetDescriptor other) { Func _isMember = (a) => { + // Each member is a collection of nodes, like the edge {a,b}; a plain value is not. + // Lists such as (a,b) are still accepted because existing grammars rely on it (#12). + if (a.IsValue()) return false; + foreach (UtilCollection item in a) { - if (!item.IsOrdered()) return false; if (!isMember(item) && !other.isMember(item)) { return false; diff --git a/tests/SPADE.Tests/UnorderedCrossTests.cs b/tests/SPADE.Tests/UnorderedCrossTests.cs new file mode 100644 index 0000000..fcdab60 --- /dev/null +++ b/tests/SPADE.Tests/UnorderedCrossTests.cs @@ -0,0 +1,69 @@ +using SPADE; +using Xunit; + +namespace SPADE.Tests; + +// Undirected edges must be sets of nodes, such as {a,b} (#12). +public class UnorderedCrossTests +{ + private const string Graph = "{(N,E) | N is set, E subset N unorderedcross N}"; + private const string GraphWithK = "{((N,E),K) | N is set, E subset N unorderedcross N, K is int}"; + + private static StringParser Parse(string grammar, string instance) + { + StringParser parser = new StringParser(grammar); + parser.parse(instance); + return parser; + } + + [Theory] + [InlineData("({a,b,c},{{a,b},{b,c}})")] + [InlineData("({1,2,3},{{2,1},{3,1}})")] + [InlineData("({a,b},{})")] + public void AcceptsSetEdges(string instance) + { + StringParser parser = Parse(Graph, instance); + Assert.Equal(new UtilCollection(instance)[1], parser["E"]); + } + + [Fact] + public void AcceptsGraphWithK() + { + StringParser parser = Parse(GraphWithK, "(({1,2,3},{{1,2},{2,3}}),2)"); + Assert.Equal("2", parser["K"].ToString()); + } + + // Released versions accept list edges, and some grammars for directed + // graphs rely on it, so this keeps that behavior until they move to cross (#12). + [Theory] + [InlineData("({a,b,c},{(a,b)})")] + [InlineData("({a,b,c},{{a,b},(b,c)})")] + public void StillAcceptsListEdges(string instance) + { + StringParser parser = Parse(Graph, instance); + Assert.Equal(new UtilCollection(instance)[1], parser["E"]); + } + + [Fact] + public void RejectsListEdgesWithUnknownNodes() + { + Assert.ThrowsAny(() => Parse(Graph, "({a,b,c},{(a,d)})")); + } + + [Theory] + [InlineData("({a,b,c},{{a,d}})")] + [InlineData("({a,b,c},{{x,y}})")] + public void RejectsEdgesWithUnknownNodes(string instance) + { + Assert.ThrowsAny(() => Parse(Graph, instance)); + } + + [Fact] + public void RejectsPlainValueEdgeAsConstraintFailure() + { + // A plain value is not an edge. It should fail the constraint, not + // crash while trying to enumerate the value. + Exception e = Assert.ThrowsAny(() => Parse(Graph, "({a,b},{a})")); + Assert.IsNotType(e); + } +} From 52b710fce7a77828f65198686295e3f5ec0084e1 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:33:14 -0600 Subject: [PATCH 03/16] fix(grammar): reject list edges in unorderedcross An unorderedcross member is an unordered pair, so it must be a set such as {a,b}. A list such as (a,b) is an ordered pair, which is what cross describes, so it is now rejected here too. This is what d7e982e set out to do. Grammars for directed graphs that use unorderedcross with list edges need to switch to cross. Refs #12 --- grammar/desciptors/SetDescriptor.cs | 6 +++--- tests/SPADE.Tests/UnorderedCrossTests.cs | 13 ++++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/grammar/desciptors/SetDescriptor.cs b/grammar/desciptors/SetDescriptor.cs index 4157cac..f90b869 100644 --- a/grammar/desciptors/SetDescriptor.cs +++ b/grammar/desciptors/SetDescriptor.cs @@ -84,9 +84,9 @@ public SetDescriptor UnorderedCross(SetDescriptor other) { Func _isMember = (a) => { - // Each member is a collection of nodes, like the edge {a,b}; a plain value is not. - // Lists such as (a,b) are still accepted because existing grammars rely on it (#12). - if (a.IsValue()) return false; + // Each member is a set of nodes, like the edge {a,b}. A list such as (a,b) + // is an ordered pair and belongs to cross, and a plain value is not an edge. + if (a.IsValue() || a.IsOrdered()) return false; foreach (UtilCollection item in a) { diff --git a/tests/SPADE.Tests/UnorderedCrossTests.cs b/tests/SPADE.Tests/UnorderedCrossTests.cs index fcdab60..45682d5 100644 --- a/tests/SPADE.Tests/UnorderedCrossTests.cs +++ b/tests/SPADE.Tests/UnorderedCrossTests.cs @@ -33,21 +33,20 @@ public void AcceptsGraphWithK() Assert.Equal("2", parser["K"].ToString()); } - // Released versions accept list edges, and some grammars for directed - // graphs rely on it, so this keeps that behavior until they move to cross (#12). + // A list edge is an ordered pair, which belongs to cross, not unorderedcross. [Theory] [InlineData("({a,b,c},{(a,b)})")] [InlineData("({a,b,c},{{a,b},(b,c)})")] - public void StillAcceptsListEdges(string instance) + public void RejectsListEdges(string instance) { - StringParser parser = Parse(Graph, instance); - Assert.Equal(new UtilCollection(instance)[1], parser["E"]); + Assert.ThrowsAny(() => Parse(Graph, instance)); } [Fact] - public void RejectsListEdgesWithUnknownNodes() + public void CrossAcceptsListEdges() { - Assert.ThrowsAny(() => Parse(Graph, "({a,b,c},{(a,d)})")); + StringParser parser = Parse("{(N,E) | N is set, E subset N cross N}", "({a,b,c},{(a,b),(b,a)})"); + Assert.Equal(2, parser["E"].Count()); } [Theory] From aa5b2e22330eabe4805b76168dbf0b9c0188f722 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:39:54 -0600 Subject: [PATCH 04/16] fix(grammar): make is int, is set and is list check their type "N is int" checked nothing at all: int reaches the constraint as a set descriptor, and the "is" branch only handled the set, list and int keywords. So abc, {1} and (1,2) all passed. It now requires a single value that parses as an integer. "S is set" accepted a plain value, because a value is not ordered. It now requires an unordered collection. "is list" already rejected values and sets; it is now covered by tests. Closes #10 --- grammar/codeGen.cs | 7 ++ grammar/desciptors/ConstraintDescriptor.cs | 6 +- tests/SPADE.Tests/TypeConstraintTests.cs | 87 ++++++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tests/SPADE.Tests/TypeConstraintTests.cs diff --git a/grammar/codeGen.cs b/grammar/codeGen.cs index 83ea6ef..07bcf6b 100644 --- a/grammar/codeGen.cs +++ b/grammar/codeGen.cs @@ -125,6 +125,13 @@ public void ExitConstraint([NotNull] collParser.ConstraintContext context) { constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, context.@object().BUILT_IN().ToString()!)); } + else if (context.@object().BUILT_IN_SET() != null && context.DEFINITION().ToString() == "is") + { + // "N is int" is a type check, like "N is set". The integer set descriptor + // pushed for it is only needed by other definitions, such as "S subset int". + setDescriptorStack.Pop(); + constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, context.@object().BUILT_IN_SET().ToString()!)); + } else { constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, setDescriptorStack.Pop())); diff --git a/grammar/desciptors/ConstraintDescriptor.cs b/grammar/desciptors/ConstraintDescriptor.cs index 70ff95e..2ae1af6 100644 --- a/grammar/desciptors/ConstraintDescriptor.cs +++ b/grammar/desciptors/ConstraintDescriptor.cs @@ -33,7 +33,7 @@ public void check(Dictionary map) switch (built_in) { case "set": - if (map[term].IsOrdered()) + if (map[term].IsValue() || map[term].IsOrdered()) { throw new Exception($"Constraint {term} {definition} {built_in} failed because {term} was not a set"); } @@ -45,9 +45,9 @@ public void check(Dictionary map) } break; case "int": - if (!map[term].IsValue()) + if (!map[term].IsValue() || !int.TryParse(map[term].ToString(), out _)) { - throw new Exception($"Constraint {term} {definition} {built_in} failed because {term} was not a value"); + throw new Exception($"Constraint {term} {definition} {built_in} failed because {map[term]} was not an integer"); } break; } diff --git a/tests/SPADE.Tests/TypeConstraintTests.cs b/tests/SPADE.Tests/TypeConstraintTests.cs new file mode 100644 index 0000000..40194f9 --- /dev/null +++ b/tests/SPADE.Tests/TypeConstraintTests.cs @@ -0,0 +1,87 @@ +using SPADE; +using Xunit; + +namespace SPADE.Tests; + +// "is int", "is set" and "is list" must check the type they name (#10). +public class TypeConstraintTests +{ + private static void Parse(string grammar, string instance) + { + new StringParser(grammar).parse(instance); + } + + [Theory] + [InlineData("3")] + [InlineData("-12")] + [InlineData("0")] + public void IsIntAcceptsIntegers(string instance) + { + Parse("{N | N is int}", instance); + } + + [Theory] + [InlineData("abc")] + [InlineData("1.5")] + [InlineData("{1}")] + [InlineData("(1,2)")] + public void IsIntRejectsNonIntegers(string instance) + { + Assert.ThrowsAny(() => Parse("{N | N is int}", instance)); + } + + [Fact] + public void IsIntChecksNestedFields() + { + string grammar = "{((N,E),K) | N is set, E subset N unorderedcross N, K is int}"; + Parse(grammar, "(({1,2},{{1,2}}),2)"); + Assert.ThrowsAny(() => Parse(grammar, "(({1,2},{{1,2}}),two)")); + } + + [Fact] + public void IsIntChecksInsideSetComprehension() + { + string grammar = "{(N,E) | N is set, E subset {(e, w) | e is N unorderedcross N, w is int}}"; + Parse(grammar, "({1,2,3},{({1,2},4),({2,3},5)})"); + Assert.ThrowsAny(() => Parse(grammar, "({1,2,3},{({1,2},heavy)})")); + } + + [Theory] + [InlineData("{1,2}")] + [InlineData("{}")] + public void IsSetAcceptsSets(string instance) + { + Parse("{S | S is set}", instance); + } + + [Theory] + [InlineData("balanced")] + [InlineData("(1,2)")] + public void IsSetRejectsValuesAndLists(string instance) + { + Assert.ThrowsAny(() => Parse("{S | S is set}", instance)); + } + + [Theory] + [InlineData("(1,2)")] + [InlineData("()")] + public void IsListAcceptsLists(string instance) + { + Parse("{L | L is list}", instance); + } + + [Theory] + [InlineData("balanced")] + [InlineData("{1,2}")] + public void IsListRejectsValuesAndSets(string instance) + { + Assert.ThrowsAny(() => Parse("{L | L is list}", instance)); + } + + [Fact] + public void SubsetIntStillChecksEveryElement() + { + Parse("{S | S subset int}", "{1,2,3}"); + Assert.ThrowsAny(() => Parse("{S | S subset int}", "{1,2,x}")); + } +} From 8ba6e46b6b6cd15f008e1b9679cdf53d2bbfa828 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:00:30 -0600 Subject: [PATCH 05/16] feat(parser): read instance strings in one pass Replaces the substring-based reader in UtilCollection(string) with InstanceScanner, which walks the input once. Whitespace (#2): spaces, tabs and line breaks around { } ( ) , and at either end of the input are skipped, so {2, 3} now holds 3 and a trailing new line no longer breaks an instance. Inside a value, whitespace next to [ ] ; : = is dropped, plain spaces are kept (New York), and a run with a tab or line break becomes one space. These match the rules Redux applies before parsing today. Errors (#4): malformed input throws SpadeParseException with the position, what was expected and what was found, e.g. "expected , or } at position 5, found end of input" for {1,2. Input that used to be misread without an error, such as {1,2 or {1,2}junk, is now rejected. Time and depth (#7): parsing is linear in the input's length, and ToString builds its text in one buffer. Nesting deeper than 256 levels is rejected instead of overflowing the stack and crashing the process. 40,000 elements took 104 seconds before; 100,000 now take well under a second. Closes #2 Closes #7 Refs #4 --- Documentation/InstanceString.md | 29 ++++- InstanceScanner.cs | 152 ++++++++++++++++++++++ SpadeParseException.cs | 32 +++++ UtilCollection.cs | 121 ++++++----------- tests/SPADE.Tests/InstanceScannerTests.cs | 146 +++++++++++++++++++++ 5 files changed, 396 insertions(+), 84 deletions(-) create mode 100644 InstanceScanner.cs create mode 100644 SpadeParseException.cs create mode 100644 tests/SPADE.Tests/InstanceScannerTests.cs diff --git a/Documentation/InstanceString.md b/Documentation/InstanceString.md index 3b529e9..9fbb377 100644 --- a/Documentation/InstanceString.md +++ b/Documentation/InstanceString.md @@ -11,4 +11,31 @@ So a set may look something like {a,b,c}. A list **V**, will be represented usin ``` (e_0,...,e_k) ``` -as such, an example of a list could look something like (1,2,3). Also note that since an element of a set or list may also be a set of a list, so a set of lists can be represented as {(1,2),(2,3)} \ No newline at end of file +as such, an example of a list could look something like (1,2,3). Also note that since an element of a set or list may also be a set of a list, so a set of lists can be represented as {(1,2),(2,3)} + +## Whitespace +Whitespace is ignored where it can't be part of a value, so an instance can be spread over several lines: + +``` +{ + (1, 2), + (2, 3) +} +``` + +reads the same as `{(1,2),(2,3)}`. The rules are: + +- Spaces, tabs and line breaks before or after `{ } ( ) ,`, and at the start and end of the input, are skipped. +- Inside a value, whitespace next to `[ ] ; : =` is dropped, so `x1 : True` reads as `x1:True`. +- Other whitespace inside a value is kept. Plain spaces stay as typed, so `{New York, Los Angeles}` holds `New York`. A run containing a tab or line break becomes a single space, so a value wrapped onto two lines, `New` and `York`, reads as `New York`. + +## Errors +An instance string that can't be read throws `SpadeParseException`, which says where the problem is, what was expected and what was found: + +``` +expected , or } at position 5, found end of input +``` + +for `{1,2`. Its `Position` (counting from 1), `Expected` and `Found` properties hold the same details. Nothing is guessed: unbalanced braces, extra text after the instance, empty elements such as `{1,,2}`, and empty input are all errors. + +Sets and lists can be nested up to 256 levels deep. Deeper input is rejected with a `SpadeParseException` rather than risking a stack overflow. diff --git a/InstanceScanner.cs b/InstanceScanner.cs new file mode 100644 index 0000000..12cdce3 --- /dev/null +++ b/InstanceScanner.cs @@ -0,0 +1,152 @@ +using System.Text; + +namespace SPADE; + +/// +/// Reads an instance string such as ({1,2},{(1,2)}) into a +/// in one pass, so the time taken grows in proportion to the input's length. +/// +/// +/// Whitespace rules (#2): +/// +/// Whitespace before or after { } ( ) , and at either end of the input is skipped. +/// Inside a value, whitespace next to [ ] ; : = is dropped, so x1 : True reads as x1:True. +/// Any other whitespace inside a value is kept. A run of plain spaces stays exactly as typed +/// (New York); a run containing a tab or line break becomes one space, since a line break +/// inside a value is a wrap (New\nYork reads as New York). +/// +/// These match the rules Redux applied before parsing, so that layer can be removed. +/// +internal class InstanceScanner +{ + /// + /// Deepest nesting of sets and lists accepted. Far beyond any real instance; it stops + /// deeply nested input from overflowing the stack and crashing the process (#7). + /// + public const int MaxDepth = 256; + + // Characters that end a value. + private const string Structural = "{}(),"; + + // Characters inside a value that whitespace never belongs next to. + private const string ValueDelimiters = "[];:="; + + private readonly string text; + private int pos; + + private InstanceScanner(string text) + { + this.text = text; + } + + public static UtilCollection Parse(string text) + { + if (text == null) throw new ArgumentNullException(nameof(text)); + return new InstanceScanner(text).ParseInstance(); + } + + private UtilCollection ParseInstance() + { + SkipWhitespace(); + if (AtEnd()) throw Error("a value, { or ("); + UtilCollection result = ParseElement(0); + SkipWhitespace(); + if (!AtEnd()) throw Error("end of input"); + return result; + } + + // Expects pos at the first character of an element, after any whitespace. + private UtilCollection ParseElement(int depth) + { + char c = text[pos]; + if (c == '{' || c == '(') return ParseCollection(depth + 1); + if (Structural.IndexOf(c) >= 0) throw Error("a value, { or ("); + return ParseValue(); + } + + private UtilCollection ParseCollection(int depth) + { + if (depth > MaxDepth) throw Error($"at most {MaxDepth} levels of nesting"); + + char open = text[pos]; + char close = open == '{' ? '}' : ')'; + pos++; + + List items = new(); + SkipWhitespace(); + if (!AtEnd() && text[pos] == close) + { + pos++; + } + else + { + while (true) + { + SkipWhitespace(); + if (AtEnd()) throw Error("a value, { or ("); + items.Add(ParseElement(depth)); + + SkipWhitespace(); + if (AtEnd() || (text[pos] != ',' && text[pos] != close)) throw Error($", or {close}"); + if (text[pos++] == close) break; + } + } + + if (open == '(') return new UtilCollection(items); + return new UtilCollection(items.ToHashSet()); + } + + private UtilCollection ParseValue() + { + int start = pos; + while (!AtEnd() && Structural.IndexOf(text[pos]) < 0) pos++; + return UtilCollection.FromValue(NormalizeValue(start, pos)); + } + + // Applies the whitespace rules to text[start..end). text[start] is never whitespace. + private string NormalizeValue(int start, int end) + { + StringBuilder sb = new(end - start); + int i = start; + while (i < end) + { + if (!char.IsWhiteSpace(text[i])) + { + sb.Append(text[i++]); + continue; + } + + int j = i; + bool plainSpaces = true; + while (j < end && char.IsWhiteSpace(text[j])) + { + if (text[j] != ' ') plainSpaces = false; + j++; + } + + bool trailing = j == end; + bool besideDelimiter = ValueDelimiters.IndexOf(sb[sb.Length - 1]) >= 0 + || (!trailing && ValueDelimiters.IndexOf(text[j]) >= 0); + if (!trailing && !besideDelimiter) + { + if (plainSpaces) sb.Append(text, i, j - i); + else sb.Append(' '); + } + i = j; + } + return sb.ToString(); + } + + private void SkipWhitespace() + { + while (!AtEnd() && char.IsWhiteSpace(text[pos])) pos++; + } + + private bool AtEnd() => pos >= text.Length; + + private SpadeParseException Error(string expected) + { + string found = AtEnd() ? "end of input" : text[pos].ToString(); + return new SpadeParseException(pos + 1, expected, found); + } +} diff --git a/SpadeParseException.cs b/SpadeParseException.cs new file mode 100644 index 0000000..be2ca02 --- /dev/null +++ b/SpadeParseException.cs @@ -0,0 +1,32 @@ +namespace SPADE; + +/// +/// Thrown when an instance string can't be read or doesn't fit its definition string. +/// Every bad-input case throws this type, so callers can tell bad input apart from a bug. +/// +public class SpadeParseException : FormatException +{ + /// Character position in the instance string, counting from 1, when the error has one. + public int? Position { get; } + + /// What the parser expected at , when the error has one. + public string? Expected { get; } + + /// What the parser found instead, when the error has one. + public string? Found { get; } + + /// A syntax error at a position, e.g. "expected , or } at position 14, found x". + public SpadeParseException(int position, string expected, string found) + : base($"expected {expected} at position {position}, found {found}") + { + Position = position; + Expected = expected; + Found = found; + } + + /// An error with no single position, such as a failed constraint. + public SpadeParseException(string message) + : base(message) + { + } +} diff --git a/UtilCollection.cs b/UtilCollection.cs index ea3ebcb..6a70fe7 100644 --- a/UtilCollection.cs +++ b/UtilCollection.cs @@ -1,5 +1,6 @@ using System.Collections; using System.Runtime.Serialization; +using System.Text; namespace SPADE; @@ -20,39 +21,26 @@ public UtilCollection() } + /// + /// Reads an instance string such as ({1,2},{(1,2)}). Throws + /// if it isn't a well-formed value, set or list. + /// See for how whitespace is handled. + /// public UtilCollection(string instance) { - if (instance[0] == '{') - { - isOrdered = false; - isValue = false; - instance = instance.Substring(1, instance.Length - 2); - while (instance != "") - { - string item = findMatchingBrace(instance); - set.Add(new UtilCollection(item)); - instance = instance.Substring(item.Length).TrimStart(','); - } - } - else if (instance[0] == '(') - { - isOrdered = true; - isValue = false; - instance = instance.Substring(1, instance.Length - 2); - while (instance != "") - { - string item = findMatchingBrace(instance); - list.Add(new UtilCollection(item)); - instance = instance.Substring(item.Length).TrimStart(','); - } - } - else - { - isValue = true; - value = instance; - } + UtilCollection parsed = InstanceScanner.Parse(instance); + set = parsed.set; + list = parsed.list; + value = parsed.value; + isOrdered = parsed.isOrdered; + isValue = parsed.isValue; } - + + internal static UtilCollection FromValue(string value) + { + return new UtilCollection { value = value, isValue = true }; + } + public UtilCollection(HashSet set_) { isOrdered = false; @@ -85,43 +73,6 @@ public UtilCollection(HashSet set__) isOrdered = false; } - private string findMatchingBrace(string instance) - { - Dictionary braces = new Dictionary - { - { '(', ')' }, - { '{', '}' }, - }; - - if (!braces.ContainsKey(instance[0])) - { - return instance.Split(',')[0]; - } - - Stack stack = new Stack(); - - for (int i = 0; i < instance.Length; i++) - { - if (braces.ContainsKey(instance[i])) - { - stack.Push(instance[i]); - } - else if (braces.Values.Contains(instance[i])) - { - char openBracket = stack.Pop(); - if (braces[openBracket] != instance[i]) - { - throw new Exception("Braces not matched"); - } - if (stack.Count() == 0) - { - return instance.Substring(0, i + 1); - } - } - } - throw new Exception("Braces not matched"); - } - public UtilCollection this[int index] { get { @@ -344,25 +295,29 @@ override public string ToString() { return value!; } - - string str = ""; - if (isOrdered) - str += "("; - else - str += "{"; - foreach (var item in this) + StringBuilder sb = new StringBuilder(); + AppendTo(sb); + return sb.ToString(); + } + + // Builds the text in one buffer; adding strings with += copied it once per element. + private void AppendTo(StringBuilder sb) + { + if (isValue) { - //Console.WriteLine(item.ToString()); - str += item.ToString() + ","; + sb.Append(value); + return; } - str = str.TrimEnd(','); - if (isOrdered) - str += ")"; - else - str += "}"; - - return str; + sb.Append(isOrdered ? '(' : '{'); + bool first = true; + foreach (UtilCollection item in ToList()) + { + if (!first) sb.Append(','); + item.AppendTo(sb); + first = false; + } + sb.Append(isOrdered ? ')' : '}'); } } diff --git a/tests/SPADE.Tests/InstanceScannerTests.cs b/tests/SPADE.Tests/InstanceScannerTests.cs new file mode 100644 index 0000000..4954f24 --- /dev/null +++ b/tests/SPADE.Tests/InstanceScannerTests.cs @@ -0,0 +1,146 @@ +using System.Diagnostics; +using SPADE; +using Xunit; + +namespace SPADE.Tests; + +// Reading instance strings: whitespace (#2), clear errors (#4), and bounded time and depth (#7). +public class InstanceScannerTests +{ + private static UtilCollection C(string s) => new UtilCollection(s); + + // --- Whitespace (#2) --- + + [Theory] + [InlineData("{2, 3}", "{2,3}")] + [InlineData("{1,2}\n", "{1,2}")] + [InlineData(" \t{ 1 ,\r\n 2 }\r\n", "{1,2}")] + [InlineData("( {1, 2} , 4 )", "({1,2},4)")] + [InlineData("{\n (a, b),\n (c, d)\n}", "{(a,b),(c,d)}")] + [InlineData(" 3 ", "3")] + public void SkipsWhitespaceAroundStructure(string input, string expected) + { + Assert.Equal(C(expected), C(input)); + Assert.Equal(expected, C(input).ToString()); + } + + [Fact] + public void SpacedSetContainsItsValues() + { + Assert.True(C("{2, 3}").Contains(C("3"))); + } + + [Theory] + [InlineData("{New York, Los Angeles}", "New York")] + [InlineData("{x1 | !x2, x3}", "x1 | !x2")] + [InlineData("{a b}", "a b")] + public void KeepsSpacesInsideValues(string input, string value) + { + Assert.True(C(input).Contains(C(value))); + Assert.Equal(value, C(input).ToList().First(v => v.ToString().Contains(' ')).ToString()); + } + + [Theory] + [InlineData("{New\nYork}", "New York")] + [InlineData("{New \r\n York}", "New York")] + [InlineData("{a\tb}", "a b")] + public void LineBreaksAndTabsInsideValuesBecomeOneSpace(string input, string value) + { + Assert.Equal("{" + value + "}", C(input).ToString()); + } + + [Theory] + [InlineData("{x1 : True}", "{x1:True}")] + [InlineData("{a = 1, b= 2}", "{a=1,b=2}")] + [InlineData("(1 ; 2 ; 3)", "(1;2;3)")] + [InlineData("{[ 1 ]}", "{[1]}")] + public void DropsWhitespaceBesideDelimitersInsideValues(string input, string expected) + { + Assert.Equal(expected, C(input).ToString()); + } + + // --- Errors (#4) --- + + [Theory] + [InlineData("", "expected a value, { or ( at position 1, found end of input")] + [InlineData(" ", "expected a value, { or ( at position 4, found end of input")] + [InlineData("{1,2", "expected , or } at position 5, found end of input")] + [InlineData("{1,2}junk", "expected end of input at position 6, found j")] + [InlineData("{1,(2))}", "expected , or } at position 7, found )")] + [InlineData("{(1,2},3)", "expected , or ) at position 6, found }")] + [InlineData("}", "expected a value, { or ( at position 1, found }")] + [InlineData("{1,,2}", "expected a value, { or ( at position 4, found ,")] + [InlineData("{1,}", "expected a value, { or ( at position 4, found }")] + [InlineData("{,1}", "expected a value, { or ( at position 2, found ,")] + [InlineData("f(x)", "expected end of input at position 2, found (")] + [InlineData("{1, 2} x", "expected end of input at position 8, found x")] + public void MalformedInputThrowsWithPosition(string input, string message) + { + SpadeParseException e = Assert.Throws(() => C(input)); + Assert.Equal(message, e.Message); + } + + [Fact] + public void ErrorExposesPositionExpectedAndFound() + { + SpadeParseException e = Assert.Throws(() => C("{1,2}junk")); + Assert.Equal(6, e.Position); + Assert.Equal("end of input", e.Expected); + Assert.Equal("j", e.Found); + } + + [Fact] + public void ParseErrorIsAFormatException() + { + Assert.IsAssignableFrom(Assert.Throws(() => C("{"))); + } + + // --- Depth and time (#7) --- + + // InstanceScanner.MaxDepth, which is internal. + private const int MaxDepth = 256; + + private static string Nested(int depth) => new string('{', depth) + new string('}', depth); + + [Fact] + public void AcceptsNestingUpToTheLimit() + { + C(Nested(MaxDepth)); + } + + [Theory] + [InlineData(MaxDepth + 1)] + [InlineData(100_000)] + public void RejectsDeeperNestingWithoutCrashing(int depth) + { + SpadeParseException e = Assert.Throws(() => C(Nested(depth))); + Assert.Equal(MaxDepth + 1, e.Position); + } + + [Fact] + public void LargeFlatSetParsesQuickly() + { + // 40,000 elements took 104 seconds before; this is 100,000. + string input = "{" + string.Join(", ", Enumerable.Range(0, 100_000)) + "}"; + Stopwatch timer = Stopwatch.StartNew(); + UtilCollection set = C(input); + string text = set.ToString(); + timer.Stop(); + + Assert.Equal(100_000, set.Count()); + Assert.Equal(input.Replace(" ", "").Length, text.Length); + Assert.True(timer.Elapsed < TimeSpan.FromSeconds(5), $"took {timer.Elapsed}"); + } + + [Fact] + public void DeepNestingParsesQuickly() + { + // Depth 4,000 took 103 seconds before; the limit is lower, so check many limit-depth parses. + string input = Nested(MaxDepth); + Stopwatch timer = Stopwatch.StartNew(); + for (int i = 0; i < 100; i++) C(input); + timer.Stop(); + + Assert.True(timer.Elapsed < TimeSpan.FromSeconds(5), $"took {timer.Elapsed}"); + } +} From 3935e696e8470d9bf5bd22ff853bb5b63a989e58 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:00:31 -0600 Subject: [PATCH 06/16] fix(grammar): report shape and constraint failures as SpadeParseException Input that is well formed but doesn't fit the definition string threw whatever happened to fail: InvalidOperationException ("Cannot index into a set"), ArgumentOutOfRangeException, or a bare Exception whose message said PLACEHOLDER. All of these now throw SpadeParseException with a message that names what failed: - A format such as (N,E) checks that it got a list of exactly that many elements. A longer list used to be accepted and the extra elements dropped. - Constraint failures quote the constraint as written, e.g. "Constraint E subset N unorderedcross N failed: {1,9} is not a member". Long values are shortened. - subset on a plain value, and cross on anything that isn't a pair, fail the constraint instead of crashing. Closes #4 --- Documentation/example.md | 19 ++++++ grammar/codeGen.cs | 14 ++-- grammar/desciptors/ConstraintDescriptor.cs | 33 ++++++++-- grammar/desciptors/SetDescriptor.cs | 3 +- grammar/desciptors/formatDescriptor.cs | 6 +- tests/SPADE.Tests/DefinitionErrorTests.cs | 76 ++++++++++++++++++++++ tests/SPADE.Tests/TypeConstraintTests.cs | 12 ++-- tests/SPADE.Tests/UnorderedCrossTests.cs | 6 +- 8 files changed, 148 insertions(+), 21 deletions(-) create mode 100644 tests/SPADE.Tests/DefinitionErrorTests.cs diff --git a/Documentation/example.md b/Documentation/example.md index df23deb..2b7dbe7 100644 --- a/Documentation/example.md +++ b/Documentation/example.md @@ -28,3 +28,22 @@ foreach (UtilCollection c in graph["E"]) ``` Above is example code for parsing a graph type problem. The StringParser class is provided with a [definition string](DefinitionString.md), and then with a [instance string](InstanceString.md). It then parses and verifies the instance string. Now, you can access items of that problem by using a key access, which returns a [UtilCollection](utilcollection.md) class, which can then be further used however you need to use it. + +## Handling bad input + +Every problem with an instance string throws `SpadeParseException`: text that can't be read, a shape that doesn't match the definition string, or a failed constraint. Catch it to report bad input, for example as a 400 response: + +```c# +try +{ + graph.parse(instanceString); +} +catch (SpadeParseException e) +{ + // e.g. "expected , or } at position 5, found end of input" + // or "Constraint E subset N cross N failed: (A,X) is not a member" + Console.WriteLine(e.Message); +} +``` + +Any other exception type means a bug in SPADE or in the definition string, not bad input. diff --git a/grammar/codeGen.cs b/grammar/codeGen.cs index 07bcf6b..9aaa8f0 100644 --- a/grammar/codeGen.cs +++ b/grammar/codeGen.cs @@ -35,6 +35,12 @@ public void Parse(UtilCollection utilCollection) setDescriptor!.isMember(utilCollection); } + // A rule's text as written in the definition string, spacing included, for error messages. + private static string SourceText(ParserRuleContext context) + { + return context.Start.InputStream.GetText(Interval.Of(context.Start.StartIndex, context.Stop.StopIndex)); + } + /// /// Enter a parse tree produced by . /// @@ -91,7 +97,7 @@ public void ExitFormat([NotNull] collParser.FormatContext context) lst.Add(formatStack.Pop()); } lst.Reverse(); // have to reverse the list, since the leftmost should be the one that was popped first - formatStack.Push(new FormatDescriptor(lst)); + formatStack.Push(new FormatDescriptor(lst, SourceText(context))); } if (context.TERM() != null) { @@ -123,18 +129,18 @@ public void ExitConstraint([NotNull] collParser.ConstraintContext context) { if (context.@object().BUILT_IN() != null) { - constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, context.@object().BUILT_IN().ToString()!)); + constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, context.@object().BUILT_IN().ToString()!, SourceText(context))); } else if (context.@object().BUILT_IN_SET() != null && context.DEFINITION().ToString() == "is") { // "N is int" is a type check, like "N is set". The integer set descriptor // pushed for it is only needed by other definitions, such as "S subset int". setDescriptorStack.Pop(); - constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, context.@object().BUILT_IN_SET().ToString()!)); + constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, context.@object().BUILT_IN_SET().ToString()!, SourceText(context))); } else { - constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, setDescriptorStack.Pop())); + constraintDescriptorStack.Push(new ConstraintDescriptor(context.TERM().ToString()!, context.DEFINITION().ToString()!, setDescriptorStack.Pop(), SourceText(context))); } } /// diff --git a/grammar/desciptors/ConstraintDescriptor.cs b/grammar/desciptors/ConstraintDescriptor.cs index 2ae1af6..1cbd7c8 100644 --- a/grammar/desciptors/ConstraintDescriptor.cs +++ b/grammar/desciptors/ConstraintDescriptor.cs @@ -8,19 +8,36 @@ class ConstraintDescriptor String definition; SetDescriptor? set; String? built_in; + // The constraint as written in the definition string, e.g. "E subset N unorderedcross N". + String text; - public ConstraintDescriptor(String _term, String _definition, SetDescriptor _set) + public ConstraintDescriptor(String _term, String _definition, SetDescriptor _set, String _text) { term = _term; definition = _definition; set = _set; + text = _text; } - public ConstraintDescriptor(String _term, String _definition, String _built_in) + public ConstraintDescriptor(String _term, String _definition, String _built_in, String _text) { term = _term; definition = _definition; built_in = _built_in; + text = _text; + } + + // Values in error messages are shortened, so a huge instance doesn't make a huge message. + public static string Describe(UtilCollection collection) + { + const int maxLength = 60; + string s = collection.ToString(); + return s.Length <= maxLength ? s : s.Substring(0, maxLength) + "..."; + } + + private SpadeParseException Failure(string reason) + { + return new SpadeParseException($"Constraint {text} failed: {reason}"); } @@ -35,19 +52,19 @@ public void check(Dictionary map) case "set": if (map[term].IsValue() || map[term].IsOrdered()) { - throw new Exception($"Constraint {term} {definition} {built_in} failed because {term} was not a set"); + throw Failure($"{Describe(map[term])} is not a set"); } break; case "list": if (map[term].IsUnordered()) { - throw new Exception($"Constraint {term} {definition} {built_in} failed because {term} was not a list"); + throw Failure($"{Describe(map[term])} is not a list"); } break; case "int": if (!map[term].IsValue() || !int.TryParse(map[term].ToString(), out _)) { - throw new Exception($"Constraint {term} {definition} {built_in} failed because {map[term]} was not an integer"); + throw Failure($"{Describe(map[term])} is not an integer"); } break; } @@ -56,11 +73,15 @@ public void check(Dictionary map) case "subset": if (set != null) { + if (map[term].IsValue()) + { + throw Failure($"{Describe(map[term])} is not a set"); + } foreach (UtilCollection item in map[term]) { if (!set.isMember(item)) { - throw new Exception($"Constraint {term} {definition} PLACEHOLDER failed because {item} was not a subset of PLACEHOLDER"); + throw Failure($"{Describe(item)} is not a member"); } } } diff --git a/grammar/desciptors/SetDescriptor.cs b/grammar/desciptors/SetDescriptor.cs index f90b869..282cb65 100644 --- a/grammar/desciptors/SetDescriptor.cs +++ b/grammar/desciptors/SetDescriptor.cs @@ -75,7 +75,8 @@ public SetDescriptor(Dictionary _map, FormatDescriptor f public SetDescriptor Cross(SetDescriptor other) { - Func _isMember = (a) => isMember(a[0]) && other.isMember(a[1]); + Func _isMember = (a) => + a.IsOrdered() && a.Count() == 2 && isMember(a[0]) && other.isMember(a[1]); Func _genSet = () => genSet().Cross(other.genSet()); return new SetDescriptor(map, _isMember, _genSet); } diff --git a/grammar/desciptors/formatDescriptor.cs b/grammar/desciptors/formatDescriptor.cs index d86f1ab..bdce081 100644 --- a/grammar/desciptors/formatDescriptor.cs +++ b/grammar/desciptors/formatDescriptor.cs @@ -13,10 +13,14 @@ public FormatDescriptor(String term) parseFunc = (map, coll) => {map[term] = coll;}; } - public FormatDescriptor(List lst) + public FormatDescriptor(List lst, String text) { parseFunc = (map, coll) => { + if (!coll.IsOrdered() || coll.Count() != lst.Count()) + { + throw new SpadeParseException($"expected a list of {lst.Count()} elements for {text}, found {ConstraintDescriptor.Describe(coll)}"); + } for (int i = 0; i < lst.Count(); i++) { lst[i].parseFunc(map, coll[i]); diff --git a/tests/SPADE.Tests/DefinitionErrorTests.cs b/tests/SPADE.Tests/DefinitionErrorTests.cs new file mode 100644 index 0000000..05f3ab9 --- /dev/null +++ b/tests/SPADE.Tests/DefinitionErrorTests.cs @@ -0,0 +1,76 @@ +using SPADE; +using Xunit; + +namespace SPADE.Tests; + +// Input that is well formed but doesn't fit the definition string throws SpadeParseException +// with a message naming what failed (#4). +public class DefinitionErrorTests +{ + private const string Graph = "{(N,E) | N is set, E subset N unorderedcross N}"; + + private static SpadeParseException Fails(string grammar, string instance) + { + return Assert.Throws(() => new StringParser(grammar).parse(instance)); + } + + [Theory] + [InlineData("({1,2},{{1,2}},extra)", "expected a list of 2 elements for (N,E), found ({1,2},{{1,2}},extra)")] + [InlineData("({1,2})", "expected a list of 2 elements for (N,E), found ({1,2})")] + [InlineData("{{1,2},{{1,2}}}", "expected a list of 2 elements for (N,E), found ")] + [InlineData("graph", "expected a list of 2 elements for (N,E), found graph")] + public void WrongShapeNamesTheFormat(string instance, string messageStart) + { + Assert.StartsWith(messageStart, Fails(Graph, instance).Message); + } + + [Fact] + public void FailedSubsetNamesConstraintAndMember() + { + Assert.Equal( + "Constraint E subset N unorderedcross N failed: {1,9} is not a member", + Fails(Graph, "({1,2},{{1,9}})").Message); + } + + [Fact] + public void SubsetOfAValueFailsCleanly() + { + Assert.Equal( + "Constraint E subset N unorderedcross N failed: edges is not a set", + Fails(Graph, "({1,2},edges)").Message); + } + + [Theory] + [InlineData("{N | N is int}", "abc", "Constraint N is int failed: abc is not an integer")] + [InlineData("{S | S is set}", "word", "Constraint S is set failed: word is not a set")] + [InlineData("{L | L is list}", "{1}", "Constraint L is list failed: {1} is not a list")] + public void FailedTypeCheckNamesConstraint(string grammar, string instance, string message) + { + Assert.Equal(message, Fails(grammar, instance).Message); + } + + [Fact] + public void LongValuesAreShortenedInMessages() + { + string big = "{" + string.Join(",", Enumerable.Range(0, 1000)) + "}"; + string message = Fails("{L | L is list}", big).Message; + Assert.True(message.Length < 150, message); + Assert.Contains("...", message); + } + + [Theory] + [InlineData("({a,b},{{a,b}})")] + [InlineData("({a,b},{(a)})")] + [InlineData("({a,b},{(a,b,a)})")] + [InlineData("({a,b},{x})")] + public void CrossRejectsNonPairsCleanly(string instance) + { + Fails("{(N,E) | N is set, E subset N cross N}", instance); + } + + [Fact] + public void SyntaxErrorsComeThroughStringParser() + { + Assert.Equal("expected a value, { or ( at position 9, found end of input", Fails(Graph, "({1,2},{").Message); + } +} diff --git a/tests/SPADE.Tests/TypeConstraintTests.cs b/tests/SPADE.Tests/TypeConstraintTests.cs index 40194f9..f8857e0 100644 --- a/tests/SPADE.Tests/TypeConstraintTests.cs +++ b/tests/SPADE.Tests/TypeConstraintTests.cs @@ -27,7 +27,7 @@ public void IsIntAcceptsIntegers(string instance) [InlineData("(1,2)")] public void IsIntRejectsNonIntegers(string instance) { - Assert.ThrowsAny(() => Parse("{N | N is int}", instance)); + Assert.Throws(() => Parse("{N | N is int}", instance)); } [Fact] @@ -35,7 +35,7 @@ public void IsIntChecksNestedFields() { string grammar = "{((N,E),K) | N is set, E subset N unorderedcross N, K is int}"; Parse(grammar, "(({1,2},{{1,2}}),2)"); - Assert.ThrowsAny(() => Parse(grammar, "(({1,2},{{1,2}}),two)")); + Assert.Throws(() => Parse(grammar, "(({1,2},{{1,2}}),two)")); } [Fact] @@ -43,7 +43,7 @@ public void IsIntChecksInsideSetComprehension() { string grammar = "{(N,E) | N is set, E subset {(e, w) | e is N unorderedcross N, w is int}}"; Parse(grammar, "({1,2,3},{({1,2},4),({2,3},5)})"); - Assert.ThrowsAny(() => Parse(grammar, "({1,2,3},{({1,2},heavy)})")); + Assert.Throws(() => Parse(grammar, "({1,2,3},{({1,2},heavy)})")); } [Theory] @@ -59,7 +59,7 @@ public void IsSetAcceptsSets(string instance) [InlineData("(1,2)")] public void IsSetRejectsValuesAndLists(string instance) { - Assert.ThrowsAny(() => Parse("{S | S is set}", instance)); + Assert.Throws(() => Parse("{S | S is set}", instance)); } [Theory] @@ -75,13 +75,13 @@ public void IsListAcceptsLists(string instance) [InlineData("{1,2}")] public void IsListRejectsValuesAndSets(string instance) { - Assert.ThrowsAny(() => Parse("{L | L is list}", instance)); + Assert.Throws(() => Parse("{L | L is list}", instance)); } [Fact] public void SubsetIntStillChecksEveryElement() { Parse("{S | S subset int}", "{1,2,3}"); - Assert.ThrowsAny(() => Parse("{S | S subset int}", "{1,2,x}")); + Assert.Throws(() => Parse("{S | S subset int}", "{1,2,x}")); } } diff --git a/tests/SPADE.Tests/UnorderedCrossTests.cs b/tests/SPADE.Tests/UnorderedCrossTests.cs index 45682d5..03fcfec 100644 --- a/tests/SPADE.Tests/UnorderedCrossTests.cs +++ b/tests/SPADE.Tests/UnorderedCrossTests.cs @@ -39,7 +39,7 @@ public void AcceptsGraphWithK() [InlineData("({a,b,c},{{a,b},(b,c)})")] public void RejectsListEdges(string instance) { - Assert.ThrowsAny(() => Parse(Graph, instance)); + Assert.Throws(() => Parse(Graph, instance)); } [Fact] @@ -54,7 +54,7 @@ public void CrossAcceptsListEdges() [InlineData("({a,b,c},{{x,y}})")] public void RejectsEdgesWithUnknownNodes(string instance) { - Assert.ThrowsAny(() => Parse(Graph, instance)); + Assert.Throws(() => Parse(Graph, instance)); } [Fact] @@ -62,7 +62,7 @@ public void RejectsPlainValueEdgeAsConstraintFailure() { // A plain value is not an edge. It should fail the constraint, not // crash while trying to enumerate the value. - Exception e = Assert.ThrowsAny(() => Parse(Graph, "({a,b},{a})")); + Exception e = Assert.Throws(() => Parse(Graph, "({a,b},{a})")); Assert.IsNotType(e); } } From ac55d26f76870d43e2ba94d89c9771d36b0dd884 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:17:20 -0600 Subject: [PATCH 07/16] ci: run the Redux Build System (rbs) report, soft for now rbs runs audit, format-check, lint (dotnet build) and unit tests with a coverage floor inside the repo's devcontainer, the same way it does for Redux. It reports without failing (soft) while ci.yml stays the gate; publish.yml keeps doing the NuGet release, since rbs has no NuGet push. - SPADE.slnx: the dotnet engine needs a solution. It names only DiscreteParser.csproj for now; the test project joins it with the first fix PR. ci.yml and publish.yml name DiscreteParser.csproj explicitly, so the MSB1011 a bare `dotnet build` now gives does not affect them. - rbs.toml: coverage floor 70; the goal is 80%+. - .devcontainer: SDK 10 (reads .slnx, builds the net6.0 library) plus the .NET 8 runtime the tests run on. - .gitignore: rbs's .rbs/ work directory and report.md. --- .devcontainer/devcontainer.json | 29 +++++++++++++++++++++++++++++ .github/workflows/rbs.yml | 23 +++++++++++++++++++++++ .gitignore | 2 ++ SPADE.slnx | 3 +++ rbs.toml | 9 +++++++++ 5 files changed, 66 insertions(+) create mode 100644 .devcontainer/devcontainer.json create mode 100644 .github/workflows/rbs.yml create mode 100644 SPADE.slnx create mode 100644 rbs.toml diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..ca172ea --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,29 @@ +{ + "name": "SPADE .NET Dev Container", + // SDK 10 reads SPADE.slnx (the .slnx format needs SDK 9.0.200+) and still builds the net6.0 library. + "image": "mcr.microsoft.com/dotnet/sdk:10.0", + "features": { + "ghcr.io/devcontainers/features/common-utils:2": { + "username": "vscode", + "installZsh": false, + "upgradePackages": false + }, + // The tests target net8.0, so they need the .NET 8 runtime next to SDK 10. + "ghcr.io/devcontainers/features/dotnet:2": { + "version": "none", + "dotnetRuntimeVersions": "8.0" + }, + "ghcr.io/devcontainers/features/docker-outside-of-docker:1": { "moby": false }, + "ghcr.io/reduxisu/features/rbs:1": {} + }, + "remoteUser": "vscode", + "customizations": { + "vscode": { + "extensions": [ + "ms-dotnettools.csharp" + ] + } + }, + "postStartCommand": "if [ \"$(stat -c %U /workspaces/SPADE)\" != \"vscode\" ]; then sudo chown -R vscode:vscode /workspaces/SPADE; fi", + "postCreateCommand": "dotnet --list-sdks && dotnet --list-runtimes" +} diff --git a/.github/workflows/rbs.yml b/.github/workflows/rbs.yml new file mode 100644 index 0000000..b4b1899 --- /dev/null +++ b/.github/workflows/rbs.yml @@ -0,0 +1,23 @@ +name: rbs + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + pull-requests: write + packages: write + +concurrency: + group: rbs-${{ github.ref }} + cancel-in-progress: true + +jobs: + build-test: + uses: ReduxISU/Redux_Build_System/.github/workflows/ci.yml@main + secrets: inherit + with: + # Report only while ci.yml is still the gate. + soft: true diff --git a/.gitignore b/.gitignore index 75ef810..c5be391 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ bin/ obj/ DF.sln +.rbs/ +report.md diff --git a/SPADE.slnx b/SPADE.slnx new file mode 100644 index 0000000..b5c1ef7 --- /dev/null +++ b/SPADE.slnx @@ -0,0 +1,3 @@ + + + diff --git a/rbs.toml b/rbs.toml new file mode 100644 index 0000000..cfbb4f0 --- /dev/null +++ b/rbs.toml @@ -0,0 +1,9 @@ +# Redux Build System pipeline config. See github.com/ReduxISU/Redux_Build_System. +# SPADE is a NuGet library with no image, so it has no [artifact] / [integration] / [push]; +# publish.yml still does the NuGet release. +engine = "dotnet" +solution = "SPADE.slnx" + +[unit-test] +# The standard we're holding to. Floor 70%; the goal is 80%+. New code needs tests to keep it above the floor. +coverage-min = 70 From 4d63d9a1203eeaec223547fd6fe3273a9c026868 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:18:14 -0600 Subject: [PATCH 08/16] style: run dotnet format Whitespace only: dotnet format SPADE.slnx, so rbs's format-check starts clean. Brace placement, indentation and blank lines; no code changes. --- StringParser.cs | 7 +++-- UtilCollection.cs | 36 ++++++++++++---------- grammar/BuiltInSet.cs | 3 +- grammar/codeGen.cs | 9 +++--- grammar/desciptors/ConstraintDescriptor.cs | 22 ++++++------- grammar/desciptors/SetDescriptor.cs | 14 ++++----- grammar/desciptors/formatDescriptor.cs | 4 +-- 7 files changed, 50 insertions(+), 45 deletions(-) diff --git a/StringParser.cs b/StringParser.cs index 23fb6a1..ead7bc7 100644 --- a/StringParser.cs +++ b/StringParser.cs @@ -7,11 +7,12 @@ using SPADE.Grammar; namespace SPADE; + public class StringParser { readonly string format; public Dictionary map = new(); - + private bool hasParsed; public StringParser(string f) @@ -24,7 +25,7 @@ public void parse(string instance) { hasParsed = true; AntlrInputStream stream = new AntlrInputStream(format); - collLexer lex = new collLexer(stream); + collLexer lex = new collLexer(stream); CommonTokenStream tokens = new CommonTokenStream(lex); collParser parser = new collParser(tokens); @@ -35,7 +36,7 @@ public void parse(string instance) gen.Parse(new UtilCollection(instance)); - map = gen.map; + map = gen.map; } diff --git a/UtilCollection.cs b/UtilCollection.cs index f63d236..42e9a4c 100644 --- a/UtilCollection.cs +++ b/UtilCollection.cs @@ -52,14 +52,14 @@ public UtilCollection(string instance) value = instance; } } - + public UtilCollection(HashSet set_) { isOrdered = false; set = new(); foreach (string i in set_) { - set.Add(new UtilCollection(i)); + set.Add(new UtilCollection(i)); } } @@ -69,7 +69,7 @@ public UtilCollection(HashSet> set_) set = new(); foreach (HashSet i in set_) { - set.Add(new UtilCollection(i)); + set.Add(new UtilCollection(i)); } } @@ -120,23 +120,25 @@ private string findMatchingBrace(string instance) } } throw new Exception("Braces not matched"); - } + } public UtilCollection this[int index] { - get { + get + { if (!isOrdered) throw new InvalidOperationException("Cannot index into a set"); - return list[index]; - } - set { + return list[index]; + } + set + { if (!isOrdered) throw new InvalidOperationException("Cannot index into a set"); - list[index] = value; - } + list[index] = value; + } } public void Add(UtilCollection item) { - if(isOrdered) + if (isOrdered) { list.Add(item); } @@ -182,7 +184,7 @@ public void assertOrdered() public void assertUnordered() { - if(isOrdered) throw new InvalidOperationException("Ordered"); + if (isOrdered) throw new InvalidOperationException("Ordered"); } public bool IsUnordered() @@ -239,7 +241,7 @@ public UtilCollection Intersect(UtilCollection other) throw new NotImplementedException(); } } - + public UtilCollection Union(UtilCollection other) { if (isOrdered != other.isOrdered) throw new InvalidOperationException("Can't take the union of a set and a list"); @@ -258,9 +260,9 @@ public UtilCollection Cross(UtilCollection other) UtilCollection ret = new UtilCollection(); foreach (UtilCollection a in this) { - foreach(UtilCollection b in other) + foreach (UtilCollection b in other) { - UtilCollection add = new UtilCollection(new List{a,b}); + UtilCollection add = new UtilCollection(new List { a, b }); ret.Add(add); } } @@ -317,14 +319,14 @@ override public string ToString() { return value!; } - + string str = ""; if (isOrdered) str += "("; else str += "{"; - foreach (var item in this) + foreach (var item in this) { //Console.WriteLine(item.ToString()); str += item.ToString() + ","; diff --git a/grammar/BuiltInSet.cs b/grammar/BuiltInSet.cs index 948aba7..3c48b97 100644 --- a/grammar/BuiltInSet.cs +++ b/grammar/BuiltInSet.cs @@ -1,2 +1,3 @@ namespace SPADE.Grammar.Enums; -enum BuiltInSet {integers} \ No newline at end of file + +enum BuiltInSet { integers } \ No newline at end of file diff --git a/grammar/codeGen.cs b/grammar/codeGen.cs index 83ea6ef..5be12d7 100644 --- a/grammar/codeGen.cs +++ b/grammar/codeGen.cs @@ -10,18 +10,19 @@ namespace SPADE.Grammar; -class codeGen : IcollListener { +class codeGen : IcollListener +{ public Dictionary map; public Stack formatStack; - public SetDescriptor ?setDescriptor; + public SetDescriptor? setDescriptor; public Stack setDescriptorStack; public Stack constraintDescriptorStack; public Stack> constraintDescriptorStackStack; - public codeGen() : base() + public codeGen() : base() { map = new(); formatStack = new(); @@ -88,7 +89,7 @@ public void ExitFormat([NotNull] collParser.FormatContext context) List lst = new(); for (int i = 0; i < context.format().Length; i++) { - lst.Add(formatStack.Pop()); + lst.Add(formatStack.Pop()); } lst.Reverse(); // have to reverse the list, since the leftmost should be the one that was popped first formatStack.Push(new FormatDescriptor(lst)); diff --git a/grammar/desciptors/ConstraintDescriptor.cs b/grammar/desciptors/ConstraintDescriptor.cs index 70ff95e..e673dfa 100644 --- a/grammar/desciptors/ConstraintDescriptor.cs +++ b/grammar/desciptors/ConstraintDescriptor.cs @@ -11,16 +11,16 @@ class ConstraintDescriptor public ConstraintDescriptor(String _term, String _definition, SetDescriptor _set) { - term = _term; - definition = _definition; - set = _set; + term = _term; + definition = _definition; + set = _set; } public ConstraintDescriptor(String _term, String _definition, String _built_in) { - term = _term; - definition = _definition; - built_in = _built_in; + term = _term; + definition = _definition; + built_in = _built_in; } @@ -56,13 +56,13 @@ public void check(Dictionary map) case "subset": if (set != null) { - foreach (UtilCollection item in map[term]) + foreach (UtilCollection item in map[term]) + { + if (!set.isMember(item)) { - if (!set.isMember(item)) - { - throw new Exception($"Constraint {term} {definition} PLACEHOLDER failed because {item} was not a subset of PLACEHOLDER"); - } + throw new Exception($"Constraint {term} {definition} PLACEHOLDER failed because {item} was not a subset of PLACEHOLDER"); } + } } break; } diff --git a/grammar/desciptors/SetDescriptor.cs b/grammar/desciptors/SetDescriptor.cs index 79432a4..1df471e 100644 --- a/grammar/desciptors/SetDescriptor.cs +++ b/grammar/desciptors/SetDescriptor.cs @@ -10,7 +10,7 @@ namespace SPADE.Grammar.descriptor; class SetDescriptor { - public Func isMember; + public Func isMember; public Func genSet; @@ -23,7 +23,7 @@ public SetDescriptor(Dictionary _map) genSet = () => new UtilCollection(""); } - public SetDescriptor(Dictionary _map, Func _isMember, Func _genSet) + public SetDescriptor(Dictionary _map, Func _isMember, Func _genSet) { map = _map; isMember = _isMember; @@ -60,7 +60,7 @@ public SetDescriptor(Dictionary _map, FormatDescriptor f { constraintDescriptor.check(map); } - return true; + return true; }; genSet = () => throw new NotImplementedException("generating every set descriptor not implemented"); @@ -82,7 +82,7 @@ public SetDescriptor Cross(SetDescriptor other) public SetDescriptor UnorderedCross(SetDescriptor other) { - Func _isMember = (a) => + Func _isMember = (a) => { foreach (UtilCollection item in a) { @@ -91,10 +91,10 @@ public SetDescriptor UnorderedCross(SetDescriptor other) { return false; } - } + } return true; }; - Func _genSet = () => + Func _genSet = () => { UtilCollection set = new("{}"); foreach (UtilCollection first in genSet()) @@ -114,7 +114,7 @@ public SetDescriptor UnorderedCross(SetDescriptor other) public SetDescriptor Union(SetDescriptor other) { - + Func _isMember = (a) => isMember(a) || other.isMember(a); Func _genSet = () => genSet().Union(other.genSet()); return new SetDescriptor(map, _isMember, _genSet); diff --git a/grammar/desciptors/formatDescriptor.cs b/grammar/desciptors/formatDescriptor.cs index d86f1ab..a60adfa 100644 --- a/grammar/desciptors/formatDescriptor.cs +++ b/grammar/desciptors/formatDescriptor.cs @@ -10,7 +10,7 @@ class FormatDescriptor public FormatDescriptor(String term) { - parseFunc = (map, coll) => {map[term] = coll;}; + parseFunc = (map, coll) => { map[term] = coll; }; } public FormatDescriptor(List lst) @@ -19,7 +19,7 @@ public FormatDescriptor(List lst) { for (int i = 0; i < lst.Count(); i++) { - lst[i].parseFunc(map, coll[i]); + lst[i].parseFunc(map, coll[i]); } }; } From e895be27aeaf92ce00cc436ebfe66bee794aa63c Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:21:25 -0600 Subject: [PATCH 09/16] test: add the test project to the solution and collect coverage SPADE.slnx now names tests/SPADE.Tests, so rbs's unit-test stage runs it. coverlet.collector writes the cobertura report the coverage floor reads; without it the project gets no coverage gate at all. Same version as Redux's test project. --- SPADE.slnx | 3 +++ rbs.toml | 2 +- tests/SPADE.Tests/SPADE.Tests.csproj | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/SPADE.slnx b/SPADE.slnx index b5c1ef7..13d1b80 100644 --- a/SPADE.slnx +++ b/SPADE.slnx @@ -1,3 +1,6 @@ + + + diff --git a/rbs.toml b/rbs.toml index cfbb4f0..cc44659 100644 --- a/rbs.toml +++ b/rbs.toml @@ -5,5 +5,5 @@ engine = "dotnet" solution = "SPADE.slnx" [unit-test] -# The standard we're holding to. Floor 70%; the goal is 80%+. New code needs tests to keep it above the floor. +# The standard we're holding to. Floor 70%; the goal is 80%+. Line coverage is about 8% (Oct 2026, UtilCollection equality tests only); new code needs tests to reach the floor. coverage-min = 70 diff --git a/tests/SPADE.Tests/SPADE.Tests.csproj b/tests/SPADE.Tests/SPADE.Tests.csproj index 1ff3916..7e362f8 100644 --- a/tests/SPADE.Tests/SPADE.Tests.csproj +++ b/tests/SPADE.Tests/SPADE.Tests.csproj @@ -20,6 +20,10 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + From 8f68aa39bd3e3b888fe06b3516d52f817a1072c1 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:27:28 -0600 Subject: [PATCH 10/16] style: run dotnet format Whitespace only: dotnet format SPADE.slnx on this branch's code. --- StringParser.cs | 7 +++-- UtilCollection.cs | 28 +++++++++--------- grammar/BuiltInSet.cs | 3 +- grammar/codeGen.cs | 9 +++--- grammar/desciptors/ConstraintDescriptor.cs | 34 +++++++++++----------- grammar/desciptors/SetDescriptor.cs | 14 ++++----- grammar/desciptors/formatDescriptor.cs | 4 +-- 7 files changed, 52 insertions(+), 47 deletions(-) diff --git a/StringParser.cs b/StringParser.cs index 23fb6a1..ead7bc7 100644 --- a/StringParser.cs +++ b/StringParser.cs @@ -7,11 +7,12 @@ using SPADE.Grammar; namespace SPADE; + public class StringParser { readonly string format; public Dictionary map = new(); - + private bool hasParsed; public StringParser(string f) @@ -24,7 +25,7 @@ public void parse(string instance) { hasParsed = true; AntlrInputStream stream = new AntlrInputStream(format); - collLexer lex = new collLexer(stream); + collLexer lex = new collLexer(stream); CommonTokenStream tokens = new CommonTokenStream(lex); collParser parser = new collParser(tokens); @@ -35,7 +36,7 @@ public void parse(string instance) gen.Parse(new UtilCollection(instance)); - map = gen.map; + map = gen.map; } diff --git a/UtilCollection.cs b/UtilCollection.cs index 6a70fe7..6dfd345 100644 --- a/UtilCollection.cs +++ b/UtilCollection.cs @@ -47,7 +47,7 @@ public UtilCollection(HashSet set_) set = new(); foreach (string i in set_) { - set.Add(new UtilCollection(i)); + set.Add(new UtilCollection(i)); } } @@ -57,7 +57,7 @@ public UtilCollection(HashSet> set_) set = new(); foreach (HashSet i in set_) { - set.Add(new UtilCollection(i)); + set.Add(new UtilCollection(i)); } } @@ -75,19 +75,21 @@ public UtilCollection(HashSet set__) public UtilCollection this[int index] { - get { + get + { if (!isOrdered) throw new InvalidOperationException("Cannot index into a set"); - return list[index]; - } - set { + return list[index]; + } + set + { if (!isOrdered) throw new InvalidOperationException("Cannot index into a set"); - list[index] = value; - } + list[index] = value; + } } public void Add(UtilCollection item) { - if(isOrdered) + if (isOrdered) { list.Add(item); } @@ -133,7 +135,7 @@ public void assertOrdered() public void assertUnordered() { - if(isOrdered) throw new InvalidOperationException("Ordered"); + if (isOrdered) throw new InvalidOperationException("Ordered"); } public bool IsUnordered() @@ -190,7 +192,7 @@ public UtilCollection Intersect(UtilCollection other) throw new NotImplementedException(); } } - + public UtilCollection Union(UtilCollection other) { if (isOrdered != other.isOrdered) throw new InvalidOperationException("Can't take the union of a set and a list"); @@ -209,9 +211,9 @@ public UtilCollection Cross(UtilCollection other) UtilCollection ret = new UtilCollection(); foreach (UtilCollection a in this) { - foreach(UtilCollection b in other) + foreach (UtilCollection b in other) { - UtilCollection add = new UtilCollection(new List{a,b}); + UtilCollection add = new UtilCollection(new List { a, b }); ret.Add(add); } } diff --git a/grammar/BuiltInSet.cs b/grammar/BuiltInSet.cs index 948aba7..3c48b97 100644 --- a/grammar/BuiltInSet.cs +++ b/grammar/BuiltInSet.cs @@ -1,2 +1,3 @@ namespace SPADE.Grammar.Enums; -enum BuiltInSet {integers} \ No newline at end of file + +enum BuiltInSet { integers } \ No newline at end of file diff --git a/grammar/codeGen.cs b/grammar/codeGen.cs index 9aaa8f0..76b9934 100644 --- a/grammar/codeGen.cs +++ b/grammar/codeGen.cs @@ -10,18 +10,19 @@ namespace SPADE.Grammar; -class codeGen : IcollListener { +class codeGen : IcollListener +{ public Dictionary map; public Stack formatStack; - public SetDescriptor ?setDescriptor; + public SetDescriptor? setDescriptor; public Stack setDescriptorStack; public Stack constraintDescriptorStack; public Stack> constraintDescriptorStackStack; - public codeGen() : base() + public codeGen() : base() { map = new(); formatStack = new(); @@ -94,7 +95,7 @@ public void ExitFormat([NotNull] collParser.FormatContext context) List lst = new(); for (int i = 0; i < context.format().Length; i++) { - lst.Add(formatStack.Pop()); + lst.Add(formatStack.Pop()); } lst.Reverse(); // have to reverse the list, since the leftmost should be the one that was popped first formatStack.Push(new FormatDescriptor(lst, SourceText(context))); diff --git a/grammar/desciptors/ConstraintDescriptor.cs b/grammar/desciptors/ConstraintDescriptor.cs index 1cbd7c8..254d010 100644 --- a/grammar/desciptors/ConstraintDescriptor.cs +++ b/grammar/desciptors/ConstraintDescriptor.cs @@ -13,18 +13,18 @@ class ConstraintDescriptor public ConstraintDescriptor(String _term, String _definition, SetDescriptor _set, String _text) { - term = _term; - definition = _definition; - set = _set; - text = _text; + term = _term; + definition = _definition; + set = _set; + text = _text; } public ConstraintDescriptor(String _term, String _definition, String _built_in, String _text) { - term = _term; - definition = _definition; - built_in = _built_in; - text = _text; + term = _term; + definition = _definition; + built_in = _built_in; + text = _text; } // Values in error messages are shortened, so a huge instance doesn't make a huge message. @@ -73,17 +73,17 @@ public void check(Dictionary map) case "subset": if (set != null) { - if (map[term].IsValue()) - { - throw Failure($"{Describe(map[term])} is not a set"); - } - foreach (UtilCollection item in map[term]) + if (map[term].IsValue()) + { + throw Failure($"{Describe(map[term])} is not a set"); + } + foreach (UtilCollection item in map[term]) + { + if (!set.isMember(item)) { - if (!set.isMember(item)) - { - throw Failure($"{Describe(item)} is not a member"); - } + throw Failure($"{Describe(item)} is not a member"); } + } } break; } diff --git a/grammar/desciptors/SetDescriptor.cs b/grammar/desciptors/SetDescriptor.cs index 282cb65..e2607d3 100644 --- a/grammar/desciptors/SetDescriptor.cs +++ b/grammar/desciptors/SetDescriptor.cs @@ -10,7 +10,7 @@ namespace SPADE.Grammar.descriptor; class SetDescriptor { - public Func isMember; + public Func isMember; public Func genSet; @@ -23,7 +23,7 @@ public SetDescriptor(Dictionary _map) genSet = () => new UtilCollection(""); } - public SetDescriptor(Dictionary _map, Func _isMember, Func _genSet) + public SetDescriptor(Dictionary _map, Func _isMember, Func _genSet) { map = _map; isMember = _isMember; @@ -60,7 +60,7 @@ public SetDescriptor(Dictionary _map, FormatDescriptor f { constraintDescriptor.check(map); } - return true; + return true; }; genSet = () => throw new NotImplementedException("generating every set descriptor not implemented"); @@ -83,7 +83,7 @@ public SetDescriptor Cross(SetDescriptor other) public SetDescriptor UnorderedCross(SetDescriptor other) { - Func _isMember = (a) => + Func _isMember = (a) => { // Each member is a set of nodes, like the edge {a,b}. A list such as (a,b) // is an ordered pair and belongs to cross, and a plain value is not an edge. @@ -95,10 +95,10 @@ public SetDescriptor UnorderedCross(SetDescriptor other) { return false; } - } + } return true; }; - Func _genSet = () => + Func _genSet = () => { UtilCollection set = new("{}"); foreach (UtilCollection first in genSet()) @@ -118,7 +118,7 @@ public SetDescriptor UnorderedCross(SetDescriptor other) public SetDescriptor Union(SetDescriptor other) { - + Func _isMember = (a) => isMember(a) || other.isMember(a); Func _genSet = () => genSet().Union(other.genSet()); return new SetDescriptor(map, _isMember, _genSet); diff --git a/grammar/desciptors/formatDescriptor.cs b/grammar/desciptors/formatDescriptor.cs index bdce081..6198412 100644 --- a/grammar/desciptors/formatDescriptor.cs +++ b/grammar/desciptors/formatDescriptor.cs @@ -10,7 +10,7 @@ class FormatDescriptor public FormatDescriptor(String term) { - parseFunc = (map, coll) => {map[term] = coll;}; + parseFunc = (map, coll) => { map[term] = coll; }; } public FormatDescriptor(List lst, String text) @@ -23,7 +23,7 @@ public FormatDescriptor(List lst, String text) } for (int i = 0; i < lst.Count(); i++) { - lst[i].parseFunc(map, coll[i]); + lst[i].parseFunc(map, coll[i]); } }; } From 7145b7d0626ad239596b3846a64525c43ac5312b Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:35:34 -0600 Subject: [PATCH 11/16] ci: run the devcontainer on SDK 10 alone The tests move to net10.0, so the container no longer needs the .NET 8 runtime feature next to the SDK 10 image. SDK 10 still reads SPADE.slnx and builds the net6.0 library. --- .devcontainer/devcontainer.json | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index ca172ea..80dd4f7 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -8,11 +8,6 @@ "installZsh": false, "upgradePackages": false }, - // The tests target net8.0, so they need the .NET 8 runtime next to SDK 10. - "ghcr.io/devcontainers/features/dotnet:2": { - "version": "none", - "dotnetRuntimeVersions": "8.0" - }, "ghcr.io/devcontainers/features/docker-outside-of-docker:1": { "moby": false }, "ghcr.io/reduxisu/features/rbs:1": {} }, From 8829639cebe18b7f67b054ee5c1af3a6bc929686 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:37:20 -0600 Subject: [PATCH 12/16] test: run the tests on .NET 10 The test project moves from net8.0 to net10.0, and ci.yml and publish.yml install only the 10.0.x SDK, which still builds the net6.0 library. DiscreteParser.csproj stays on net6.0, the NuGet package's target. The test packages (xunit.v3 3.2.2, xunit.runner.visualstudio 4.0.0, Microsoft.NET.Test.Sdk 18.9.0, coverlet.collector 10.1.0) run on net10.0 unchanged. development.md now says the tests need the .NET 10 SDK. --- .github/workflows/ci.yml | 4 +--- .github/workflows/publish.yml | 4 +--- Documentation/development.md | 2 +- tests/SPADE.Tests/SPADE.Tests.csproj | 2 +- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55bb012..ef9143a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,9 +12,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-dotnet@v4 with: - dotnet-version: | - 6.0.x - 8.0.x + dotnet-version: 10.0.x - name: Build run: dotnet build DiscreteParser.csproj -c Release # The test project arrives with the first fix PR; until then there is nothing to run. diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index aa42087..fcdc2b1 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -15,9 +15,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-dotnet@v4 with: - dotnet-version: | - 6.0.x - 8.0.x + dotnet-version: 10.0.x - name: Check the tag matches the project version run: | version=$(grep -oP '(?<=)[^<]+' DiscreteParser.csproj) diff --git a/Documentation/development.md b/Documentation/development.md index 78728f6..8b23bbf 100644 --- a/Documentation/development.md +++ b/Documentation/development.md @@ -8,7 +8,7 @@ The tests live in tests/SPADE.Tests and use xUnit. Run them from the repository dotnet test tests/SPADE.Tests ``` -The test project targets .NET 8, so you need the .NET 8 SDK or newer. The library itself still targets .NET 6. +The test project targets .NET 10, so you need the .NET 10 SDK or newer. The library itself still targets .NET 6, and SDK 10 builds it. ## antlr diff --git a/tests/SPADE.Tests/SPADE.Tests.csproj b/tests/SPADE.Tests/SPADE.Tests.csproj index 7e362f8..2073855 100644 --- a/tests/SPADE.Tests/SPADE.Tests.csproj +++ b/tests/SPADE.Tests/SPADE.Tests.csproj @@ -1,7 +1,7 @@ - net8.0 + net10.0 enable enable false From 082150dc14a2e71d8965c062187a2b36ed0d7eff Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:53:34 -0600 Subject: [PATCH 13/16] ci: keep the coverage note in rbs.toml free of a per-branch figure The measured figure changes with every PR in the stack, so the comment states only the floor and the goal; the rbs report shows the current number. --- rbs.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rbs.toml b/rbs.toml index cc44659..cfbb4f0 100644 --- a/rbs.toml +++ b/rbs.toml @@ -5,5 +5,5 @@ engine = "dotnet" solution = "SPADE.slnx" [unit-test] -# The standard we're holding to. Floor 70%; the goal is 80%+. Line coverage is about 8% (Oct 2026, UtilCollection equality tests only); new code needs tests to reach the floor. +# The standard we're holding to. Floor 70%; the goal is 80%+. New code needs tests to keep it above the floor. coverage-min = 70 From dbc816b0ed86037c3841f6af6d31cc8fd5cbfd78 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:10:48 -0600 Subject: [PATCH 14/16] fix(collections): freeze a collection once it is hashed A HashSet or Dictionary files each element under the hash it had when it went in. UtilCollection is mutable, so changing a collection (or anything nested in it) after it has been hashed left it under a stale hash: Contains and equality lookups missed it and duplicates slipped in, with no error. The first GetHashCode() call now freezes the collection. A list's or set's hash is built from its elements' GetHashCode(), so they freeze too, all the way down. Add and the indexer setter throw InvalidOperationException on a frozen collection. The hash is computed once and cached. The List and HashSet constructors copy their argument instead of keeping the caller's reference, and ToList() on a list returns a copy, so a frozen collection can't be changed from outside either. --- Documentation/utilcollection.md | 20 ++- UtilCollection.cs | 37 ++++- .../SPADE.Tests/UtilCollectionFreezeTests.cs | 156 ++++++++++++++++++ 3 files changed, 207 insertions(+), 6 deletions(-) create mode 100644 tests/SPADE.Tests/UtilCollectionFreezeTests.cs diff --git a/Documentation/utilcollection.md b/Documentation/utilcollection.md index 89e68b1..66ebd72 100644 --- a/Documentation/utilcollection.md +++ b/Documentation/utilcollection.md @@ -35,7 +35,7 @@ Hamiltonian path is a graph problem, and the string follows the formal definitio UtilCollections provides a few Utility methods to help process and extract the instance information. Each method works for both ordered and unordered collections, unless specified otherwise. ### Indexing -If the collection is ordered, then you can index into the list using square brackets to get each element of the list, the same as you would for an array. Returns a UtilCollections object. +If the collection is ordered, then you can index into the list using square brackets to get each element of the list, the same as you would for an array. Returns a UtilCollections object. Assigning to an index (`list[0] = item`) replaces that element, unless the list has been hashed (see [Collections freeze once hashed](#collections-freeze-once-hashed)). ### Enumeration Using foreach loops to iterate over each element in the collection is also possible, for example, using a UtilCollection object named collection: @@ -57,7 +57,7 @@ Returns True if the two UtilCollections objects are equal to each other and Fals Returns the length of the collection. ### ToList -Returns a list representing the elements of the collection. If the collection is ordered, it returns the same list. If the collection is unordered, it is cast to a list and then returned. +Returns a list representing the elements of the collection. If the collection is ordered, it returns a copy of the list, so changing the copy doesn't change the collection. If the collection is unordered, it is cast to a list and then returned. ### ToString Returns the string representation of the UtilCollection. @@ -66,7 +66,21 @@ Returns the string representation of the UtilCollection. Checks if the collection contains another UtilCollection object. Comparison is done by value. Returns true if it contains it, false if it doesn't. ### Add -Adds a UtilCollection object to the collection. +Adds a UtilCollection object to the collection. Throws `InvalidOperationException` if the collection has been hashed (see below). + +### Collections freeze once hashed +A `HashSet` or `Dictionary` files each element under the hash code it had when it went in. If the element changed afterwards, it would sit under a stale hash: `Contains` would miss it and duplicates could slip in, with no error. So the first time a UtilCollection is hashed, it freezes, and `Add` or assigning to an index throws `InvalidOperationException`. That happens when it is added to a set (including a UtilCollection set), used as a dictionary key, or put inside a list or set that is itself hashed, since hashing a collection hashes everything inside it. + +Build each collection fully, then add it to a set: + +```c# +UtilCollection edge = new UtilCollection("{}"); +edge.Add(new UtilCollection("1")); +edge.Add(new UtilCollection("2")); +edges.Add(edge); // edge is frozen from here on +``` + +A collection that has never been hashed can still be changed freely. The constructors that take a `List` or `HashSet` copy it, so changing the original afterwards doesn't change the collection. ### parseInt Attempts to parse and return itself as an integer. diff --git a/UtilCollection.cs b/UtilCollection.cs index 6d7f426..1b4ab49 100644 --- a/UtilCollection.cs +++ b/UtilCollection.cs @@ -14,6 +14,12 @@ public class UtilCollection private bool isOrdered; private bool isValue; + // Set the first time this collection is hashed. A HashSet keeps an element under the hash it + // had when it went in, so changing the element (or anything inside it) afterwards would leave + // it under a stale hash, and lookups would quietly miss it. Once frozen, every change throws. + // Hashing a list or set hashes its elements, so they freeze too, all the way down. + private bool frozen; + private int hashCode; public UtilCollection() { @@ -73,15 +79,16 @@ public UtilCollection(HashSet> set_) } } + // Both copy, so later changes to the caller's list or set can't change this collection. public UtilCollection(List list__) { - list = list__; + list = new List(list__); isOrdered = true; } public UtilCollection(HashSet set__) { - set = set__; + set = new HashSet(set__); isOrdered = false; } @@ -132,12 +139,14 @@ public UtilCollection this[int index] set { if (!isOrdered) throw new InvalidOperationException("Cannot index into a set"); + ThrowIfFrozen(); list[index] = value; } } public void Add(UtilCollection item) { + ThrowIfFrozen(); if (isOrdered) { list.Add(item); @@ -299,7 +308,28 @@ public bool NotEquals(object? obj) return true; } + private void ThrowIfFrozen() + { + if (frozen) + { + throw new InvalidOperationException( + "This collection has been hashed (it is in a set or used as a key) and can't be changed; " + + "build it fully before adding it to a set."); + } + } + + // The first call freezes the collection, so the hash can't change and is worked out only once. public override int GetHashCode() + { + if (!frozen) + { + hashCode = ComputeHashCode(); + frozen = true; + } + return hashCode; + } + + private int ComputeHashCode() { if (isValue) { @@ -332,7 +362,8 @@ public List ToList() { if (isOrdered) { - return list; + // A copy, so changes to it can't reach a collection that may already be frozen. + return new List(list); } else { diff --git a/tests/SPADE.Tests/UtilCollectionFreezeTests.cs b/tests/SPADE.Tests/UtilCollectionFreezeTests.cs new file mode 100644 index 0000000..10362cf --- /dev/null +++ b/tests/SPADE.Tests/UtilCollectionFreezeTests.cs @@ -0,0 +1,156 @@ +using SPADE; +using Xunit; + +namespace SPADE.Tests; + +// A collection freezes once it is hashed, so it can't end up in a set under a stale hash. +public class UtilCollectionFreezeTests +{ + private static UtilCollection C(string s) => new UtilCollection(s); + + [Fact] + public void AddThrowsOnceInAHashSet() + { + UtilCollection item = C("{1,2}"); + HashSet outer = new() { item }; + + InvalidOperationException e = Assert.Throws(() => item.Add(C("3"))); + Assert.Contains("has been hashed", e.Message); + Assert.Equal(2, item.Count()); + Assert.Contains(C("{2,1}"), outer); + } + + [Fact] + public void AddThrowsOnceUsedAsADictionaryKey() + { + UtilCollection key = C("(1,2)"); + Dictionary counts = new() { [key] = 1 }; + + Assert.Throws(() => key.Add(C("3"))); + Assert.Equal(1, counts[C("(1,2)")]); + } + + [Fact] + public void AddingToASetFreezesTheElement() + { + UtilCollection outer = C("{}"); + UtilCollection item = C("(1,2)"); + outer.Add(item); + + Assert.Throws(() => item.Add(C("3"))); + Assert.True(outer.Contains(C("(1,2)"))); + } + + [Fact] + public void ChildOfAHashedListIsFrozen() + { + UtilCollection child = C("{1}"); + UtilCollection pair = new UtilCollection(new List { child, C("x") }); + _ = new HashSet { pair }; + + Assert.Throws(() => child.Add(C("2"))); + } + + [Fact] + public void GrandchildOfAHashedSetIsFrozen() + { + UtilCollection outer = C("{({1},a)}"); + _ = outer.GetHashCode(); + UtilCollection pair = outer.ToList()[0]; + UtilCollection grandchild = pair[0]; + + Assert.Throws(() => grandchild.Add(C("2"))); + } + + [Fact] + public void IndexerSetterThrowsOnAHashedList() + { + UtilCollection list = C("(1,2)"); + _ = new HashSet { list }; + + Assert.Throws(() => list[0] = C("3")); + Assert.Equal("(1,2)", list.ToString()); + } + + [Fact] + public void HashIsTheSameOnEveryCall() + { + UtilCollection item = C("{(1,2),{3}}"); + Assert.Equal(item.GetHashCode(), item.GetHashCode()); + Assert.Equal(C("{{3},(1,2)}").GetHashCode(), item.GetHashCode()); + } + + [Fact] + public void ListConstructorCopiesItsArgument() + { + List source = new() { C("1"), C("2") }; + UtilCollection list = new UtilCollection(source); + source.Add(C("3")); + source[0] = C("9"); + + Assert.Equal("(1,2)", list.ToString()); + } + + [Fact] + public void SetConstructorCopiesItsArgument() + { + HashSet source = new() { C("1"), C("2") }; + UtilCollection set = new UtilCollection(source); + source.Add(C("3")); + source.Remove(C("1")); + + Assert.Equal(C("{1,2}"), set); + } + + [Fact] + public void ToListOfAListIsACopy() + { + UtilCollection list = C("(1,2)"); + _ = list.GetHashCode(); + list.ToList().Add(C("3")); + + Assert.Equal(2, list.Count()); + } + + [Fact] + public void BuildingFullyThenInsertingWorks() + { + UtilCollection edges = C("{}"); + foreach ((string a, string b) in new[] { ("1", "2"), ("2", "3"), ("2", "1") }) + { + UtilCollection edge = C("{}"); + edge.Add(C(a)); + edge.Add(C(b)); + edges.Add(edge); + } + + // {1,2} and {2,1} are the same edge. + Assert.Equal(2, edges.Count()); + Assert.True(edges.Contains(C("{2,1}"))); + Assert.True(edges.Contains(C("{3,2}"))); + Assert.False(edges.Contains(C("{1,3}"))); + } + + [Fact] + public void UnhashedCollectionsCanStillChange() + { + UtilCollection set = C("{1}"); + set.Add(C("2")); + UtilCollection list = C("(1,2)"); + list.Add(C("3")); + list[0] = C("0"); + + Assert.Equal(C("{1,2}"), set); + Assert.Equal("(0,2,3)", list.ToString()); + } + + [Fact] + public void EqualsDoesNotFreeze() + { + UtilCollection set = C("{1}"); + Assert.Equal(C("{1}"), set); + set.Add(C("2")); + + Assert.Equal(2, set.Count()); + } +} From 45eea963bf4b9e3120e05e5ac883e3432f5f99d3 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:04:39 -0600 Subject: [PATCH 15/16] fix(collections): make the cached hash safe to read from several threads GetHashCode writes the hash and then sets frozen. Without a barrier, another thread could see frozen before the hash and return 0. frozen is now volatile, so a thread that sees it also sees the hash; two threads computing it at once get the same value. A frozen collection can't change, so it is safe to share between threads; building one still isn't, as with List and HashSet. Documented, with a test that races threads to hash a fresh nested set. --- Documentation/utilcollection.md | 2 ++ UtilCollection.cs | 4 +++- tests/SPADE.Tests/UtilCollectionFreezeTests.cs | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Documentation/utilcollection.md b/Documentation/utilcollection.md index 66ebd72..8c63907 100644 --- a/Documentation/utilcollection.md +++ b/Documentation/utilcollection.md @@ -82,6 +82,8 @@ edges.Add(edge); // edge is frozen from here on A collection that has never been hashed can still be changed freely. The constructors that take a `List` or `HashSet` copy it, so changing the original afterwards doesn't change the collection. +A frozen collection can't change, so it is safe to share between threads: hashing, comparing, `Contains` and enumerating it from several threads at once all work. Building a collection is not thread-safe, the same as `List` and `HashSet`: fill it on one thread, then share it. + ### parseInt Attempts to parse and return itself as an integer. diff --git a/UtilCollection.cs b/UtilCollection.cs index 1b4ab49..d434588 100644 --- a/UtilCollection.cs +++ b/UtilCollection.cs @@ -18,7 +18,9 @@ public class UtilCollection // had when it went in, so changing the element (or anything inside it) afterwards would leave // it under a stale hash, and lookups would quietly miss it. Once frozen, every change throws. // Hashing a list or set hashes its elements, so they freeze too, all the way down. - private bool frozen; + // volatile: GetHashCode writes hashCode before frozen, so a thread that sees frozen also sees + // the hash. A frozen collection is safe to read from several threads. + private volatile bool frozen; private int hashCode; public UtilCollection() diff --git a/tests/SPADE.Tests/UtilCollectionFreezeTests.cs b/tests/SPADE.Tests/UtilCollectionFreezeTests.cs index 10362cf..50fb709 100644 --- a/tests/SPADE.Tests/UtilCollectionFreezeTests.cs +++ b/tests/SPADE.Tests/UtilCollectionFreezeTests.cs @@ -144,6 +144,21 @@ public void UnhashedCollectionsCanStillChange() Assert.Equal("(0,2,3)", list.ToString()); } + [Fact] + public void HashingFromManyThreadsAgrees() + { + // Each round starts unfrozen, so the threads race to compute and cache the hash. + for (int round = 0; round < 200; round++) + { + UtilCollection set = C("{({1,2},{3,4}),({5,6},(7,8)),9,{10,{11,12}}}"); + int expected = C("{({1,2},{3,4}),({5,6},(7,8)),9,{10,{11,12}}}").GetHashCode(); + int[] seen = new int[8]; + Parallel.For(0, seen.Length, i => seen[i] = set.GetHashCode()); + + Assert.All(seen, h => Assert.Equal(expected, h)); + } + } + [Fact] public void EqualsDoesNotFreeze() { From 2b8b0187620ee4d42d33c81ce133f8644fa74779 Mon Sep 17 00:00:00 2001 From: Michael Trosper <140276486+Trosper3@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:11:22 -0600 Subject: [PATCH 16/16] feat(collections): add Freeze() and document using SPADE from several threads Freezing used to happen only as a side effect of hashing, so a list that was built but never hashed could still be changed while other threads read it. Freeze() freezes a collection and everything inside it on purpose and returns it; IsFrozen() reports it. Documentation/threads.md says what is safe to share (frozen collections, sets) and what isn't (a StringParser while it parses, a collection still being built), with the parse-once, freeze, then share pattern. --- Documentation/index.md | 1 + Documentation/threads.md | 35 +++++++++++++++++++ Documentation/utilcollection.md | 2 ++ UtilCollection.cs | 13 +++++++ .../SPADE.Tests/UtilCollectionFreezeTests.cs | 31 ++++++++++++++++ 5 files changed, 82 insertions(+) create mode 100644 Documentation/threads.md diff --git a/Documentation/index.md b/Documentation/index.md index ef2580f..5737189 100644 --- a/Documentation/index.md +++ b/Documentation/index.md @@ -7,4 +7,5 @@ SPADE is a tool that parses strings representing instances of discrete math type - [Instance Strings](InstanceString.md) - [Definition String](DefinitionString.md) - [Utilcollections](utilcollection.md) + - [Using SPADE from several threads](threads.md) - [SPADE Development](development.md) \ No newline at end of file diff --git a/Documentation/threads.md b/Documentation/threads.md new file mode 100644 index 0000000..0a8ff18 --- /dev/null +++ b/Documentation/threads.md @@ -0,0 +1,35 @@ +# Using SPADE from several threads + +SPADE's objects follow the usual .NET rule: build on one thread, then share. Once an instance has been parsed and frozen, any number of threads can read it at the same time. + +## Safe to share + +- **A frozen `UtilCollection`.** Hashing, `Equals`, `Contains`, `Count`, indexing, enumerating and `ToString` are all safe from several threads at once. A frozen collection can't change, and its hash is computed once and published safely. +- **Sets as lookup tables.** Everything inside a set is frozen as soon as it goes in, so a shared `HashSet` can't be changed under another thread (as long as no thread adds to the set itself). + +## Not safe to share + +- **A `StringParser` while it parses.** `parse()` replaces the parser's results, so two threads parsing with the same `StringParser` overwrite each other. Use one `StringParser` per thread, or per parse; they are cheap to create. Reading the results of a finished parse from several threads is fine. +- **A collection that is still being built.** Don't call `Add`, or assign to an index, while another thread reads the collection. The same is true of `List` and `HashSet`. + +## Recommended pattern + +Parse once, freeze the result, then hand it to the workers: + +```c# +StringParser parser = new("{((N,E),K) | N is set, E subset N unorderedcross N, K is int}"); +parser.parse(instance); + +UtilCollection nodes = parser["N"].Freeze(); +UtilCollection edges = parser["E"].Freeze(); + +Dictionary degree = new(); +Parallel.ForEach(nodes.ToList(), node => +{ + // Every thread reads the same frozen collections. + int d = edges.ToList().Count(edge => edge.Contains(node)); + lock (degree) degree[node.ToString()] = d; +}); +``` + +`Freeze()` freezes a collection and everything inside it, and returns the same collection. After that, any attempt to change it throws `InvalidOperationException`, so a stray `Add` in a worker fails loudly instead of corrupting shared data. `IsFrozen()` tells you whether a collection is frozen. diff --git a/Documentation/utilcollection.md b/Documentation/utilcollection.md index 8c63907..6a29fa0 100644 --- a/Documentation/utilcollection.md +++ b/Documentation/utilcollection.md @@ -71,6 +71,8 @@ Adds a UtilCollection object to the collection. Throws `InvalidOperationExceptio ### Collections freeze once hashed A `HashSet` or `Dictionary` files each element under the hash code it had when it went in. If the element changed afterwards, it would sit under a stale hash: `Contains` would miss it and duplicates could slip in, with no error. So the first time a UtilCollection is hashed, it freezes, and `Add` or assigning to an index throws `InvalidOperationException`. That happens when it is added to a set (including a UtilCollection set), used as a dictionary key, or put inside a list or set that is itself hashed, since hashing a collection hashes everything inside it. +To freeze a collection on purpose, call `Freeze()`. It freezes the collection and everything inside it, and returns the collection, so `new UtilCollection(text).Freeze()` works. `IsFrozen()` says whether a collection is frozen. + Build each collection fully, then add it to a set: ```c# diff --git a/UtilCollection.cs b/UtilCollection.cs index d434588..85f2b04 100644 --- a/UtilCollection.cs +++ b/UtilCollection.cs @@ -310,6 +310,19 @@ public bool NotEquals(object? obj) return true; } + // Freezes this collection and everything inside it, so it can be shared between threads, or + // kept, knowing nothing will change it. Hashing does the freezing, all the way down. + public UtilCollection Freeze() + { + GetHashCode(); + return this; + } + + public bool IsFrozen() + { + return frozen; + } + private void ThrowIfFrozen() { if (frozen) diff --git a/tests/SPADE.Tests/UtilCollectionFreezeTests.cs b/tests/SPADE.Tests/UtilCollectionFreezeTests.cs index 50fb709..820ae5d 100644 --- a/tests/SPADE.Tests/UtilCollectionFreezeTests.cs +++ b/tests/SPADE.Tests/UtilCollectionFreezeTests.cs @@ -159,6 +159,37 @@ public void HashingFromManyThreadsAgrees() } } + [Fact] + public void FreezeLocksTheCollectionAndEverythingInside() + { + UtilCollection child = C("(1,2)"); + UtilCollection list = new UtilCollection(new List { child, C("{3}") }); + + Assert.Same(list, list.Freeze()); + Assert.True(list.IsFrozen()); + Assert.True(child.IsFrozen()); + Assert.Throws(() => list.Add(C("4"))); + Assert.Throws(() => child[0] = C("0")); + Assert.Equal("((1,2),{3})", list.ToString()); + } + + [Fact] + public void FreezeTwiceIsHarmlessAndKeepsTheHash() + { + UtilCollection set = C("{1,{2,3}}"); + int hash = set.Freeze().GetHashCode(); + + Assert.Equal(hash, set.Freeze().GetHashCode()); + Assert.Equal(C("{{3,2},1}").GetHashCode(), hash); + } + + [Fact] + public void NewCollectionsAreNotFrozen() + { + Assert.False(C("{1,2}").IsFrozen()); + Assert.False(C("(1,2)").IsFrozen()); + } + [Fact] public void EqualsDoesNotFreeze() {