Repository navigation
Conversation
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 ReduxISU#9
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 ReduxISU#12
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 ReduxISU#12
"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 ReduxISU#10
This was referenced Oct 1, 2026
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.
Whitespace only: dotnet format SPADE.slnx, so rbs's format-check starts clean. Brace placement, indentation and blank lines; no code changes.
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.
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.
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.
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.
Contributor
Author
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<UtilCollection> and HashSet<UtilCollection> 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.
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.
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
N is intchecked nothing, andS is setaccepted plain values, so grammars that relied on them let bad input through. Both now check the type they name.Stacked on #13 (which is stacked on #11). Only the last commit,
fix(grammar): make is int, is set and is list check their type, belongs to this PR.Merge order
#11, #13, #15 and #16 are stacked. Merge them into
mainin this order:dotnet format(the new base)unorderedcrossedgesis intandis setconstraints don't check their type #10):is int/is set/is listtype checks (this PR)After each merge, the next PR shows only its own commits. If one is squash-merged, I'll rebase the next one onto
mainbefore it's reviewed.Releasing for Redux: publish a new version only after all four are merged. Redux's grammar fixes (ReduxISU/Redux#628, ReduxISU/Redux#629, ReduxISU/Redux#630) work with 0.1.2 and can land any time before the upgrade; only ReduxISU/Redux#630's test changes go in the same Redux PR that upgrades the SPADE package. Tracked in ReduxISU/Redux#627.
What changed
is int:intreaches the constraint as a set rather than a keyword, and theisbranch only handled keywords, so nothing was checked.abc,{1}and(1,2)all passed.codeGen.ExitConstraintnow routesis intto the type check, which requires a single value that parses as an integer.S subset intis unchanged and still checks every element.is set: a plain value isn't ordered, so it passed. It now needs an unordered collection.is listalready behaved correctly; it's now covered by tests.tests/SPADE.Tests/TypeConstraintTests.cs, includingis intnested in a tuple and inside a set comprehension (w is intfor edge weights).Not in this PR
x is <set expression>(e.g.e is N unorderedcross N,u is N) still checks nothing, and undefined names likestringare still ignored. Turning those on would break several Redux grammars that useiswhere they meansubset, so it's tracked separately in #14.Testing
dotnet test tests/SPADE.Tests: 45 passed. Before the fix, 7 fail.k is intfor a field namedK), which the oldis intnever looked up. 1 is a Subset Sum test that expected the bare certificate101to read as an empty set.Before releasing
Redux needs ReduxISU/Redux#630 (as well as ReduxISU/Redux#629 for #13) before it upgrades.
Closes #10