Skip to content

A quantity is a value type - #210

Open
matt-edmondson wants to merge 2 commits into
mainfrom
claude/peaceful-mayer-oo6l2u
Open

A quantity is a value type#210
matt-edmondson wants to merge 2 commits into
mainfrom
claude/peaceful-mayer-oo6l2u

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Every generated physical quantity was a record class deriving from an abstract PhysicalQuantity, whose Create was:

new TQuantity() with { Quantity = value }

That is two heap allocations for one numbernew TQuantity() builds an instance and with clones it — and every operator and every unit factory went through it.

Measured

Length<double>, allocation per operation, before and after:

before after
a + b 48.0 B 0.0 B
FromMeter 48.0 B 0.0 B
FromFoot (converting) 48.0 B 0.0 B
Length / Duration 48.0 B 0.0 B
Velocity3D + * 48.0 B 0.0 B

In a loop over a few thousand entities at 60 Hz that is tens of megabytes a second of garbage, from the library whose entire job is to be the type a number is stored in. QuantityValueTypeTests measures it now, so it cannot come back quietly.

What changed

All 212 generated quantities are readonly record struct. A struct cannot inherit, so the abstract PhysicalQuantity record is gone and what it carried is split in two:

  • IPhysicalQuantity<TSelf, T> declares static abstract TSelf Create(T), replacing the where TSelf : PhysicalQuantity<TSelf, T>, new() constraint the CRTP base needed, so generic code over quantities still works.
  • PhysicalQuantityCore holds the validity and cross-dimension comparison rules. Each quantity delegates in one line rather than carrying a copy, and the receiver is taken by in so delegating does not box it.

Arithmetic, ordering and the IPhysicalQuantity surface are emitted per type. That is not merely how the surface is kept — it is the point: an operator declared on the struct is a plain expression the JIT inlines, where the shared generic base allocated a result object every time. The cross-dimensional operators, which called the inherited Multiply/Divide helpers, construct their result directly for the same reason.

SemanticQuantity<TStorage> and SemanticQuantity<TSelf, TStorage> are untouched. They remain available for a reference-type semantic quantity; the physical quantities simply no longer derive from them.

Behaviour preserved

The Vector0 non-negativity guards, the strictly-positive guards, divide-by-zero, V0's T.Abs(left - right) subtraction, implicit widening to a semantic overload's base, In(unit), Dimension, ToString as the bare value, and CompareTo/Equals throwing and not-throwing respectively across dimensions.

The 1119 existing tests pass unchanged, which is the evidence for that claim. Six new tests cover the value-type and allocation properties.

The one behaviour given up

A struct always has a zero value that no factory intercepts. For every quantity whose invariant is non-negativity that is fine, since zero satisfies it. For the three declaring physicalConstraints.minExclusive: "0"Wavelength, Period, HalfLifedefault reaches a value From{Unit}(0) refuses. A test pins that rather than leaving it to be discovered, and docs/migration-guide-4.0.md says so plainly.

Verification

  • dotnet build -c Release — 0 warnings, 0 errors across net8.0 / net9.0 / net10.0
  • 1125 tests pass (1119 baseline + 6 new), 0 failures
  • SonarCloud analyzers run locally per CLAUDE.mdno findings in changed code (5 pre-existing findings remain in DirectoryNameTests.cs and TextValidationAttributesTests.cs, untouched here)
  • Alias props are keyed off generated filenames, which are unchanged, so verify-generated should be clean

Docs

docs/migration-guide-4.0.md is new. CLAUDE.md, docs/architecture.md, docs/physics-generator.md and docs/strategy-unified-vector-quantities.md are updated to match.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AhoPJ5AbxP8QEBNxPQYEPk


Generated by Claude Code

Every generated physical quantity was a record class deriving from an abstract
PhysicalQuantity, whose Create was:

    new TQuantity() with { Quantity = value }

That is two heap allocations for one number -- `new TQuantity()` builds an
instance and `with` clones it -- and every operator and every unit factory went
through it. Measured on Length<double>, before and after:

    a + b             48.0 -> 0.0 bytes/op
    FromMeter         48.0 -> 0.0 bytes/op
    FromFoot          48.0 -> 0.0 bytes/op
    Length / Duration 48.0 -> 0.0 bytes/op
    Velocity3D + *    48.0 -> 0.0 bytes/op

In a loop over a few thousand entities at 60 Hz that is tens of megabytes a
second of garbage, from the library whose entire job is to be the type a number
is stored in. QuantityValueTypeTests measures it now, so it cannot come back
quietly.

All 212 generated quantities are `readonly record struct`. A struct cannot
inherit, so the abstract PhysicalQuantity record is gone and what it carried is
split in two:

  - IPhysicalQuantity<TSelf, T> declares `static abstract TSelf Create(T)`,
    replacing the `where TSelf : PhysicalQuantity<TSelf, T>, new()` constraint
    the CRTP base needed, so generic code over quantities still works.

  - PhysicalQuantityCore holds the validity and cross-dimension comparison
    rules. Each quantity delegates in one line rather than carrying a copy, and
    the receiver is taken by `in` so delegating does not box it.

Arithmetic, ordering and the IPhysicalQuantity surface are emitted per type.
That is not merely how the surface is kept -- it is the point: an operator
declared on the struct is a plain expression the JIT inlines, where the shared
generic base allocated a result object every time. The cross-dimensional
operators, which called the inherited Multiply/Divide helpers, now construct
their result directly for the same reason.

SemanticQuantity<TStorage> and SemanticQuantity<TSelf, TStorage> are untouched.
They remain available for a reference-type semantic quantity; the physical
quantities simply no longer derive from them.

Behaviour is preserved throughout: the Vector0 non-negativity guards, the
strictly-positive guards, divide-by-zero, V0's `T.Abs(left - right)`
subtraction, implicit widening to a semantic overload's base, `In(unit)`,
`Dimension`, `ToString` as the bare value, and CompareTo/Equals throwing and
not-throwing respectively across dimensions. The 1119 existing tests pass
unchanged, which is the evidence for that claim, and six new ones cover the
value-type and allocation properties.

One behaviour is given up, and docs/migration-guide-4.0.md says so plainly: a
struct always has a zero value that no factory intercepts. For every quantity
whose invariant is non-negativity that is fine, since zero satisfies it. For
the three declaring `physicalConstraints.minExclusive: "0"` -- Wavelength,
Period and HalfLife -- `default` reaches a value `From{Unit}(0)` refuses. A
test pins that rather than leaving it to be discovered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AhoPJ5AbxP8QEBNxPQYEPk
MSTEST0037, the one new issue SonarCloud reported on the PR. Asking
`Assert.IsTrue(list.Count == 0, …)` makes the assertion library report a
false boolean; `Assert.IsEmpty(list, …)` lets it report which collection was
not empty. The custom message is kept either way, since it is what names the
offending types.

Missed by the local Sonar run: it is INFO severity under MSTEST0037 rather
than an S-rule, so the grep for `error|warning S` in that pass did not match
it.

1125 tests still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AhoPJ5AbxP8QEBNxPQYEPk

Copy link
Copy Markdown
Contributor Author

github-advanced-security is red on b881dd1, and it is not this PR's failure.

The job died during setup, before it analysed anything:

Fatal error fetching CCA job details: Error: CAPI proxy GET /agents/swe/agent/jobs/… fetch failed;
cause: TimeoutError: The operation was aborted due to timeout

That is a timeout reaching api.business.githubcopilot.com — GitHub's own Copilot service — so no scan ran and no finding was reported. Three things say it isn't the diff:

  • The same check passed on 4ff1d42, the previous head of this branch.
  • The only change between the two commits is one line in QuantityValueTypeTests.cs: Assert.IsTrue(list.Count == 0, …)Assert.IsEmpty(list, …), addressing the MSTEST0037 that SonarCloud reported.
  • The error names a service the diff does not touch, and the failure is in job setup rather than in any analysis.

There is no fix to port — nothing in this repository is broken.

I could not re-run it. Both rerun-failed-jobs and rerun return 403 This workflow run cannot be retried; it is a GitHub-managed dynamic workflow (dynamic/agents/github-advanced-security), not one of this repo's own, so the API will not retry it. It needs either a re-run by someone with the means, or the next push to this branch to trigger a fresh one.

Everything else on this head is green or still running: Generated files up to date, Discover Test Projects and Analyze (actions) have passed; the two test jobs and the CodeQL C# analyses are in progress. On the previous head all 13 checks passed, including both test jobs and the SonarCloud quality gate.

I'm still watching this PR and will keep checking until it is green and mergeable.


Generated by Claude Code

@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants