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/12] 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/12] 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/12] 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 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 04/12] 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 05/12] 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 06/12] 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 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 07/12] 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 08/12] 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 09/12] 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 10/12] 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 11/12] 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 12/12] 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() {