Repository navigation
A node type cannot be defined outside the kernel assembly: five of Entity's abstract members are internal #1026
Description
Activity
Written up as
Docs/Contributing/NodeContract.mdin #1040, and the CS0534 is reproduced there from an actual build rather than asserted — exactly five errors, naming those five members, from a class library referencing the builtAngouriMath.dlland implementing all six reachable members.The decision is to open the contract without publishing anything. Four of the five are engine machinery with a default the kernel can supply and be right about —
Priorityis read only underFunctions/Output/and 27 node types already inheritPriority.Func;SortHashNameis a deduplication table over kernel node kinds (Sumf/Minusfboth hashsumminus_) that an external node has no partner in;ToSymPyalready throwsNotSufficientlySupportedExceptionfrom a base class; an empty inversion is what 17 of the 60 implementations already return. OnlyIntrinsicConditionis genuine contract: it is the one read by apublicmember, andBoolean.Trueis the positive claim that an operation is total, which for an unknown node is the unsafe direction. So it becomesprotected abstractand the other four getvirtualdefaults — publishing them instead would draginternal enum Priority(30|-composed values) andinternal enum SortLevelinto the public API for the same capability.Two things the issue could not have known. The hierarchy is already open:
Number.Complex,Real,RationalandVariableare concrete and unsealed, andrecord MyReal(EDecimal D) : Entity.Number.Real(D)compiles against the nupkg today and goes throughSimplifyandSolveintact — so closedness is already only a convention, which is most of Option B's case. And there is a twelfth obligation written nowhere: all 68 node types carryToString() => Stringize(), and the five defaults run through theEveryNodeSurvivesEveryPipelineTestpipeline list on a node kind the kernel has never seen held on sixteen of seventeen — the seventeenth beingToStringrecursing through the record-synthesizedEntity.PrintMembersuntil the stack guard fires.Entityshould declare itsealedonce.Correcting my own framing above, from a measurement. The title says a node type cannot be
defined outside the kernel assembly. That is true for deriving directly fromEntity, and false as
a blanket claim.Four types in the hierarchy are concrete and unsealed, by exemption from the
SealedOrAbstract
analyzer —Entity.Number.Complex,Entity.Number.Real,Entity.Number.Rationaland
Entity.Variable, eachpublic partial recordwith nosealedand noabstract:Core/Entity/Continuous/Entity.Continuous.Complex.Definition.cs:19: public partial record Complex : Number Core/Entity/GenericMath/Entity.GenericMath.Classes.cs:29: partial record Real : Core/Entity/GenericMath/Entity.GenericMath.Classes.cs:48: partial record Rational : Core/Entity/Omni/Entity.Variable.cs:24: public partial record Variable : EntitySo an external assembly can already derive from one of those and get a working node — the five
blocked members are inherited as implemented. The seam is narrower than the issue claims: what is
blocked is a node that is not a kind of number or variable, which is most of what a domain package
would want, but not all of it.Two consequences worth recording:
EveryNodeSurvivesEveryPipelineTestcannot see such a node. It enumerates
typeof(Entity).Assembly.GetTypes(), so an externally-derived type is outside its sweep, and the
guarantee that every node survives every pipeline does not extend to it. Whether it should is an
open question, not an answered one.- Whether those four being externally derivable is intended is unknown. They are exemptions from
an analyzer, which is a decision someone made about this assembly's internals; it may never have
been read as a public extension point. That wants a maintainer's answer.
The five non-public abstract members are still exactly as listed, verified again on current
master,
and a sixth sits below them:Set.SpecialSet.ToDomain()isinternal abstract, so a new special set
is blocked the same way.The design document answering this is #1040.
#1022's fix (PR #1047) adds two more to the table, taking it from five to seven:
member accessibility where StringizeNode()private protected abstractFunctions/Output/ToString.Definition.csLatexizeNode()private protected abstractFunctions/Output/ToString.Definition.csBoth are a direct consequence of the fix being right: the
domain(...)wrapper is one decision, so
it is taken once onEntityand each node renders only itself. That is the correct shape, and it is
the shape this issue is about — the per-node rendering that waspublic overrideand could in
principle have been supplied from outside is now a member no external type can implement.Recording it here because it moves the wall rather than merely touching it, and because it is worth
knowing that this seam is still being narrowed while the issue is open. Nothing in #1047 is worse for
an external node type than the existing five made it — a type that cannot implementPriorityis
already not compiling — but a fix for this issue now has seven members to answer, not five.One measured detail that may matter to the fix: it was because of those five that removing the 130
per-nodeStringize()/Latexize()overrides in #1047 was safe to do at all. No assembly outside the
kernel can have derived a node, so no outside override could be orphaned. Whatever opens this seam
will remove that argument along with the wall.The count in the title is wrong: it is eight, not five. By reflection over
typeof(Entity)
(Instance | Public | NonPublic | DeclaredOnly,IsAbstract) there are 12 abstract members, of
which eight are unreachable from another assembly:member accessibility since Priorityinternalthe original five SortHashName(SortLevel)private protectedIntrinsicConditionprivate protectedToSymPy()internalInvertNode(Entity, Entity)private protectedStringizeNode()private protected#1047 LatexizeNode()private protected#1047 DefaultCodomaininternal#1047 Plus
Set.SpecialSet.ToDomain(), which blocks a new special set only and is separable.
NodeContract.md's own table already listsDefaultCodomain; its prose still says five.Option A, member by member
Nothing of it is implemented — all eight are abstract at the accessibilities above, and 70
override string ToString() => Stringize();lines remain. Shipping it:Priority→internal virtual, defaulting toPriority.Func. Safe: nothing reads it outside
Functions/Output/.SortHashName→private protected virtual, the type name at every level.ToSymPy→internal virtual, throwingNotSufficientlySupportedException.InvertNode→private protected virtual, empty.IntrinsicCondition,StringizeNode,LatexizeNode,DefaultCodomain→protected abstract.
No correct default exists for a node's domain or its printed form, andDomainis already public.
So: four
protected abstractmembers, four virtual defaults, one sealedToString, and the compile
test from the acceptance criteria. The public surface gains four protected names.What it does and does not unblock
Does: an external node type — tier 9, #440, the first half of #338.
Does not: moving the LaTeX printer or the SymPy exporter out of the kernel. Those stay per-node
members consumed byEntity.Latexize()andToSymPy(); making them overridable does not relocate
the 68 kernel implementations. That needs a type-keyed registry or a visitor, which §4 rightly keeps
out of A. Worth saying because it is the reason #1008 was closed without moving any boundary.Also unsettled by A: 34
switchsites across 10 kernel files whose default arm throws, each
needing a decision about what an unknown node kind gets; and
EveryNodeSurvivesEveryPipelineTest, which sweeps one assembly and would not see a foreign node.Recommendation
Implement Option A amended to eight members, in one PR with the compile test — and enumerate
those 34 throwing defaults first, stating per site what an unknown node gets. That enumeration is the
measurement §7 says is missing, and without it "a node type can be defined outside the kernel" is
true at compile time and false in use.A smaller step worth taking first, on its own:
public sealed override string ToString() => Stringize();onEntity. It deletes 70 lines and an undocumented obligation, and it is independent
of everything above.A library can either be extensible in its data (classes) or operations (methods/functions). The former encourages inheritance or interface implementation against a fixed set of operations, the latter encourages matching on a fixed data hierarchy. To do both needs a beast of its own in the language like multiple dispatching (Julia) that .NET does not do.
Do we really want an extensible node that our solvers and simplifiers cannot handle effectively? Just tell anyone who needs a new node to PR upstream.
What is wrong
Entityispublic abstract partial record, and its constructor isprotected, which reads as aninvitation to derive from it. But a subclass in another assembly does not compile. Five of its
abstract members are
internalorprivate protected, so an external type can never satisfy them:Priorityinternal abstractCore/Entity/Entity.Definition.cs:378SortHashName(SortLevel)private protected abstractFunctions/TreeAnalyzer/Sort.Definition.cs:19IntrinsicConditionprivate protected abstractFunctions/Evaluation/Evaluation.Definition.cs:72ToSymPy()internal abstractFunctions/Output/ToSympy.Definition.cs:13InvertNode(Entity, Entity)private protected abstractFunctions/Continuous/Solvers/InvertNode.Definition.cs:30Deriving from
Entityin a separate assembly fails withCS0534, naming each of those five amongthe members it cannot implement. This is not a policy that could be relaxed by an
InternalsVisibleTo: a published extension package cannot be on that list.Why it matters, and what is behind it
This is the concrete blocker under several roadmap items, and it is not the one they name.
#746 item 56 says the extensibility seam
is #338 — looking a type up so it can be
parsed from a string. That is the second half. The first half is being able to write the type at
all, and today you cannot. The same wall sits under item 53 (groups, rings and fields as
first-class, #440) and under the whole
tier-9 domain-package idea, which assumes a geometry or statistics pack can contribute node types.
It also means one thing the packaging decision cannot decide.
Docs/Contributing/Packaging.md(#746 item 78) works out which capabilities belong in the kernel and which ship separately, and its
answer for domain packages is blocked, not decided — because a package boundary is not what is
in the way here. Moving the LaTeX printer or the SymPy exporter out of the kernel runs into the same
five members from the other side: they are abstract members of
Entity, so moving them means eitherthis contract becoming extensible or a visitor over all 68 node types.
Scope
This is a design decision before it is an implementation, and it should be argued before it is
built. The shape of the question:
that a visitor or a registry should own instead?
ToSymPyandSortHashNamelook like the secondkind;
PriorityandIntrinsicConditionlook like the first.publicorprotected, it becomes a compatibility surface — every futurechange to
SortLevel,Priorityor theInvertNodesignature is then breaking. That is the realcost and it should be stated before the change, not discovered after.
that carries a name and an operation table — so that a domain package contributes data rather than
a subclass? That keeps
Entityclosed, which is what makes exhaustive matching and theEveryNodeSurvivesEveryPipelineTestreflection sweep work today.scanning. See #363,
#552 and
Docs/Contributing/Trimming.md.Acceptance criteria
Docs/Contributing/, saying what a node type outside the kernel may and maynot do, and which of the five members are part of the contract.
parse is not required, but
ToString,Latexize,InnerSimplified, substitution and structuralequality are. A test that only asserts the type exists proves nothing; this has to build.
EveryNodeSurvivesEveryPipelineTeststill passes, and it is stated whether an external node isin scope for it.
Dependencies
Independent of everything currently open. It does not depend on
#338 — it is what #338 needs, not the
reverse. It is a prerequisite for #746 items 53 and 56 and for tier 9.
Found while measuring package boundaries for #746 item 78.