From 0f8b41a7d4b4dcef53b237ac50e8d7f5235781f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 00:27:36 +0000 Subject: [PATCH] State the strict-floor rule once, and pin it [patch] Both readers of dimensions.json decided for themselves which physicalConstraints value opts a V0 overload into the stricter floor, by each carrying a copy of `MinExclusive == "0"`. The copies agree today. They did not always: the C++ reader used to test the constraints object for null instead, so an overload declaring `{}` or any floor other than "0" would have been strictly positive in C++ and non-negative in C#, and nothing would have caught it. Move the rule into OverloadDeclaration.IsStrictFloor, which both readers now ask rather than restate, and cover the cases that separate it from the presence test: an empty constraints object, and a floor of some other value. Both new cases fail against the old rule; the rest of the suite does not, which is the gap. The real metadata declares no such constraint yet, so these are driven by a document written for the purpose -- the point is to fail on the day it does. Fixes ktsu-dev/Semantics#218 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WJpXUy8JcVGpjDmK1MMHKb --- Semantics.Cpp.Test/StrictFloorTests.cs | 119 ++++++++++++++++++ Semantics.Cpp/MetadataProjection.cs | 10 +- .../Models/VocabularyProjection.cs | 11 +- Semantics.Vocabulary/DimensionDeclaration.cs | 29 ++++- 4 files changed, 161 insertions(+), 8 deletions(-) create mode 100644 Semantics.Cpp.Test/StrictFloorTests.cs diff --git a/Semantics.Cpp.Test/StrictFloorTests.cs b/Semantics.Cpp.Test/StrictFloorTests.cs new file mode 100644 index 0000000..3ffef3a --- /dev/null +++ b/Semantics.Cpp.Test/StrictFloorTests.cs @@ -0,0 +1,119 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Semantics.Cpp.Test; + +using System; + +using ktsu.Semantics.Cpp; +using ktsu.Semantics.Vocabulary; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers which declared constraint opts a V0 overload into the stricter floor, which is the one +/// rule about physicalConstraints that both projections of dimensions.json read. +/// +/// +/// The rule is that minExclusive: "0" specifically opts in, and it is the +/// documented one: CLAUDE.md's design decision #4 says the strict guard is what that value asks +/// for. A reader that tested whether a constraints object was present instead answered the same +/// for the three sites the metadata declares today -- Wavelength, Period and +/// HalfLife, all { "minExclusive": "0" } -- and would have answered differently for +/// the first constraint of any other kind, giving one declared quantity two domains depending on +/// which language it was generated into. That is ktsu-dev/Semantics#218. +/// +/// So these are driven by a metadata document written for the purpose rather than by the real one: +/// the case that separates the two rules is a constraint the real file does not declare yet, and +/// the point is to fail on the day it does. +/// +/// +[TestClass] +public sealed class StrictFloorTests +{ + /// + /// One dimension, whose magnitude form carries an overload per way of declaring -- or not + /// declaring -- a floor. Only ZeroFloor asks for the strict one. + /// + private const string Metadata = """ + { + "physicalDimensions": [ + { + "name": "Length", + "dimensionalFormula": { "length": 1 }, + "quantities": { + "vector0": { + "base": "Length", + "overloads": [ + { "name": "NoConstraints", "description": "Declares no constraints at all." }, + { "name": "EmptyConstraints", "description": "Declares a constraints object with no floor in it.", "physicalConstraints": { } }, + { "name": "OtherFloor", "description": "Declares a floor that is not zero.", "physicalConstraints": { "minExclusive": "1" } }, + { "name": "ZeroFloor", "description": "Declares the floor that opts into the strict guard.", "physicalConstraints": { "minExclusive": "0" } } + ] + } + } + } + ] + } + """; + + private static CppQuantityOutput Output { get; } = new CppQuantityGenerator( + new CppQuantityOptions { Namespace = "holo" }).Generate(QuantityMetadata.Parse(Metadata)); + + /// + /// The value the rule is written around gets the strict comparison, which is the behaviour the + /// three real constraint sites rely on. + /// + [TestMethod] + public void GuardsAZeroFloorStrictly() => + Assert.Contains("assert(value.count() > 0", Output.Files["ZeroFloor.hpp"], StringComparison.Ordinal); + + /// + /// A constraints object with no floor in it is not an opt-in. MinExclusive defaults to + /// empty, so a reader testing the object for null rather than reading its value turns the strict + /// guard on here -- and the C# generator, which reads the value, leaves it off. + /// + [TestMethod] + public void DoesNotGuardEmptyConstraintsStrictly() => + Assert.Contains( + "assert(value.count() >= 0", + Output.Files["EmptyConstraints.hpp"], + StringComparison.Ordinal); + + /// + /// Neither is a floor of some other value. Nothing about minExclusive: "1" says a + /// quantity may not be zero -- it says considerably more than that -- and no guard for it is + /// emitted, so the strict comparison would be an assertion the metadata never asked for. + /// + [TestMethod] + public void DoesNotGuardANonZeroFloorStrictly() => + Assert.Contains("assert(value.count() >= 0", Output.Files["OtherFloor.hpp"], StringComparison.Ordinal); + + /// + /// An overload with no constraints at all keeps its magnitude form's own floor, as does the base + /// it refines. This is the case the two rules always agreed about. + /// + [TestMethod] + public void GuardsAnUnconstrainedOverloadAsAMagnitude() + { + Assert.Contains("assert(value.count() >= 0", Output.Files["NoConstraints.hpp"], StringComparison.Ordinal); + Assert.Contains("assert(value.count() >= 0", Output.Files["Length.hpp"], StringComparison.Ordinal); + } + + /// + /// The rule itself, stated once and asked by both readers. Testing it here rather than through + /// each projection is the point of it being one function: the C# generator and this one cannot + /// answer differently, because there is no longer a second answer for them to hold. + /// + [TestMethod] + [DataRow("0", true, "the documented opt-in")] + [DataRow(null, false, "no constraints object at all")] + [DataRow("", false, "a constraints object carrying no floor")] + [DataRow("1", false, "a floor of some other value")] + [DataRow("0.0", false, "a floor that is zero but is not spelled the way the rule names it")] + [DataRow(" 0", false, "a floor that would only match if it were trimmed first")] + public void ReadsTheStrictFloorOffTheValue(string? minExclusive, bool expected, string because) => + Assert.AreEqual( + expected, + OverloadDeclaration.IsStrictFloor(minExclusive), + $"minExclusive {minExclusive ?? "(null)"} is {because}."); +} diff --git a/Semantics.Cpp/MetadataProjection.cs b/Semantics.Cpp/MetadataProjection.cs index 97b1dc1..84f6863 100644 --- a/Semantics.Cpp/MetadataProjection.cs +++ b/Semantics.Cpp/MetadataProjection.cs @@ -41,10 +41,14 @@ [.. dimension.DotProducts.Select(Relationship)], // A constraint is carried as the flag the vocabulary reads rather than as its value: only a // strict-positive floor is declared anywhere, and what the vocabulary needs is whether it is // there. The value itself is the C# generator's business, which is where the guard is emitted. - // Read off the constraint's value rather than off the presence of the object holding it, so a - // constraint of some other kind, when one is added, does not silently turn the strict floor on. + // Which values mean "strict" is the vocabulary's rule rather than this reader's, so it is asked + // rather than restated: reading the presence of the object instead of its value is #218, and a + // copy of the rule per reader is how the two projections got to hold different ones. private static OverloadDeclaration Overload(MetadataOverload overload) => - new(overload.Name, overload.Description, overload.PhysicalConstraints?.MinExclusive == "0"); + new( + overload.Name, + overload.Description, + OverloadDeclaration.IsStrictFloor(overload.PhysicalConstraints?.MinExclusive)); private static RelationshipDeclaration Relationship(MetadataRelationship relationship) => new(relationship.Other, relationship.Result, [.. relationship.Forms]); diff --git a/Semantics.SourceGenerators/Models/VocabularyProjection.cs b/Semantics.SourceGenerators/Models/VocabularyProjection.cs index 6f575a7..32b72b3 100644 --- a/Semantics.SourceGenerators/Models/VocabularyProjection.cs +++ b/Semantics.SourceGenerators/Models/VocabularyProjection.cs @@ -46,11 +46,14 @@ [.. dimension.DotProducts.Select(Relationship)], // The constraint is carried as the flag the vocabulary reads rather than as its value: what the // vocabulary needs is whether a stricter floor is declared. The value itself stays here, where - // the Vector0Guards.EnsurePositive call is emitted from — and the flag is read off that value - // rather than off the presence of the object holding it, so a constraint of some other kind, - // when one is added, does not silently turn the strict floor on. + // the Vector0Guards.EnsurePositive call is emitted from — and which values mean "strict" is the + // vocabulary's rule rather than this reader's, so it is asked rather than restated: a copy of + // the rule per reader is how the two projections came to hold different ones in #218. private static OverloadDeclaration Overload(OverloadDefinition overload) => - new(overload.Name, overload.Description, overload.PhysicalConstraints?.MinExclusive == "0"); + new( + overload.Name, + overload.Description, + OverloadDeclaration.IsStrictFloor(overload.PhysicalConstraints?.MinExclusive)); private static RelationshipDeclaration Relationship(RelationshipDefinition relationship) => new(relationship.Other, relationship.Result, [.. relationship.Forms]); diff --git a/Semantics.Vocabulary/DimensionDeclaration.cs b/Semantics.Vocabulary/DimensionDeclaration.cs index fa7b59a..a46c05f 100644 --- a/Semantics.Vocabulary/DimensionDeclaration.cs +++ b/Semantics.Vocabulary/DimensionDeclaration.cs @@ -2,6 +2,7 @@ namespace ktsu.Semantics.Vocabulary; +using System; using System.Collections.Generic; /// @@ -66,7 +67,33 @@ internal sealed record FormDeclaration(string Base, IReadOnlyList -internal sealed record OverloadDeclaration(string Name, string Description, bool IsStrictlyPositive); +internal sealed record OverloadDeclaration(string Name, string Description, bool IsStrictlyPositive) +{ + /// + /// The one physicalConstraints.minExclusive value that opts an overload into the stricter + /// floor. + /// + internal const string StrictFloor = "0"; + + /// + /// Decides whether a declared physicalConstraints.minExclusive opts into the stricter + /// floor. + /// + /// + /// The value the overload declares, null when it declares no constraints at all, and empty when + /// it declares a constraints object without this field. + /// + /// Whether the overload is strictly positive. + /// + /// One implementation because there are two readers. The rule is that + /// minExclusive: "0" specifically opts in -- not that constraints are present -- + /// so a constraint of some other kind, when one is added, does not silently turn the strict + /// floor on in one projection and leave it off in the other. Reading the presence of the object + /// instead is ktsu-dev/Semantics#218, and a copy of the rule per reader is how it got there. + /// + internal static bool IsStrictFloor(string? minExclusive) => + string.Equals(minExclusive, StrictFloor, StringComparison.Ordinal); +} /// One declared relationship between dimensions. /// The dimension on the other side of the operator.