Unify associatedtype constraint representation; make autodiff requirement access order-agnostic#11368
Unify associatedtype constraint representation; make autodiff requirement access order-agnostic#11368csyonghe wants to merge 3 commits into
Conversation
…ment access order-agnostic Model `associatedtype A : IFoo` identically to `associatedtype A where A : IFoo`: both surface forms relocate to the enclosing interface as sibling constraint requirements (mirroring how a generic parameter and its constraint are parallel members of the enclosing generic). Add the derived-interface `__constraint` form that produces the same representation. To make this work for the built-in differentiable interfaces, introduce a hoistable `IRBuiltinRequirementKey(kind)` so built-in interface requirements are deduplicated by role (not by name/identity), and convert the auto-diff backend to address `IBackwardDifferentiable`/`IBwdCallable` requirements by role instead of by hardcoded operand index.
csyonghe
requested review from
bmillsNV and
Copilot
and removed request for
a team
📝 WalkthroughWalkthroughImplements interface-level ChangesInterface-Level Constraints
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR unifies the internal representation of associated-type constraints so that associatedtype A : IFoo, associatedtype A where A : IFoo, and derived-interface __constraint declarations all lower/check as interface-level requirements. It also updates the IR + autodiff pipeline to identify built-in interface requirements by role (via a hoistable IRBuiltinRequirementKey) rather than by decl identity or operand order, and adds regression tests for derived-interface associated-type constraints.
Changes:
- Parser: route associated-type
:/whereconstraints to the enclosing interface and add__constraintinterface-body declarations. - Front-end + IR: validate and surface interface-level constraints, introduce hoistable built-in requirement keys, and lower relocated subtype constraints with correct witness-table-typed requirement values.
- Autodiff: replace index-based interface requirement access with role-based lookup and adjust linking/emission behavior for built-in requirement keys; add new interface constraint tests.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/language-feature/interfaces/derived-interface-constraint.slang | New interpreter test demonstrating __constraint DataType == This enabling T.DataType ≡ T. |
| tests/language-feature/interfaces/derived-interface-constraint-deep.slang | New interpreter test verifying constraints remain visible through multi-level interface inheritance. |
| tests/language-feature/interfaces/derived-interface-constraint-error.slang | New diagnostic test ensuring violated derived-interface constraints are rejected. |
| source/slang/slang-parser.cpp | Adds __constraint parsing and unifies associated-type constraint attachment at interface scope. |
| source/slang/slang-check-decl.cpp | Validates interface-level GenericTypeConstraintDecl requirements during conformance checking. |
| source/slang/slang-check-inheritance.cpp | Surfaces interface-level constraints to associated-type accesses during inheritance/base computation. |
| source/slang/slang-ast-decl-ref.cpp | Resolves built-in associated-type constraint identity from constraint subject (not parent decl). |
| source/slang/slang-ast-support-types.h | Extends BuiltinRequirementKind with witness/conformance roles needed for relocated constraints. |
| source/slang/slang-ir-insts.lua | Adds BuiltinRequirementKey and BuiltinRequirementDecoration IR constructs. |
| source/slang/slang-ir-insts.h | Implements IR builder support for built-in requirement keys + decoration. |
| source/slang/slang-ir-insts-stable-names.lua | Assigns stable-name IDs for the new IR opcode/decoration. |
| source/slang/slang-lower-to-ir.cpp | Switches interface requirement key caching to IRInst*, creates hoistable built-in keys, and fixes lowering for relocated subtype constraints. |
| source/slang/slang-ir-link.cpp | Prevents built-in requirement keys from entering mangled-name keyed deferred cloning; eagerly clones their witness-table entries. |
| source/slang/slang-emit-c-like.cpp | Skips emitting kIROp_BuiltinRequirementKey as metadata-only global insts. |
| source/slang/slang-ir-autodiff.h | Updates autodiff shared context key types to IRInst* and declares role-based entry lookup helper. |
| source/slang/slang-ir-autodiff.cpp | Uses role-based requirement entry lookup when initializing autodiff shared context. |
| source/slang/slang-ir-autodiff-fwd.cpp | Replaces index-based backward-diff requirement access with role-based lookup in forward-diff lowering. |
| auto diffTypeEntry = getInterfaceEntryByBuiltinRequirement( | ||
| differentiableInterfaceType, | ||
| BuiltinRequirementKind::DifferentialType); | ||
| auto diffWitnessEntry = getInterfaceEntryByBuiltinRequirement( | ||
| differentiableInterfaceType, | ||
| BuiltinRequirementKind::DifferentialWitness); | ||
| auto zeroEntry = getInterfaceEntryByBuiltinRequirement( | ||
| differentiableInterfaceType, | ||
| BuiltinRequirementKind::DZeroFunc); | ||
| auto addEntry = getInterfaceEntryByBuiltinRequirement( | ||
| differentiableInterfaceType, | ||
| BuiltinRequirementKind::DAddFunc); | ||
|
|
| auto ptrTypeEntry = getInterfaceEntryByBuiltinRequirement( | ||
| differentiablePtrInterfaceType, | ||
| BuiltinRequirementKind::DifferentialPtrType); | ||
| auto ptrWitnessEntry = getInterfaceEntryByBuiltinRequirement( | ||
| differentiablePtrInterfaceType, | ||
| BuiltinRequirementKind::DifferentialPtrWitness); | ||
|
|
||
| differentialAssocRefTypeStructKey = ptrTypeEntry->getRequirementKey(); | ||
| differentialAssocRefTypeWitnessStructKey = ptrWitnessEntry->getRequirementKey(); | ||
| differentialAssocRefTypeWitnessTableType = | ||
| cast<IRWitnessTableType>(ptrWitnessEntry->getRequirementVal()); | ||
|
|
||
| isPtrInterfaceAvailable = true; | ||
| } |
| auto bwdDiffContextTypeReqKey = getInterfaceEntryByBuiltinRequirement( | ||
| bwdDiffInterfaceType, | ||
| BuiltinRequirementKind::BwdCallableContextType) | ||
| ->getRequirementKey(); | ||
|
|
| auto bwdCallableOpReqKey = getInterfaceEntryByBuiltinRequirement( | ||
| bwdCallableInterfaceType, | ||
| BuiltinRequirementKind::BwdCallablePropFunc) | ||
| ->getRequirementKey(); |
| builder.createWitnessTableEntry( | ||
| newWitnessTable, | ||
| as<IRInterfaceRequirementEntry>(baseConformanceType->getOperand(0))->getRequirementKey(), | ||
| getInterfaceEntryByBuiltinRequirement( | ||
| baseConformanceType, | ||
| BuiltinRequirementKind::BwdCallableContextType) | ||
| ->getRequirementKey(), | ||
| contextType); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/slang/slang-lower-to-ir.cpp (1)
11708-11749:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep interface equality constraints witness-table typed.
Line 11729 falls back to
ensureDecl()for direct interfaceGenericTypeConstraintDecls. For__constraint A == B, that returns the requirement key, so the entry ends up withKeyTypeinstead ofWitnessTableType(B), andremoveLinkageDecorations()mutates the key itself. Equality constraints should take the same witness-table path as the relocated:constraints.Proposed fix
- else if ( - relocatedSubtypeConstraint && - !relocatedSubtypeConstraint.getDecl()->isEqualityConstraint) + else if (relocatedSubtypeConstraint) { - // A subtype constraint on an associated type (`associatedtype A : IBar`, - // `associatedtype A where A : IBar`) is recorded in the unified - // representation as an interface-level requirement (a sibling of `A`). It is - // a *conformance* requirement: its witness is a witness table for the bound. - // We must lower it the same way the conformance was lowered when the bound was - // nested in the associated type (see the associated-type constraint loop - // below), i.e. with a `WitnessTableType` requirement value -- otherwise - // consumers (e.g. autodiff) that read the requirement get a malformed entry. - // (Equality constraints, e.g. `__constraint A == B`, are handled by the - // generic path below.) + // Interface-level type constraints are witness-valued requirements. + // Equality constraints also lower to a `TypeEqualityWitness`, whose + // IR type is `WitnessTableType(sup)`. auto irBaseType = lowerType( subContext, getSup(subContext->astBuilder, relocatedSubtypeConstraint)); entry->setRequirementVal(subBuilder->getWitnessTableType(irBaseType)); }As per coding guidelines, "IR pass correctness — ensure SSA form and type invariants are maintained."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 288891e2-8f67-40c1-b931-a041f5df5ac1
📒 Files selected for processing (17)
source/slang/slang-ast-decl-ref.cppsource/slang/slang-ast-support-types.hsource/slang/slang-check-decl.cppsource/slang/slang-check-inheritance.cppsource/slang/slang-emit-c-like.cppsource/slang/slang-ir-autodiff-fwd.cppsource/slang/slang-ir-autodiff.cppsource/slang/slang-ir-autodiff.hsource/slang/slang-ir-insts-stable-names.luasource/slang/slang-ir-insts.hsource/slang/slang-ir-insts.luasource/slang/slang-ir-link.cppsource/slang/slang-lower-to-ir.cppsource/slang/slang-parser.cpptests/language-feature/interfaces/derived-interface-constraint-deep.slangtests/language-feature/interfaces/derived-interface-constraint-error.slangtests/language-feature/interfaces/derived-interface-constraint.slang
| else if (auto constraintDecl = as<GenericTypeConstraintDecl>(requirementKey)) | ||
| { | ||
| auto assocFromExp = [](TypeExp const& exp) -> AssocTypeDecl* | ||
| { | ||
| if (auto declRefType = as<DeclRefType>(exp.type)) | ||
| return as<AssocTypeDecl>(declRefType->getDeclRef().getDecl()); | ||
| return nullptr; | ||
| }; | ||
| auto assoc = assocFromExp(constraintDecl->sub); | ||
| if (!assoc) | ||
| assoc = assocFromExp(constraintDecl->sup); | ||
| if (assoc) | ||
| { | ||
| builtinReq = assoc->findModifier<BuiltinRequirementModifier>(); | ||
| isConstraint = true; |
There was a problem hiding this comment.
Try both sides when recovering the builtin requirement.
This only falls back to sup when sub is not an associated type. If sub is a non-builtin assoc type and the builtin differential assoc type is on sup, the special-case returns nullptr, so A == Differential and Differential == A stop resolving equivalently.
Proposed fix
else if (auto constraintDecl = as<GenericTypeConstraintDecl>(requirementKey))
{
- auto assocFromExp = [](TypeExp const& exp) -> AssocTypeDecl*
+ auto builtinReqFromExp = [](TypeExp const& exp) -> BuiltinRequirementModifier*
{
if (auto declRefType = as<DeclRefType>(exp.type))
- return as<AssocTypeDecl>(declRefType->getDeclRef().getDecl());
+ if (auto assoc = as<AssocTypeDecl>(declRefType->getDeclRef().getDecl()))
+ return assoc->findModifier<BuiltinRequirementModifier>();
return nullptr;
};
- auto assoc = assocFromExp(constraintDecl->sub);
- if (!assoc)
- assoc = assocFromExp(constraintDecl->sup);
- if (assoc)
+ builtinReq = builtinReqFromExp(constraintDecl->sub);
+ if (!builtinReq)
+ builtinReq = builtinReqFromExp(constraintDecl->sup);
+ if (builtinReq)
{
- builtinReq = assoc->findModifier<BuiltinRequirementModifier>();
isConstraint = true;
}
}| IRInterfaceRequirementEntry* getInterfaceEntryByBuiltinRequirement( | ||
| IRInterfaceType* interface, | ||
| UInt index) | ||
| BuiltinRequirementKind kind) | ||
| { | ||
| SLANG_UNUSED(moduleInst); | ||
| if (interface) | ||
| if (!interface) | ||
| return nullptr; | ||
| for (UInt i = 0; i < interface->getOperandCount(); i++) | ||
| { | ||
| // Assume for now that IDifferentiable has exactly five fields. | ||
| // SLANG_ASSERT(interface->getOperandCount() == 5); | ||
| if (auto entry = as<IRInterfaceRequirementEntry>(interface->getOperand(index))) | ||
| return entry; | ||
| else | ||
| auto entry = as<IRInterfaceRequirementEntry>(interface->getOperand(i)); | ||
| if (!entry) | ||
| continue; | ||
| if (auto decor = | ||
| entry->getRequirementKey()->findDecoration<IRBuiltinRequirementDecoration>()) | ||
| { | ||
| SLANG_UNEXPECTED("IDifferentiable interface entry unexpected type"); | ||
| if ((BuiltinRequirementKind)decor->getKind() == kind) | ||
| return entry; | ||
| } | ||
| } | ||
|
|
||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
Fail fast when a built-in requirement role is missing.
Returning nullptr here turns any lowering/pass-order mismatch into an unchecked null dereference at the new call sites (for example Line 266, Line 307, Line 925, and Line 3560). In core compiler code this helper should diagnose or hard-assert when the requested role is absent instead of silently returning null.
Suggested fix
IRInterfaceRequirementEntry* getInterfaceEntryByBuiltinRequirement(
IRInterfaceType* interface,
BuiltinRequirementKind kind)
{
- if (!interface)
- return nullptr;
+ SLANG_RELEASE_ASSERT(interface);
for (UInt i = 0; i < interface->getOperandCount(); i++)
{
auto entry = as<IRInterfaceRequirementEntry>(interface->getOperand(i));
if (!entry)
continue;
if (auto decor =
entry->getRequirementKey()->findDecoration<IRBuiltinRequirementDecoration>())
{
if ((BuiltinRequirementKind)decor->getKind() == kind)
return entry;
}
}
- return nullptr;
+ SLANG_UNEXPECTED("missing builtin interface requirement entry");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| IRInterfaceRequirementEntry* getInterfaceEntryByBuiltinRequirement( | |
| IRInterfaceType* interface, | |
| UInt index) | |
| BuiltinRequirementKind kind) | |
| { | |
| SLANG_UNUSED(moduleInst); | |
| if (interface) | |
| if (!interface) | |
| return nullptr; | |
| for (UInt i = 0; i < interface->getOperandCount(); i++) | |
| { | |
| // Assume for now that IDifferentiable has exactly five fields. | |
| // SLANG_ASSERT(interface->getOperandCount() == 5); | |
| if (auto entry = as<IRInterfaceRequirementEntry>(interface->getOperand(index))) | |
| return entry; | |
| else | |
| auto entry = as<IRInterfaceRequirementEntry>(interface->getOperand(i)); | |
| if (!entry) | |
| continue; | |
| if (auto decor = | |
| entry->getRequirementKey()->findDecoration<IRBuiltinRequirementDecoration>()) | |
| { | |
| SLANG_UNEXPECTED("IDifferentiable interface entry unexpected type"); | |
| if ((BuiltinRequirementKind)decor->getKind() == kind) | |
| return entry; | |
| } | |
| } | |
| return nullptr; | |
| } | |
| IRInterfaceRequirementEntry* getInterfaceEntryByBuiltinRequirement( | |
| IRInterfaceType* interface, | |
| BuiltinRequirementKind kind) | |
| { | |
| SLANG_RELEASE_ASSERT(interface); | |
| for (UInt i = 0; i < interface->getOperandCount(); i++) | |
| { | |
| auto entry = as<IRInterfaceRequirementEntry>(interface->getOperand(i)); | |
| if (!entry) | |
| continue; | |
| if (auto decor = | |
| entry->getRequirementKey()->findDecoration<IRBuiltinRequirementDecoration>()) | |
| { | |
| if ((BuiltinRequirementKind)decor->getKind() == kind) | |
| return entry; | |
| } | |
| } | |
| SLANG_UNEXPECTED("missing builtin interface requirement entry"); | |
| } |
| static NodeBase* parseInterfaceConstraintDecl(Parser* parser, void*) | ||
| { | ||
| auto constraint = parser->astBuilder->create<GenericTypeConstraintDecl>(); | ||
| parser->FillPosition(constraint); | ||
| constraint->sub = parser->ParseTypeExp(); | ||
| Token constraintToken; | ||
| if (AdvanceIf(parser, TokenType::OpEql, &constraintToken)) | ||
| { | ||
| constraint->isEqualityConstraint = true; | ||
| } | ||
| else | ||
| { | ||
| constraintToken = parser->ReadToken(TokenType::Colon); | ||
| } | ||
| constraint->loc = constraintToken.loc; | ||
| constraint->sup = parser->ParseTypeExp(); | ||
| parser->ReadToken(TokenType::Semicolon); | ||
| return constraint; |
There was a problem hiding this comment.
Reject __constraint outside interface bodies.
This accepts __constraint in any declaration context. Since GenericTypeConstraintDecl is not covered by the nesting-validation tables, misplaced uses won't get a direct parser diagnostic and will leak an invalid AST into later checking. Please guard this path to InterfaceDecl parents, or wire GenericTypeConstraintDecl into isDeclAllowed().
As per coding guidelines, source/slang/**: "(5) Null pointer safety and proper error handling via diagnostics."
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
This PR modifies IR instruction definition files. Please review if you need to update the following constants in
These version numbers help ensure compatibility between different versions of compiled modules. |
Modeling `associatedtype __Tag : __BuiltinIntegerType` as an interface-level constraint requirement means the `__EnumType` conformance check now validates it. The compiler-synthesized enum conformance did not supply that witness (a long-standing TODO), so permitted-but-non-conforming tag types such as `bool` were wrongly rejected. Provide the `__Tag : __BuiltinIntegerType` witness in the enum conformance synthesis (a real subtype witness for integer tags, a NoneWitness for `bool`), and have the conformance check respect a witness already provided by synthesis instead of re-deriving it. Fixes the regression in tests/bugs/11043-enum-constant-wraparound.slang.
Dynamic-dispatch lowering read the witness-table requirement key of an `IRGetTagForMappedSet` via `cast<IRStructKey>`. A built-in interface requirement (e.g. an IDifferentiable/IBackwardDifferentiable method reached through dynamic dispatch) now uses the hoistable `IRBuiltinRequirementKey`, which is not an `IRStructKey`: with assertions enabled the cast aborted (Linux/macOS debug CI), and in release the unchecked cast led to a segfault on the CPU dynamic-dispatch autodiff tests. The key is only used as a witness-table lookup key (`findWitnessTableEntry`) and to read a name hint, both of which accept any `IRInst*`, so type it as `IRInst*` in both `TypeFlowSpecializationContext::collectDispatchActions` and the `GetTagForMappedSet` lowering. Fixes the dynamic-dispatch/existential autodiff regressions (diff-assoctype-generic-interface, dynamic-dispatch-*, dyn-interface-*, dynamic-object-bwd-diff-*, existential-*, material/diff-falcor-*).
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 683cdcb6-769a-4bff-83f2-12dc663b442a
📒 Files selected for processing (2)
source/slang/slang-ir-lower-dynamic-dispatch-insts.cppsource/slang/slang-ir-typeflow-specialize.cpp
| // A witness-table requirement key. This is `IRInst*` rather than | ||
| // `IRStructKey*` because a built-in interface requirement (e.g. an | ||
| // `IDifferentiable`/`IBackwardDifferentiable` method reached through | ||
| // dynamic dispatch) uses the hoistable `IRBuiltinRequirementKey`, which is | ||
| // not an `IRStructKey`. It is only used below as a witness-table lookup key | ||
| // and to read a name hint, both of which accept any `IRInst*`. | ||
| IRInst* lookupKey = nullptr; |
There was a problem hiding this comment.
Synchronize IR schema with widened lookup key type
This consumer now accepts builtin requirement keys (IRBuiltinRequirementKey) via IRInst*, but the IR schema still declares dispatcher lookup keys as IRStructKey (source/slang/slang-ir-insts.lua, Lines 3006-3019). That contract mismatch can violate IR type invariants when dynamic dispatch carries builtin requirement roles.
Suggested follow-up (cross-file contract fix)
--- a/source/slang/slang-ir-insts.lua
+++ b/source/slang/slang-ir-insts.lua
@@
- operands = {{"witnessTableSet", "IRWitnessTableSet"}, {"lookupKey", "IRStructKey"}}
+ operands = {{"witnessTableSet", "IRWitnessTableSet"}, {"lookupKey", "IRInst"}}As per coding guidelines, "source/slang/: This is the core compiler AND all target emitters ... Review for: (1) IR pass correctness — ensure SSA form and type invariants are maintained."**
Also applies to: 6401-6401
There was a problem hiding this comment.
Verdict: 🟡 Has issues — 0 bugs, 4 gaps
The PR unifies three associated-type constraint forms (: IFoo, where A : IFoo, __constraint) into one representation, adds the __constraint keyword for derived interfaces, replaces hardcoded operand-index access in autodiff with a role-based getInterfaceEntryByBuiltinRequirement, and adds a hoistable IRBuiltinRequirementKey deduplicated by role. Findings are all gaps: a missing module-version bump, a missing user-guide entry for the new __constraint keyword, missing test coverage for several behaviors the PR explicitly implements (subtype __constraint, syntax equivalence, enum : bool NoneWitness path, multi-segment subjects), and unguarded null-deref at every callsite of the new role-based lookup helper.
Changes Overview
Constraint unification & new __constraint syntax (slang-parser.cpp, slang-check-decl.cpp, slang-check-inheritance.cpp, slang-ast-decl-ref.cpp, slang-lower-to-ir.cpp)
parseAssocTyperoutes both the:clause and thewhereclause to the enclosing interface; newparseInterfaceConstraintDecladds__constraint(subtype or equality) as a sibling member of the interface.findWitnessForInterfaceRequirementvalidates an interface-levelGenericTypeConstraintDeclrequirement (with==requiring a type-equality witness)._calcInheritanceInfowalks the anchor chain of an associated-type access, matches constraints whose subject (relative toThis) names the chain, and surfaces the opposite endpoint as a base — guarded against cycles (isBeingChecked()) and unbounded equality recursion (depth check).LookupDeclRef::tryResolveresolves aGenericTypeConstraintDeclrequirement key to its associated-type role from the constraint's subject.EnumDeclsynthesis now records the__Tag : __BuiltinIntegerTypewitness (usingNoneWitnessforbool).
Hoistable role-based requirement key (slang-ir-insts.lua, slang-ir-insts-stable-names.lua, slang-ir-insts.h, slang-ast-support-types.h, slang-lower-to-ir.cpp, slang-ir-link.cpp, slang-emit-c-like.cpp)
- New hoistable
IRBuiltinRequirementKey(kind)deduplicated by itskindoperand andIRBuiltinRequirementDecoration(kind).BuiltinRequirementKindextended withDifferentialWitness,DifferentialPtrWitness,BwdCallableContextWitness.getInterfaceRequirementKeyreturns the hoistable key for built-in roles (including derived witness roles). Linker clones built-in-requirement entries eagerly (no mangled name); emit skipskIROp_BuiltinRequirementKey. The requirement-key type is widened fromIRStructKey*toIRInst*ingetInterfaceRequirementKey,interfaceRequirementKeys,addEntry,slang-ir-typeflow-specialize.cpp, andslang-ir-lower-dynamic-dispatch-insts.cpp.
Role-based autodiff requirement access (slang-ir-autodiff.cpp, slang-ir-autodiff.h, slang-ir-autodiff-fwd.cpp)
- New
getInterfaceEntryByBuiltinRequirement(interface, kind)replaces the deletedgetInterfaceEntryAtIndex.AutoDiffSharedContextandmaybeTranslateBackwardDerivativeWitnessnow addressIDifferentiable/IBackwardDifferentiable/IBwdCallablerequirements by role. BrittlegetRequirementCount() == Nasserts are removed.
Lowering of relocated subtype constraints (slang-lower-to-ir.cpp)
addEntrylowers a relocatedGenericTypeConstraintDeclsubtype constraint with aWitnessTableTyperequirement-val (matching the previous nested-bound shape); equality constraints take the generic path.visitGenericTypeConstraintDeclreturns the requirement key when the constraint is a direct interface member.
Tests (tests/language-feature/interfaces/derived-interface-constraint*.slang)
- Three INTERPRET / DIAGNOSTIC tests exercising single-segment
__constraint X == This(basic, multi-level inheritance, and aBad : IDerivedwhoseDataType != Thisis rejected withE38029).
Findings (4 total)
| Severity | Location | Finding |
|---|---|---|
| 🟡 Gap | source/slang/slang-ir-insts-stable-names.lua:837 (and slang-ir.h:2180) |
k_maxSupportedModuleVersion not bumped despite adding two new IR ops |
| 🟡 Gap | source/slang/slang-parser.cpp:10341 |
New __constraint keyword not documented in docs/user-guide/ |
| 🟡 Gap | tests/language-feature/interfaces/derived-interface-constraint.slang |
Missing tests: subtype __constraint, :/where equivalence, enum : bool NoneWitness, multi-segment subjects |
| 🟡 Gap | source/slang/slang-ir-autodiff-fwd.cpp:~925 (and ~3550–3667, plus slang-ir-autodiff.cpp ctor) |
getInterfaceEntryByBuiltinRequirement(...)->getRequirementKey() callers do not null-check |
| ["IncrementBranchCoverageCounter"] = 859 | ||
| ["IncrementBranchCoverageCounter"] = 859, | ||
| ["Decoration.BuiltinRequirementDecoration"] = 860, | ||
| ["builtinRequirementKey"] = 861 |
There was a problem hiding this comment.
🟡 Gap: IRModule::k_maxSupportedModuleVersion not bumped despite adding two new IR ops
This PR registers two new stable-named IR ops here (Decoration.BuiltinRequirementDecoration = 860, builtinRequirementKey = 861), but source/slang/slang-ir.h:2180 still has k_maxSupportedModuleVersion = 18. The comment immediately above that constant (lines 2171-2174) is explicit about the contract:
Additionally this should be updated when new instructions are added, however only
k_maxSupportedModuleVersionneeds to be incremented in that case
A pre-PR Slang build that loads a module serialized by this PR will read it at the same version 18 as one with no new ops, so it has no way to refuse loading a module that may contain kIROp_BuiltinRequirementKey or kIROp_BuiltinRequirementDecoration it doesn't know how to interpret. That defeats the purpose of the version field for these two ops.
Suggested fix: bump k_maxSupportedModuleVersion from 18 to 19 in source/slang/slang-ir.h.
|
|
||
| _makeParseDecl("typedef", parseTypeDef), | ||
| _makeParseDecl("associatedtype", parseAssocType), | ||
| _makeParseDecl("__constraint", parseInterfaceConstraintDecl), |
There was a problem hiding this comment.
🟡 Gap: New user-facing __constraint keyword is not documented in docs/user-guide/
This line registers __constraint as a new top-level decl keyword that users can write in interface bodies (e.g. __constraint DataType == This; or __constraint X : IFoo;). The chapter that covers associatedtype constraints and where clauses (docs/user-guide/06-interfaces-generics.md) is unchanged, so users have no way to discover the new syntax, when it differs from where clauses, or that subtype (:) and equality (==) forms are both accepted.
The PR description also says "associatedtype A : IFoo" and "associatedtype A where A : IFoo" are now genuinely the same representation — that user-visible equivalence claim deserves to be documented, since users have so far been told they may differ.
Suggested fix: add a subsection to docs/user-guide/06-interfaces-generics.md (or wherever associated-type bounds are described) documenting:
- the
__constraint <type> == <type>and__constraint <type> : <type>forms inside an interface body, - that
__constraint X : Iis equivalent to attaching: Idirectly toX'sassociatedtypedecl, - the derived-interface use case (
interface IDerived : IBase { __constraint DataType == This; }) that motivated this PR.
If __constraint is intentionally an internal/unstable spelling, leading-double-underscore aside, please mark it as such where it's parsed (or gate it behind an internal-only mode), so users don't start depending on it.
| @@ -0,0 +1,46 @@ | |||
| //TEST:INTERPRET(filecheck=CHECK): | |||
There was a problem hiding this comment.
🟡 Gap: Test coverage misses several behaviors the PR explicitly implements
The three new tests (this file, -deep, -error) all use the __constraint X == Y form on an interface that constrains DataType against This. Several behaviors the PR explicitly implements have no test:
-
__constraint X : IFoo(subtype, not equality) is untested.parseInterfaceConstraintDeclaccepts bothOpEqlandColon, andslang-lower-to-ir.cpp::addEntryhas a special branch for non-equality relocated subtype constraints that lowers them with aWitnessTableTyperequirement value (!relocatedSubtypeConstraint.getDecl()->isEqualityConstraint). That entire branch — and its consumer side in autodiff/dynamic-dispatch — is reached today only indirectly through the desugaredassociatedtype A : IBarform. A test likeinterface IDerived : IBase { __constraint DataType : ISomething; }plus a generic<T : IDerived>consumer that calls a method declared onISomethingviat.getData()would pin this path. Pair with a negative diagnostic test whereT.DataTypedoes not implementISomething. -
Equivalence of
: IFooandwhere A : IFoois unverified. The parser comment claims the two surface forms produce identical representations after this PR. There is no test that exercises both syntaxes for the same scenario in the same file (or in paired files) and asserts they accept/reject the same conformances. A regression that broke just one of the two parser paths would still pass every existing test. -
enum : bool(theNoneWitnessfallback) is untested.slang-check-decl.cpp::visitEnumDeclnow synthesizes a witness for__Tag : __BuiltinIntegerType; for an integer tag type it emits the real subtype witness, but forbool(a permitted tag type that does not conform to__BuiltinIntegerType) it emitsNoneWitness. The PR comment notes this "addresses a long-standing TODO that became load-bearing once the bound is modeled as a requirement," meaning a regression here would silently break compilation of any usage ofenum : bool. Today the onlyenum : booltest (tests/bugs/11043-enum-constant-wraparound.slang) is a wraparound diagnostic that does not exercise lowering. An INTERPRET test that definesenum E : bool { ... }, instantiates one, and switches on it would lock this in. Pair withenum E : intto also cover the real-witness branch. -
Multi-segment constraint subjects (
This.TA.TB) are untested.slang-check-inheritance.cppadds a multi-anchor walk and a "skip a constraint that is currently being checked" cycle-breaker explicitly described as needed when "resolving a multi-level subject (e.g.This.TA.TB)".derived-interface-constraint-deep.slangexercises interface-inheritance depth (IBase → IMid → ILeaf) but only a single-segment subject (DataType == This); the cycle-breaker and the deeper part of the chain-decomposition logic are never reached. A test with__constraint This.TA.TB == This(or similar two-segment subject) would cover this.
| // by built-in requirement *role* rather than by operand position. Addressing by | ||
| // role is robust to the order in which requirements (and the relocated | ||
| // `BwdCallable : IBwdCallable` conformance) appear in the interface. | ||
| auto bwdDiffContextTypeReqKey = getInterfaceEntryByBuiltinRequirement( |
There was a problem hiding this comment.
🟡 Gap: Every getInterfaceEntryByBuiltinRequirement(...)->getRequirementKey() call is unguarded against nullptr
The new helper (defined in slang-ir-autodiff.cpp) returns nullptr when the requested role is not present on the interface:
IRInterfaceRequirementEntry* getInterfaceEntryByBuiltinRequirement(
IRInterfaceType* interface,
BuiltinRequirementKind kind)
{
if (!interface)
return nullptr;
for (UInt i = 0; i < interface->getOperandCount(); i++)
{
...
}
return nullptr;
}Every caller in this file (~6 sites starting at this line) and every caller in maybeTranslateBackwardDerivativeWitness (8 sites in the same file at lines ~3550–3667) and in AutoDiffSharedContext (slang-ir-autodiff.cpp::AutoDiffSharedContext constructor) chains ->getRequirementKey() directly on the result. If a built-in requirement decoration is missing for any reason — a stripped/partial core module, a future refactor that forgets to mint the decoration, or a cross-module link that drops it on the deduplicated hoistable key — the chain segfaults instead of producing a diagnostic.
The previous index-based code (cast<IRInterfaceRequirementEntry>(interface->getOperand(N))) at least failed via SLANG_UNEXPECTED in getInterfaceEntryAtIndex with a string identifying the issue. Role-based lookup gives up that property silently.
Suggested fix: either add SLANG_ASSERT(entry) (or a SLANG_RELEASE_ASSERT with a message naming the missing kind) immediately after each call before the ->getRequirementKey() chain, or push the assert inside getInterfaceEntryByBuiltinRequirement itself with the requested kind in the message so any null result fails loudly with the role name attached.
1. Motivation
Slang lets you bound an associated type in two ways that ought to be exactly equivalent:
They were not modeled the same way internally (the
:bound was nested inside theAssocTypeDecl; awherebound was a separate constraint), which meant the two forms couldbehave differently and there was no single, principled representation for "a constraint on an
associated type." We also had no surface syntax to constrain an inherited associated type from a
derived interface.
This PR makes the two forms identical and adds a
__constraintform for derived interfaces, e.g.:Example tests added:
tests/language-feature/interfaces/derived-interface-constraint.slang—__constraint DataType == This, checked via the interpreter (expects42).tests/language-feature/interfaces/derived-interface-constraint-deep.slang— multi-level chain (This.A.A == ...), expects15.tests/language-feature/interfaces/derived-interface-constraint-error.slang— diagnostic test for a type that violates the constraint (error E38029).2. Proposed solution
Model every associated-type constraint —
associatedtype A : IFoo,associatedtype A where A : IFoo, and__constraint A : IFoo/A == B— identically: as a constraint requirement of the enclosing interface, a sibling of the associated type.This mirrors how a generic function
void foo<T : IFoo>()represents the parameterTand theconstraint
T : IFooas parallel members of the enclosing generic. An associated type and aconstraint on it are likewise both requirements of the interface.
Making the built-in differentiable interfaces (
IDifferentiable,IBackwardDifferentiable,IBwdCallable, …) survive this relocation required removing the two places where auto-diff reliedon the old layout:
more than once for the same logical requirement) → replaced with a hoistable
IRBuiltinRequirementKeydeduplicated by construction from its role.role-based lookup.
3. Change summary
slang-parser.cpp__constraintdecl + keyword;parseAssocTyperoutes the:bound and thewhereclause to the enclosing interface.slang-check-decl.cpp,slang-check-inheritance.cpp,slang-ast-decl-ref.cppT.A's constraints to generic code; resolve the differential rule by the constraint subject.slang-ir-insts.lua,slang-ir-insts-stable-names.lua,slang-ir-insts.h,slang-ast-support-types.hIRBuiltinRequirementKey(kind)+IRBuiltinRequirementDecoration;BuiltinRequirementKind+=DifferentialWitness,DifferentialPtrWitness,BwdCallableContextWitness.slang-lower-to-ir.cppgetInterfaceRequirementKeyreturns the hoistable key for built-in roles;addEntrylowers a relocated subtype constraint with aWitnessTableTypevalue.slang-ir-link.cpp,slang-emit-c-like.cppslang-ir-autodiff.{h,cpp},slang-ir-autodiff-fwd.cppIDifferentiable/IBackwardDifferentiable/IBwdCallablerequirements by role; remove dead index-based accessor and brittle count asserts.tests/language-feature/interfaces/derived-interface-constraint*.slangVerification: core module compiles with assertions active;
tests/autodiff/,tests/autodiff-dstdlib/,tests/language-feature/{interfaces,generics,associated-types}/, all*where*tests, and the 3 new tests have 0 non-environmental failures (the only failures arethe
syn (llvm)host-execution variants, which fail identically for unrelated trivial tests inthis CI-less sandbox).
4. How this set of changes was derived (every change, with reasons and code traces)
This started as "add derived-interface constraints," became "unify all associated-type
constraints," and the bulk of the work was chasing a chain of cascading auto-diff failures that the
unification exposed. Each change below is justified by a concrete failure it fixes.
4.1 Parser — one representation for all three forms
parseAssocTypesends both the inheritance clause (parseOptionalGenericConstraints(..., constraintTarget = interface)) and thewhereclause (maybeParseGenericConstraints(..., interface)) to the enclosing interface, and__constraint(parseInterfaceConstraintDecl) buildsthe same
GenericTypeConstraintDecldirectly under the interface. Without this,: IFooandwhere A : IFooremain different ASTs and the equivalence the PR promises does not hold.4.2 Conformance checking and surfacing
findWitnessForInterfaceRequirement(slang-check-decl.cpp) gains a branch for aGenericTypeConstraintDeclrequirement: it proves the constraint holds for the conforming typevia
isSubtype(requiring a type-equality witness for==), and diagnosesTypeArgumentDoesNotConformToInterfaceotherwise. Without it, a type "conforming" toIDerivedwhoseDataType != Thiswould be accepted (unsound), and the constraint would bereported as an unsatisfied named member.
slang-check-inheritance.cppsurfaces the relocated constraint to generic code: for an accessT.A, it walksT's interface facets and adds the constraint's other endpoint as a base ofT.A. Without it,f<T:IDerived>()does not knowT.DataType == T('value' is not a member of 'T.DataType'). Includes a cycle guard (skipisBeingChecked()) and a depth guard (only thereduce-to-shallower direction of a symmetric equality is added, else self-referential equalities
such as
Differential.Differential == Differentialrecurse forever — this crashed while compilingwhere-boundedvector<T,N>).LookupDeclRef::tryResolve(slang-ast-decl-ref.cpp) answers "which associated type does thisconstraint constrain?" from the constraint subject rather than assuming the parent decl is the
associated type. Without it, auto-diff's
doesSignatureMatchRequirementcould not reduceX.Differentialand asserted.4.3 The cascade: relocating the built-in differentiable bounds
The interesting part. Relocating
associatedtype Differential : IDifferentiable(and the backwardanalogues) to interface scope broke auto-diff in a sequence of distinct ways. Each was root-caused
to an assumption that built-in interface requirements are identified by position or key
identity rather than by role.
(a) Missing / mismatched
DifferentialWitness→ crash ontests/autodiff/deduplicate-witness-table.slang.At the IR level
specializeWitnessLookup→findWitnessTableEntry(table, key)matches by pointeridentity.
getInterfaceRequirementKeyminted a fresh, linkage-namedIRStructKeyperrequirement decl, so the canonical interface constraint and a constraint synthesized while building
a type's
Differentialproduced two distinct keys for the same role → the lookup missed →replaceUsesWith(null)trippedSLANG_ASSERT(other)(and leaked / crashed with0xC0000005).Fix: a new hoistable inst
IRBuiltinRequirementKey(kind)(oneIRIntLitoperand). Because itis hoistable it is deduplicated by construction from its kind operand, so every reference to a
given built-in requirement resolves to a single key inst — across decls and across the precompiled
core-module boundary.
getInterfaceRequirementKeyreturns it for built-in roles: an explicit__builtin_requirement(kind), or a derived witness role for an associated type's: Ibound(
DifferentialType → DifferentialWitness,DifferentialPtrType → DifferentialPtrWitness,BwdCallableContextType → BwdCallableContextWitness). This is the right fix (not a lookupwork-around) because a built-in requirement is identified by its role; making "same role ⇒ same
key" hold by construction is what witness-table lookup-by-identity assumes.
It can't be a subclass of
IRStructKey—StructKeyis a concrete instantiable leaf, and making ita parent removes its
kIROp_StructKeyopcode. So the requirement-key type is widenedIRStructKey* → IRInst*throughgetInterfaceRequirementKey, its cache, and theaddEntrylambda;the auto-diff context members that hold these keys become
IRInst*(they are only ever used aslookup keys, which accept any
IRInst*).(b) Built-in keys must stay out of linkage (per design: they have no mangled name). Two link-time
sites keyed deferred witness-table-entry cloning by
getMangledName(key), which is empty for ahoistable key — multiple built-in entries would collide on
"". Fix (slang-ir-link.cpp):built-in-requirement witness entries are cloned eagerly (skipping the mangled-name-keyed deferred
dictionary), and a
LookupWitnessMethodon a built-in key does not record an empty deferred key.Fix (
slang-emit-c-like.cpp):ensureGlobalInstskipskIROp_BuiltinRequirementKey(it ismetadata, like an interface requirement entry, and being hoistable may survive unreferenced).
Without these, the build asserted on a duplicate
""dictionary key, and emit hitdiagnoseUnhandledInst("unexpected IR opcode during code emit").(c) Relocated subtype constraint lowered with the wrong requirement value. When the bound was
nested, interface lowering produced a conformance-witness requirement entry whose
requirementValis aWitnessTableType. After relocation the constraint is a sibling memberhandled by
addEntry, which only special-casedInheritanceDecland so gave a relocatedGenericTypeConstraintDecla malformed value. Fix (slang-lower-to-ir.cpp):addEntrylowers arelocated subtype constraint exactly like the nested bound (a
WitnessTableTypevalue);equality constraints keep the generic path. Without it,
tests/autodiff/high-order-backward-diff-1.slangcrashed in
specializeWitnessLookupbecause the conformance entry's value was not a witness table.(d) The auto-diff backend read requirements by hardcoded operand index. This was the root of the
remaining backward-diff / higher-order / cross-module / extension regressions (e.g.
high-order-backward-diff-1/2,autopybind-*,cuda-kernel-export,func-extension/fwd-diff-*).slang-ir-autodiff-fwd.cppreadIBackwardDifferentiable/IBwdCallablerequirement keys withcast<IRInterfaceRequirementEntry>(iface->getOperand(N))->getRequirementKey()(≈13 sites, guardedby
SLANG_ASSERT(getRequirementCount() == 6/3/1)). Relocation restructured theBwdCallable : IBwdCallableconformance requirement, shifting/altering these positions → malformedwitness tables and a null
getFuncDefinitionForContextin type-flow specialization. Fix: exposegetInterfaceEntryByBuiltinRequirement(interface, kind)and convert both the consuming block and thewitness-table-building block to role lookup; drop the brittle count asserts; delete the now-dead
getInterfaceEntryAtIndex. This is the principled fix (and the one this PR is built around):witness-table / interface requirements are an unordered key→value collection, so a consumer must
address them by role, never by position. (
IForwardDifferentiable's accesses are intentionally leftindex-based: its
op1/op2arefwd_diff's callable conformance constraints, which are notrelocated and are layout-stable.)
After (a)–(d), the full auto-diff suite and the language-feature suites are green with zero
non-environmental failures, and
associatedtype A : IFooandassociatedtype A where A : IFooaregenuinely the same representation.
🤖 Generated with Claude Code