Skip to content

Generated logarithmic conversions still null-check a parameter that is now a value type, boxing on every call #217

Description

@matt-edmondson

What happens

Commit 4ff1d42"[major] A quantity is a value type" (2026-09-09) — turned all 212 generated quantities into readonly record struct, explicitly to remove allocations (FromMeter 48.0 -> 0.0 bytes/op).

It did not touch LogarithmicScalesGenerator.cs, which still emits a reference-type guard:

// Semantics.SourceGenerators/Generators/LogarithmicScalesGenerator.cs:149-152
cb.WriteLine($"public static {fullType} {fromName}({linear}<T> linear)");
using (new Scope(cb))
{
    cb.WriteLine("ArgumentNullException.ThrowIfNull(linear);");
    cb.WriteLine("double linearValue = double.CreateChecked(linear.Value);");

linear is Concentration<T> / Gain<T> / Ratio<T> / SoundPressure<T> and friends — all now value types. Confirmed:

// Semantics.Quantities/Generated/…/Concentration.g.cs:12
public readonly partial record struct Concentration<T> : IVector0<Concentration<T>, T>, IPhysicalQuantity<Concentration<T>, T>

ArgumentNullException.ThrowIfNull(object?, string?) therefore takes an implicit boxing conversion of a non-nullable value type and compares the box to null.

Failure scenario

The check can never fire — it is unreachable on every one of the nine emitted sites: PH.g.cs:39, Decibels.g.cs:39 and :61, plus Cents, Semitones, SoundPressureLevel, SoundIntensityLevel, SoundPowerLevel, DirectionalityIndex.

The compiler emits a box for each call, so every Decibels.FromGain(...), SoundPressureLevel.FromSoundPressure(...) and so on carries an allocation on the hot path — precisely the per-call allocation the value-type commit was written to eliminate, and the one its benchmark suite does not cover for these types.

Why it matters

Two things, neither large on its own:

  • It is dead code that reads as a live contract. A consumer reading the generated source reasonably concludes null is a case worth handling here.
  • It is a measurable regression against the stated goal of the most recent major change, sitting in committed generated source that ships as written.

Suggested fix

Delete LogarithmicScalesGenerator.cs:151. If a guard is wanted for a future reference-typed linear side, emit it conditionally on the linear type actually being a reference type.

Extend QuantityValueTypeTests' allocation assertions to cover at least one logarithmic conversion, so the generator and the value-type decision cannot drift apart again — that is what let this survive the original commit.

Activity

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

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions