Skip to content

Commit 65749cf

Browse files
authored
Diagnose if extension declares new interface requirements (#9276)
## Summary This PR adds a diagnostic for missing symbol definitions during the IR linking phase. When `cloneGlobalValueWithLinkage` selects the "best value" for a symbol, it now validates that the value is actually usable—meaning it must either: - Have a function body (a concrete definition) - Be marked as a target intrinsic (via appropriate decoration) - Be marked as an imported symbol (e.g., `DLLImport` decoration) - Have SPIRV instruction or autodiff decorations If none of these conditions are met, the compiler now emits a linker error indicating the symbol lacks a definition, along with note diagnostics listing all candidate values that were considered. ## Background Previously, declaring a method signature in an extension without providing a body would silently proceed through compilation until hitting an internal error during code emission ("unexpected IR opcode"). This made it difficult to diagnose issues where users accidentally declared interface-like requirements in extensions—something that isn't semantically valid since extensions cannot add new requirements to existing types. ## Changes - **`source/slang/slang-ir-link.cpp`**: Added validation in `cloneGlobalValueWithLinkage` to check that the selected best value has a valid definition. Added helper function `isFunctionDefinedOrImported` to perform the check. Emits a clear diagnostic when a symbol is referenced but has no definition. - **`tests/diagnostics/extension-interface-requirement.slang`**: Added test case covering the scenario where an extension declares a method without a body. Closes #9198
1 parent 72761cc commit 65749cf

File tree

2 files changed

+140
-0
lines changed

2 files changed

+140
-0
lines changed

source/slang/slang-ir-link.cpp

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,13 @@ struct IRSharedSpecContext
6060

6161
// The "global" specialization environment.
6262
IRSpecEnv globalEnv;
63+
64+
// Diagnostic sink for reporting errors during linking.
65+
DiagnosticSink* sink = nullptr;
6366
};
6467

6568
void insertGlobalValueSymbol(IRSharedSpecContext* sharedContext, IRInst* gv);
69+
static bool isFunctionDefinedOrImported(IRInst* inst);
6670

6771
struct WitnessTableCloneInfo : RefObject
6872
{
@@ -1553,6 +1557,27 @@ IRInst* cloneGlobalValueWithLinkage(
15531557
return nullptr;
15541558
}
15551559

1560+
// Check that the best value we found is valid: if it's a function,
1561+
// it should either have a body, be an intrinsic, or be imported.
1562+
// This catches cases like extension methods declared without a body,
1563+
// which are not valid (extensions cannot define new requirements).
1564+
if (!isFunctionDefinedOrImported(bestVal))
1565+
{
1566+
if (auto sink = context->shared->sink)
1567+
{
1568+
sink->diagnose(bestVal->sourceLoc, Diagnostics::unresolvedSymbol, bestVal);
1569+
1570+
// Emit notes for all available declarations of this symbol
1571+
for (IRSpecSymbol* ss = sym; ss; ss = ss->nextWithSameName)
1572+
{
1573+
sink->diagnose(
1574+
ss->irGlobalValue->sourceLoc,
1575+
Diagnostics::seeDeclarationOf,
1576+
ss->irGlobalValue);
1577+
}
1578+
}
1579+
}
1580+
15561581
// Check if we've already cloned this value, for the case where
15571582
// we didn't have an original value (just a name), but we've
15581583
// now found a representative value.
@@ -1697,6 +1722,60 @@ static bool doesFuncHaveDefinition(IRFunc* func)
16971722
return false;
16981723
}
16991724

1725+
/// Check if a function (or generic containing a function) is properly defined or imported.
1726+
/// Returns true if the value is not a function, or if it has a definition, is an intrinsic,
1727+
/// or is marked as imported.
1728+
static bool isFunctionDefinedOrImported(IRInst* inst)
1729+
{
1730+
IRFunc* func = nullptr;
1731+
1732+
// Handle generic case - unwrap the generic to get the function
1733+
if (auto generic = as<IRGeneric>(inst))
1734+
{
1735+
func = as<IRFunc>(findGenericReturnVal(generic));
1736+
}
1737+
else
1738+
{
1739+
func = as<IRFunc>(inst);
1740+
}
1741+
1742+
// Not a function, no check needed
1743+
if (!func)
1744+
return true;
1745+
1746+
// Check if it has a function body
1747+
if (func->getFirstBlock() != nullptr)
1748+
return true;
1749+
1750+
// Check for decorations that indicate the function has an external/special implementation
1751+
for (auto decor : func->getDecorations())
1752+
{
1753+
switch (decor->getOp())
1754+
{
1755+
// Intrinsic decorations
1756+
case kIROp_IntrinsicOpDecoration:
1757+
case kIROp_TargetIntrinsicDecoration:
1758+
case kIROp_SPIRVOpDecoration:
1759+
// Autodiff decorations - the function's implementation is provided by the derivative
1760+
// function
1761+
case kIROp_ForwardDerivativeDecoration:
1762+
case kIROp_BackwardDerivativeDecoration:
1763+
case kIROp_UserDefinedBackwardDerivativeDecoration:
1764+
case kIROp_PrimalSubstituteDecoration:
1765+
return true;
1766+
default:
1767+
continue;
1768+
}
1769+
}
1770+
1771+
// Check for import decorations on the original inst (for generics)
1772+
if (inst->findDecoration<IRImportDecoration>() || inst->findDecoration<IRDllImportDecoration>())
1773+
return true;
1774+
1775+
1776+
return false;
1777+
}
1778+
17001779
static bool doesWitnessTableHaveDefinition(IRWitnessTable* wt)
17011780
{
17021781
auto interfaceType = as<IRInterfaceType>(wt->getConformanceType());
@@ -2065,6 +2144,7 @@ LinkedIR linkIR(CodeGenContext* codeGenContext)
20652144

20662145
auto sharedContext = state->getSharedContext();
20672146
initializeSharedSpecContext(sharedContext, session, nullptr, targetReq);
2147+
sharedContext->sink = codeGenContext->getSink();
20682148

20692149
state->irModule = sharedContext->module;
20702150

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// extension-interface-requirement.slang
2+
3+
// Test that extension method declarations without bodies are properly diagnosed.
4+
// Extensions cannot declare new interface requirements - they must provide implementations.
5+
// See https://github.com/shader-slang/slang/issues/9198
6+
7+
//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK):-target spirv
8+
9+
// Simple interface
10+
public interface IROTensor<T, let D : int>
11+
{
12+
T read_buffer(int idx);
13+
}
14+
15+
// Concrete type implementing the interface
16+
public struct ROTensor<T, let D : int> : IROTensor<T, D>
17+
{
18+
public StructuredBuffer<T> _data;
19+
public uint[D] _strides;
20+
public uint _offset;
21+
22+
public T read_buffer(int idx)
23+
{
24+
return _data[idx];
25+
}
26+
}
27+
28+
// Helper function
29+
int _idx(int i0, uint stride[1], uint offset)
30+
{
31+
return i0 * stride[0] + offset;
32+
}
33+
34+
// This concrete extension works fine - it has a function body
35+
public extension<T> ROTensor<T, 1>
36+
{
37+
public T load(int i0)
38+
{
39+
int idx = _idx(i0, this._strides, this._offset);
40+
return this.read_buffer(idx);
41+
}
42+
}
43+
44+
// ERROR: Extension with generic type parameter that declares a method without a body.
45+
// Extensions cannot declare new interface requirements.
46+
public extension<T, TensorType : IROTensor<T, 1>> TensorType
47+
{
48+
// CHECK: ([[# @LINE+1]]): error 45001
49+
public T loadGeneric(int i0); // Error: function declaration without definition
50+
}
51+
52+
// Test entry point
53+
[shader("compute")]
54+
[numthreads(1, 1, 1)]
55+
void main()
56+
{
57+
ROTensor<float, 1> tensor;
58+
float value = tensor.load(0);
59+
float value2 = tensor.loadGeneric(0); // Uses the extension method
60+
}

0 commit comments

Comments
 (0)