Repository navigation
Conditional compilation for feature subsets: use trait-based test selection and package seams instead #783
Description
Activity
Adopted. I would close this. Both alternatives this proposed have landed, and the argument against
#iffor feature subsets should be treated as settled.- Trait-based test selection:
[Trait("Area", …)]went from 0 to 230 of 252 test files, 12
area values, over 3,010Fact/Theory. - The trimming/AOT gate:
TrimmingAndAot.ymlpublishes and runsAotSmokeTeston three
operating systems;Trimming.mddocuments it; it passes on master today. - The reflection surface is now zero. The only
GetMethod/Activator.CreateInstance/
MakeGenericTypeoccurrences left in the kernel are inside comments explaining why the table is
written out instead.
Two riders, both measured, and both are follow-ups rather than objections:
1. The traits are not used by anything. No workflow passes
--filter;CSharpTestruns the whole
suite across an OS matrix. Nothing enforces the attribute either, so the 22 files without one will
become more. Either shard CI by area or say plainly that the benefit is local-only — at the moment the
mechanism is in place and unexercised.2. The TFM allowance now carries a public capability, unmeasured. Serialization is
#if NET8_0_OR_GREATERandGenericMathisCompile Removed below net7.UnitTeststargets
net10.0only, soPublicApi.txtrecords one leg and the netstandard2.0 public surface is checked
by nothing. That is this issue's own "unbuilt configurations rot silently", arriving through the one
construct it permits. A per-TFM public-surface check closes it, and it is the more valuable of the two.Neither of those is a reason to keep this open — they are new, smaller issues, and I would rather file
them than leave a settled proposal open to carry them.- Trait-based test selection:
Closing as adopted. Both alternatives this proposed have landed: trait-based selection is on 230 of 252 test files over 3,010 facts, and the trimming/AOT gate runs
AotSmokeTeston three operating systems. Kernel reflection is zero.The two riders in the comment above are follow-ups rather than unfinished parts of this, and I am filing them separately: nothing consumes the traits yet, and the netstandard2.0 public surface is checked by nothing.
A recurring suggestion is to guard subsets of the library with conditional
compilation symbols, so that CI can test less and consumers can omit parts they
do not need. This issue argues against
#ifas that mechanism, and proposes thetwo things that do address the underlying goals.
I could not find an existing issue for this. The nearest ones are
#539 (trimming, but only as an obstacle to a
single-file
AngouriMath.Terminal),#363 (AOT-supported Linq compilation) and
#746, whose modularity section commits to
separate packages plus trimming/AOT-safety and never to
#if.Measurements
Taken on
masterat the time of writing:Sources/AngouriMathnet7.0;netstandard2.0Compile Remove="Core/Entity/GenericMath/**"below net7.0)UnitTests[Trait]attributes inUnitTestsAssembly.GetExecutingAssembly().GetName().VersioninCore/Exceptions/SysExceptions.csWhy
#ifdoes not serve either goalTest time. Conditional compilation does not select tests, it selects code.
If a symbol removes part of the library, the test project must compile in a
matching configuration, so CI builds and runs N configurations instead of one.
Wall-clock goes up. What selects tests is
dotnet test --filter, and it hasnothing to filter on today because the suite carries no traits.
Consumer footprint. NuGet ships one compiled binary. A consumer restoring
AngouriMathcannot set our compilation symbols; only someone building fromsource can. Making subsets reachable from NuGet means publishing N packages —
at which point the boundary is an assembly boundary and the
#ifis redundant.Correctness, which is the specific objection for a CAS. Our pipelines are
order-dependent: limit rules must be appended at a particular point in
LimitSolvers, and solver ordering has already produced timeouts where acorrect rule was starved by a search in front of it. Removing a rule from a
chain therefore changes the answers to queries that were not about that rule. A
subset build is not "the same library minus a feature"; it is a different
simplifier, and it would have to be tested as a product in its own right to be
documented honestly. With N symbols that is 2^N configurations against a CI that
builds one, so unbuilt configurations rot silently. That is the failure mode to
avoid, and it arrives gradually — each individually reasonable
#ifaddedalongside a feature is what accumulates it.
Proposed instead
1. Trait-based test selection. Add
[Trait("Area", "...")]as files aretouched, so
dotnet test --filtercan run a subset locally and CI can shard.No runtime cost, no build matrix, no behaviour change. This is the part that is
safe to trickle in per-PR.
2. Keep the seams package-ready, per feature. New domains behind an
interface with no core references by name; no static cross-area registries or
assembly scanning (#746 identifies these as what breaks trimming and NativeAOT);
internalunless the public API requires otherwise. Done consistently, splittinga domain such as number theory or statistics into its own package later is a
repackaging rather than a rewrite.
3. Land the trimming/AOT smoke test (roadmap item 79 of #746): publish a
small sample app with
PublishTrimmed, so regressions surface on the PR thatcauses them. Worth noting that the library is closer to trim-ready than #363
suggests — the reflection surface is a single call site for the assembly
version. The remaining obstacles are
System.Linq.ExpressionsinCompile,which is an AOT problem rather than a trimming one, and the 4,863-line generated
ANTLR parser, which is retained for as long as
MathS.FromStringis reachable.The one place
#ifstays appropriate is where it already is: target-frameworkshims.
Opinions welcome, particularly from anyone who has a consumer scenario where
per-feature source-level exclusion would actually be usable.