Share the quantity vocabulary between both generators - #219
Merged
Merged
Conversation
The physics lived in Semantics.Cpp. Only the C++ projection knew whether a
declared relationship was dimensionally true, because it needed the exponents
to write Quantity<D> and so could not avoid multiplying them out. The C#
generator checked that a relationship's names resolved (SEM001) and that its
forms existed (SEM003), and then emitted the operator.
So Sensitivity * Pressure -> ElectricPotential has been shipping as
public static VoltageMagnitude<T> operator *(Sensitivity<T> l, Pressure<T> r)
with Sensitivity at M⁻¹L⁻¹T²I and Pressure at ML⁻¹T⁻², whose product is L⁻²I
while ElectricPotential is ML²T⁻³I⁻¹. It compiles, it runs, and it computes the
wrong physics. C++ has refused it by name since the projection existed.
QuantityVocabulary, DimensionVector and the resolution around them now live in
Semantics.Vocabulary and both generators run them. The two keep their own
readers of dimensions.json -- they differ in what they carry beyond the physics,
which is the point QuantityMetadata's remarks already made -- and each projects
onto a small declaration shape the vocabulary owns.
It is shared source rather than a project, which its README argues at length.
Briefly: the C# generator is a Roslyn component that bundles its dependencies
for analysis-time loading, and Semantics.Cpp is packed, so an assembly would
cost a load-time hazard on one side and a published package on the other. The
price is that the files compile under two analysis configurations, which is why
everything in them stays inside netstandard2.0's surface.
The new check is SEM008, and it reports rather than drops. Removing an operator
from a shipped package is a breaking change nobody asked for, and the metadata
bug behind the worst of these is a physics call that CLAUDE.md already declines
to guess at. ktsu.Sdk builds warnings as errors, so SEM008 is suppressed in
Semantics.Quantities alone, and UnkeepableRelationshipTests pins the refused set
to exactly the five that are documented -- a sixth fails there rather than
disappearing into that suppression.
Two tests asserted the real metadata reported nothing at all. That invariant was
true only because nothing was checking; they now assert nothing unexpected, with
SEM008 excluded by identifier and the set pinned elsewhere.
Generated output is byte-identical, and the C++ tests that compile the whole
vocabulary with g++ and clang++ pass unchanged, so the lift moved the code
without moving the behaviour.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QGCMUrT3jBgmANHNHcPmBf
Path.Combine discards everything before a rooted segment, which is what the quality analyzer flags. Nothing can be rooted here -- both segments after the base directory are constant relative literals -- so this was not a live bug, but Path.Join has no such rule, is the right API for joining segments already known to be relative, and is how the identical call in Semantics.Cpp.Test is already written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGCMUrT3jBgmANHNHcPmBf
|
matt-edmondson
pushed a commit
that referenced
this pull request
Sep 12, 2026
Six, all in code #219 added, and none of them arguable once the repository's own conventions are checked rather than assumed. The gate passed there and it merged before these landed. Four are MSTest analyzer findings on the new test, and this repository wants exactly what they ask for: Assert.Contains has 105 call sites in Semantics.Test and StringAssert.Contains had two, both of them from that PR; Assert.DoesNotContain has ten. Worth noting because the same rule went the other way in ktsu.Coder, where StringAssert.Contains has 188 call sites and the bare form has none -- the rule is the same and the right answer is the opposite, which is why the count is worth taking before the suggestion is. DoesNotContain takes an expected value and a collection rather than a collection and a predicate, so the two assertions project the refusals to their kinds first, which reads more directly than the IsFalse-around-Any they replace. The other two are S3358, nested ternaries in the vocabulary, at major severity. Both predate the lift and moved with the file, which is why they are counted as new code; both are now named methods -- how far down an overload is bounded, and which forms a relationship asks for -- with the reasoning that was implicit in the nesting written out. Behaviour is unchanged: 1136 and 27 tests pass, and the generated output is still byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGCMUrT3jBgmANHNHcPmBf
matt-edmondson
added a commit
that referenced
this pull request
Sep 12, 2026
Address the SonarCloud findings from #219
This was referenced Sep 14, 2026
Damon3000s
pushed a commit
to Damon3000s/Semantics
that referenced
this pull request
Sep 15, 2026
`QuantityVocabulary` has resolved the metadata into the quantities and operators it describes since ktsu-dev#219, and refused the relationships whose exponents contradict their declared result. The C++ projection emitted from that resolution. The C# generator did not: it built its own type and operator lists by walking `dimensions.json` a second time, and called the vocabulary only to report SEM008 against operators it then emitted anyway. So `QuantitiesGenerator` now emits from `vocabulary.Types` and `vocabulary.Relationships`. One physics model, literally: which classes exist, what each one is and how it is bounded, and which operators relate them, all decided once and read by both targets. Two things stay on this side, and both are the language rather than the physics. A relationship becomes four C# operators — as declared, commuted, and the two divisions that undo it — because a caller who writes `duration * velocity` is not making a different claim; that expansion is `CollectOperators`. And an overload's `relationships` are C# expressions written in the metadata and pasted through, so there is nothing in them for a shared model to hold. **Breaking.** Sixteen operators and three methods are gone, being the five refused relationships in each direction C# spelled them: Sensitivity * Pressure -> VoltageMagnitude TorqueMagnitude * Angle -> Energy MomentOfInertia * AngularSpeed -> AngularMomentumMagnitude MomentOfInertia * AngularAccelerationMagnitude -> TorqueMagnitude Force{2,3,4}D.Dot(Displacement{2,3,4}D) -> Energy That is the whole of the generated diff: 105 deletions across 13 files and not one line added. `docs/migration-guide-5.0.md` says what to write instead of each, and why the fix is a physics call rather than a spelling one in all five cases. SEM008 still reports them and now says no operator is generated; `UnkeepableRelationshipTests` asserts their absence from the compiled surface by reflection, because a call that does not compile cannot be written down. Along the way: - `VocabularyIssue` carries an `IssueSite` — the names, the kind and the form — so SEM001, SEM002, SEM003 and SEM008 are all reported from the vocabulary's refusals, each with the location it had before, rather than from a second walk that reported what it dropped. - `Missing` checks each participant at the form it actually has to supply. The right operand of a product is the magnitude the vector is scaled by, so what it needs is a `vector0` however many components the relationship asks for; checking it at the declared form could name no participant at all. - The SEM003 fixtures declare real physics (`Length x Force -> Torque`). They multiplied two lengths into a length, which the vocabulary refuses for its exponents before it ever reaches the question of forms — so they would have reported SEM008 and never SEM003. They now assert which participant the message names, not merely that the name appears in it. - Both readers derive the strict-positive floor from the constraint's value rather than from the presence of the object holding it, so a constraint of another kind will not silently turn it on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGCMUrT3jBgmANHNHcPmBf
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.



Lifts
QuantityVocabularyout ofSemantics.Cppso the C# generator runs the same physics model.What this found
Only the C++ projection knew whether a declared relationship was dimensionally true — it needed the exponents to write
Quantity<D>, so it could not avoid multiplying them out. The C# generator checked that a relationship's names resolved (SEM001) and that its forms existed (SEM003), and then emitted the operator.So
Sensitivity * Pressure -> ElectricPotentialhas been shipping asSensitivityisM⁻¹L⁻¹T²IandPressureisML⁻¹T⁻², so their product isL⁻²I— butElectricPotentialisML²T⁻³I⁻¹. It compiles, it runs, and it computes the wrong physics. C++ has refused it by name since the projection existed; CLAUDE.md documents it as a pre-existing metadata bug.With the check shared, the C# side now reports all five of the relationships CLAUDE.md lists, verbatim from the build:
Shape
QuantityVocabulary,DimensionVectorand the resolution around them move toSemantics.Vocabulary. Both generators keep their own reader ofdimensions.json— they differ in what they carry beyond the physics, which is the pointQuantityMetadata's remarks already made — and each projects onto a smallDimensionDeclarationshape the vocabulary owns. That is one ~50-line adapter per side.DimensionVector.ToCppstayed behind as an extension inSemantics.Cpp. Writing a dimension as a C++ template argument list is that target's business.Shared source, not a project, argued at length in
Semantics.Vocabulary/README.md. Briefly:Semantics.SourceGeneratorsis a Roslyn component that bundles its dependencies for analysis-time loading, and its.csprojalready carries a long comment about those bundled facades colliding with in-box types; adding an assembly to that bundle is the kind of thing that breaks subtly. AndSemantics.Cppis packed, so a project reference becomes a published NuGet package — a public surface commitment for what is so far an implementation detail. Nothing passes these types across a boundary and no project references both consumers, so two compiled copies cost nothing and there is noCS0433risk.The price is real and is documented at each site: the files compile under two analysis configurations, so everything in them stays inside netstandard2.0's surface — no
System.HashCode, no range expressions, nostring.Contains(char). If a third consumer appears, this should become a project and take the packaging decision on its merits.Three judgement calls, all reversible
SEM008 reports; it does not drop the operator. Removing
Sensitivity * Pressurefrom a shipped package is a breaking change nobody asked for, and fixing the metadata is a physics call CLAUDE.md already declines to guess at. Making it drop, matching C++, is a one-line change — say the word.SEM008 is suppressed in
Semantics.Quantitiesonly.DiagnosticCatalogoffersWarningandErrorand nothing quieter, and ktsu.Sdk setsTreatWarningsAsErrors, so a diagnostic here cannot be both visible and non-fatal. Three of the five are provably unfixable by choosing exponents (the r×F versus τ·θ contradiction), so leaving it on would mean the repo does not build until a physics decision is made. The suppression is scoped to that one project with a comment naming the five, so a consumer editing metadata still sees the warning.UnkeepableRelationshipTestspins the set to exactly those five, so a sixth fails there rather than disappearing into the suppression — and so does a fourth, which would mean somebody fixed one and the list and theNoWarnshould shrink together.One invariant changed
TheRealMetadataReportsNothingandEveryGenerator_EmitsSourcesWithTheCanonicalHeaderasserted that the real metadata produced no generator diagnostics at all. That was true only because nothing was checking. Both now assert nothing unexpected, excluding SEM008 by identifier, with the set pinned by the test above. The first is renamed to match.Verification
dotnet buildanddotnet build -c Release— clean, 0 warnings, every target frameworkSemantics.Test: 1136 tests, 0 failed (1128 passed, 8 Windows-only skipped)Semantics.Cpp.Test: 27 passed, including the tests that compile the whole generated vocabulary with realg++/clang++git statusonSemantics.Quantities/Generated/is empty, so the lift moved the code without moving the behaviourNot in scope
The C# generator still builds its own type and operator lists for emission; only the check is shared so far. Making
QuantityVocabulary.Typesdrive C# emission is the larger half of "one physics model" and would risk changing committed generator output, so it is worth doing on its own.🤖 Generated with Claude Code
https://claude.ai/code/session_01QGCMUrT3jBgmANHNHcPmBf
Generated by Claude Code