Skip to content

Commit 2e52217

Browse files
author
Tim Foley
authored
Support conversion from int/uint to enum types (shader-slang#1147)
* Support conversion from int/uint to enum types The basic feature here is tiny, and is summarized in the code added to the stdlib: ``` extension __EnumType { __init(int val); __init(uint val); } ``` The front-end already makes all `enum` types implicitly conform to `__EnumType` behind the scenes, and this `extension` makes it so that all such types inherit some initializers (`__init` declarations, aka. "constructors") that take `int` and `uint`. (Note: right now all `__init` declarations in Slang are assumed to be implemented as intrinsics using `kIROp_Construct`. This obviously needs to change some day, especially so that we can support user-defined initializers.) Actually making this *work* required a bit of fleshing out pieces of the compiler that had previously been a bit ad hoc to be a bit more "correct." Most of the rest of this description is focused on those details, since the main feature is not itself very exciting. When overload resolution sees an attempt to "call" a type (e.g., `MyType(3.0)`) it needs to add appropriate overload candidates for the initializers in that type, which may take different numbers and types of parameters. The existing code for handling this case was using an ad hoc approach to try to enumerate the initializer declarations to consider, which might be found via inheritance, `extension` declarations, etc. In practice, the ad hoc logic for looking up initializers was just doing a subset of the work that already goes into doing member lookup. Changing the code so that it effectively does lookup for `MyType.__init` allows us to look up initializers in a way that is consistent with any other case of member lookup. Generalizing this lookup step brings us one step closer to being able to go from an `enum` type `E` to an initializer defined on an `extension` of an `interface` that `E` conforms to. One casualty of using the ordinary lookup logic for initializers is that we used to pass the type being constructed down into the logic that enumerated the initializers, which made it easier to short-circuit the part of overload resolution that usually asks "what type does this candidate return." It might seem "obvious" that an initializer/constructor on type `Foo` should return a value of type `Foo`, but that isn't necessarily true. Consider the `__BuiltinFloatingPointType` interface, which requires all the built-in floating-point types (`float`, `double`, `half`) to have an initializer that can take a `float`. If we call that interface in a generic context for `T : __BuiltinFloatingPointType`, then we want to treat that initializer as returning `T` and not `__BuiltinFloatingPointType`. Without the ad hoc logic in initializer overload resolution, this is the exact problem that surfaced for the stdlib definition of `clamp`. The solution to the "what type does an initializer return" problem was to introduce a notion of a `ThisType`, which refers to the type of `this` in the body of an interface. More generally, we will eventually want to have the keyword `This` be the type-level equivalent of `this`, and be usable inside any type. The `calcThisType` function introduced here computes a reasonable `Type` to represent the value of `This` within a given declaration. Inside of concrete type it refers to the type itself, while in an `interface` it will always be a `ThisType`. The existing `ThisTypeSubstitution`s, previously only applied to associated types, now apply to `ThisType`s as well, in the same situations. The next roadblock for making the simple declarations for `__EnumType` work was that the lookup logic was only doing lookup through inheritance relationships when the type being looked up in was an `interface`. The logic in play was reasonable: if you are doing lookup in a type `T` that inherits from `IFoo`, then why bother looking for `IFoo::bar` when there must be a `T::bar` if `T` actually implements the interface? The catch in this case is that `IFoo::bar` might not be a requirement of `IFoo`, but rather a concrete method added via an `extension`, in which case `T` need not have its own concrete `bar`. The simple/obvious fix here was to make the lookup logic always include inherited members, even when looking up through a concrete type. Of course, if we allow lookup to see `IFoo::bar` when looking up on `T`, then we have the problem that both `T::bar` and `IFoo::bar` show up in the lookup results, and potentially lead to an "ambiguous overload" error. This problem arises for any interface rquirement (so both methods and associated types right now). In order to get around it, I added a somewhat grungy check for comparing overload candidates (during overload resolution) or `LookupResultItem`s (during resolution of simple overloaded identifiers) that considers a member of a concrete type as automatically "better" than a member of an interface. The Right Way to solve this problem in the long run requires some more subtlety, but for now this check should Just Work. One final wrinkle is that due to our IR lowering pass being a bit overzealous, we currently end up trying to emit IR for those new `__init` declarations, which ends up causing us to try and emit IR for a `ThisType`. That is a case that will require some subtlty to handle correctly down the line, for for now we do the expedient thing and emit the `ThisType` for `IFoo` as `IFoo` itself, which is not especially correct, but doesn't matter since the concrete initializer won't ever be called. * testing: add more debug output to Unix process launch function * testing: increase timeout when running command-line tests
1 parent 895fcff commit 2e52217

20 files changed

+641
-185
lines changed

source/core/unix/slang-unix-process-util.cpp

+13-2
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,23 @@ namespace Slang {
106106
int stderrPipe[2];
107107

108108
if (pipe(stdoutPipe) == -1)
109+
{
110+
fprintf(stderr, "error: `pipe` failed\n");
109111
return SLANG_FAIL;
112+
}
110113

111114
if (pipe(stderrPipe) == -1)
115+
{
116+
fprintf(stderr, "error: `pipe` failed\n");
112117
return SLANG_FAIL;
118+
}
113119

114120
pid_t childProcessID = fork();
115121
if (childProcessID == -1)
122+
{
123+
fprintf(stderr, "error: `fork` failed\n");
116124
return SLANG_FAIL;
125+
}
117126

118127
if (childProcessID == 0)
119128
{
@@ -166,9 +175,9 @@ namespace Slang {
166175
return SLANG_FAIL;
167176
}
168177

169-
// Set a timeout of ten seconds;
178+
// Set a timeout of twenty seconds;
170179
// we really shouldn't wait too long...
171-
int pollTimeout = 10000;
180+
int pollTimeout = 20000;
172181
int pollResult = poll(pollInfos, pollInfoCount, pollTimeout);
173182
if (pollResult <= 0)
174183
{
@@ -178,6 +187,7 @@ namespace Slang {
178187
continue;
179188

180189
// timeout or error...
190+
fprintf(stderr, "error: `poll` failed or timed out\n");
181191
return SLANG_FAIL;
182192
}
183193

@@ -228,6 +238,7 @@ namespace Slang {
228238
pid_t terminatedProcessID = waitpid(childProcessID, &childStatus, 0);
229239
if (terminatedProcessID == -1)
230240
{
241+
fprintf(stderr, "error: `waitpid` failed\n");
231242
return SLANG_FAIL;
232243
}
233244

source/slang/core.meta.slang

+42
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,48 @@ interface __EnumType
5252
associatedtype __Tag : __BuiltinIntegerType;
5353
};
5454

55+
// Use an extension to declare that every `enum` type
56+
// inherits an initializer based on the tag type.
57+
//
58+
// Note: there is an important and subtle point here.
59+
// If we declared these initializers inside the `interface`
60+
// declaration above, then they would implicitly be
61+
// *requirements* of the `__EnumType` interface, and any
62+
// type that declares conformance to it would need to
63+
// provide implementations. That would put the onus on
64+
// the semantic checker to synthesize such initializers
65+
// when conforming an `enum` type to `__EnumType` (just
66+
// as it currently synthesizes the `__Tag` requirement.
67+
// Putting the declaration in an `extension` makes them
68+
// concrete declerations rather than interface requirements.
69+
// (Admittedly, they are "concrete" declarations with
70+
// no bodies, because currently all initializers are
71+
// assumed to be intrinsics).
72+
//
73+
// TODO: It might be more accurate to express this as:
74+
//
75+
// __generic<T:__EnumType> extension T { ... }
76+
//
77+
// That alternative would express an extension of every
78+
// type that conforms to `__EnumType`, rather than an
79+
// extension of `__EnumType` itself. The distinction
80+
// is subtle, and unfortunately not one the Slang type
81+
// checker is equiped to handle right now. For now we
82+
// will stick with the syntax that actually works, even
83+
// if it might be the less technically correct one.
84+
//
85+
//
86+
extension __EnumType
87+
{
88+
// TODO: this should be a single initializer using
89+
// the `__Tag` associated type from the `__EnumType`
90+
// interface, but right now the scoping for looking
91+
// up that type isn't working right.
92+
//
93+
__init(int value);
94+
__init(uint value);
95+
}
96+
5597
// A type resulting from an `enum` declaration
5698
// with the `[flags]` attribute.
5799
interface __FlagsEnumType : __EnumType

source/slang/core.meta.slang.h

+46-4
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,48 @@ SLANG_RAW(" // conflict if a user had an `enum` case called `Tag`\n")
5252
SLANG_RAW(" associatedtype __Tag : __BuiltinIntegerType;\n")
5353
SLANG_RAW("};\n")
5454
SLANG_RAW("\n")
55+
SLANG_RAW("// Use an extension to declare that every `enum` type\n")
56+
SLANG_RAW("// inherits an initializer based on the tag type.\n")
57+
SLANG_RAW("//\n")
58+
SLANG_RAW("// Note: there is an important and subtle point here.\n")
59+
SLANG_RAW("// If we declared these initializers inside the `interface`\n")
60+
SLANG_RAW("// declaration above, then they would implicitly be\n")
61+
SLANG_RAW("// *requirements* of the `__EnumType` interface, and any\n")
62+
SLANG_RAW("// type that declares conformance to it would need to\n")
63+
SLANG_RAW("// provide implementations. That would put the onus on\n")
64+
SLANG_RAW("// the semantic checker to synthesize such initializers\n")
65+
SLANG_RAW("// when conforming an `enum` type to `__EnumType` (just\n")
66+
SLANG_RAW("// as it currently synthesizes the `__Tag` requirement.\n")
67+
SLANG_RAW("// Putting the declaration in an `extension` makes them\n")
68+
SLANG_RAW("// concrete declerations rather than interface requirements.\n")
69+
SLANG_RAW("// (Admittedly, they are \"concrete\" declarations with\n")
70+
SLANG_RAW("// no bodies, because currently all initializers are\n")
71+
SLANG_RAW("// assumed to be intrinsics).\n")
72+
SLANG_RAW("//\n")
73+
SLANG_RAW("// TODO: It might be more accurate to express this as:\n")
74+
SLANG_RAW("//\n")
75+
SLANG_RAW("// __generic<T:__EnumType> extension T { ... }\n")
76+
SLANG_RAW("//\n")
77+
SLANG_RAW("// That alternative would express an extension of every\n")
78+
SLANG_RAW("// type that conforms to `__EnumType`, rather than an\n")
79+
SLANG_RAW("// extension of `__EnumType` itself. The distinction\n")
80+
SLANG_RAW("// is subtle, and unfortunately not one the Slang type\n")
81+
SLANG_RAW("// checker is equiped to handle right now. For now we\n")
82+
SLANG_RAW("// will stick with the syntax that actually works, even\n")
83+
SLANG_RAW("// if it might be the less technically correct one.\n")
84+
SLANG_RAW("//\n")
85+
SLANG_RAW("//\n")
86+
SLANG_RAW("extension __EnumType\n")
87+
SLANG_RAW("{\n")
88+
SLANG_RAW(" // TODO: this should be a single initializer using\n")
89+
SLANG_RAW(" // the `__Tag` associated type from the `__EnumType`\n")
90+
SLANG_RAW(" // interface, but right now the scoping for looking\n")
91+
SLANG_RAW(" // up that type isn't working right.\n")
92+
SLANG_RAW(" //\n")
93+
SLANG_RAW(" __init(int value);\n")
94+
SLANG_RAW(" __init(uint value);\n")
95+
SLANG_RAW("}\n")
96+
SLANG_RAW("\n")
5597
SLANG_RAW("// A type resulting from an `enum` declaration\n")
5698
SLANG_RAW("// with the `[flags]` attribute.\n")
5799
SLANG_RAW("interface __FlagsEnumType : __EnumType\n")
@@ -153,7 +195,7 @@ for (int tt = 0; tt < kBaseTypeCount; ++tt)
153195
// TODO: should this cover the full gamut of integer types?
154196
case BaseType::Int:
155197
case BaseType::UInt:
156-
SLANG_RAW("#line 153 \"core.meta.slang\"")
198+
SLANG_RAW("#line 195 \"core.meta.slang\"")
157199
SLANG_RAW("\n")
158200
SLANG_RAW(" __generic<T:__EnumType>\n")
159201
SLANG_RAW(" __init(T value);\n")
@@ -169,7 +211,7 @@ SLANG_RAW(" __init(T value);\n")
169211

170212
// Declare built-in pointer type
171213
// (eventually we can have the traditional syntax sugar for this)
172-
SLANG_RAW("#line 168 \"core.meta.slang\"")
214+
SLANG_RAW("#line 210 \"core.meta.slang\"")
173215
SLANG_RAW("\n")
174216
SLANG_RAW("\n")
175217
SLANG_RAW("__generic<T>\n")
@@ -231,7 +273,7 @@ sb << " __init(T value);\n";
231273
sb << " __init(vector<T,N> value);\n";
232274

233275
sb << "};\n";
234-
SLANG_RAW("#line 214 \"core.meta.slang\"")
276+
SLANG_RAW("#line 256 \"core.meta.slang\"")
235277
SLANG_RAW("\n")
236278
SLANG_RAW("\n")
237279
SLANG_RAW("__generic<T = float, let R : int = 4, let C : int = 4>\n")
@@ -1216,7 +1258,7 @@ for (auto op : binaryOps)
12161258
sb << "__intrinsic_op(" << int(op.opCode) << ") matrix<" << resultType << ",N,M> operator" << op.opName << "(" << leftQual << "matrix<" << leftType << ",N,M> left, " << rightType << " right);\n";
12171259
}
12181260
}
1219-
SLANG_RAW("#line 1198 \"core.meta.slang\"")
1261+
SLANG_RAW("#line 1240 \"core.meta.slang\"")
12201262
SLANG_RAW("\n")
12211263
SLANG_RAW("\n")
12221264
SLANG_RAW("// Specialized function\n")

source/slang/slang-check-conversion.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ namespace Slang
607607
overloadContext.baseExpr = nullptr;
608608
overloadContext.mode = OverloadResolveContext::Mode::JustTrying;
609609

610-
AddTypeOverloadCandidates(toType, overloadContext, toType);
610+
AddTypeOverloadCandidates(toType, overloadContext);
611611

612612
// After all of the overload candidates have been added
613613
// to the context and processed, we need to see whether

source/slang/slang-check-decl.cpp

+99-18
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,14 @@ namespace Slang
161161
if(as<SimpleTypeDecl>(decl))
162162
return true;
163163

164+
// Initializer/constructor declarations are effectively `static`
165+
// in Slang. They behave like functions that return an instance
166+
// of the enclosing type, rather than as functions that are
167+
// called on a pre-existing value.
168+
//
169+
if(as<ConstructorDecl>(decl))
170+
return true;
171+
164172
// Things nested inside functions may have dependencies
165173
// on values from the enclosing scope, but this needs to
166174
// be dealt with via "capture" so they are also effectively
@@ -1197,6 +1205,18 @@ namespace Slang
11971205
DeclRef<Decl> requiredMemberDeclRef,
11981206
RefPtr<WitnessTable> witnessTable)
11991207
{
1208+
// Sanity check: if are checking whether a type `T`
1209+
// implements, say, `IFoo::bar` and lookup of `bar`
1210+
// in type `T` yielded `IFoo::bar`, then that shouldn't
1211+
// be treated as a valid satisfaction of the requirement.
1212+
//
1213+
// TODO: Ideally this check should be comparing the `DeclRef`s
1214+
// and not just the `Decl`s, but we currently don't get exactly
1215+
// the same substitutions when we see the inherited `IFoo::bar`.
1216+
//
1217+
if(memberDeclRef.getDecl() == requiredMemberDeclRef.getDecl())
1218+
return false;
1219+
12001220
// At a high level, we want to check that the
12011221
// `memberDecl` and the `requiredMemberDeclRef`
12021222
// have the same AST node class, and then also
@@ -2524,6 +2544,78 @@ namespace Slang
25242544
getSink()->diagnose(decl->targetType.exp, Diagnostics::unimplemented, "expected a nominal type here");
25252545
}
25262546

2547+
RefPtr<Type> SemanticsVisitor::calcThisType(DeclRef<Decl> declRef)
2548+
{
2549+
if( auto interfaceDeclRef = declRef.as<InterfaceDecl>() )
2550+
{
2551+
// In the body of an `interface`, a `This` type
2552+
// refers to the concrete type that will eventually
2553+
// conform to the interface and fill in its
2554+
// requirements.
2555+
//
2556+
RefPtr<ThisType> thisType = new ThisType();
2557+
thisType->setSession(getSession());
2558+
thisType->interfaceDeclRef = interfaceDeclRef;
2559+
return thisType;
2560+
}
2561+
else if (auto aggTypeDeclRef = declRef.as<AggTypeDecl>())
2562+
{
2563+
// In the body of an ordinary aggregate type,
2564+
// such as a `struct`, the `This` type just
2565+
// refers to the type itself.
2566+
//
2567+
// TODO: If/when we support `class` types
2568+
// with inheritance, then `This` inside a class
2569+
// would need to refer to the eventual concrete
2570+
// type, much like the `interface` case above.
2571+
//
2572+
return DeclRefType::Create(
2573+
getSession(),
2574+
aggTypeDeclRef);
2575+
}
2576+
else if (auto extDeclRef = declRef.as<ExtensionDecl>())
2577+
{
2578+
// In the body of an `extension`, the `This`
2579+
// type refers to the type being extended.
2580+
//
2581+
// Note: we currently have this loop back
2582+
// around through `calcThisType` for the
2583+
// type being extended, rather than just
2584+
// using it directly. This makes a difference
2585+
// for polymorphic types like `interface`s,
2586+
// and there are reasonable arguments for
2587+
// the validity of either option.
2588+
//
2589+
// Does `extension IFoo` mean extending
2590+
// exactly the type `IFoo` (an existential,
2591+
// which could at runtime be a value of
2592+
// any type conforming to `IFoo`), or does
2593+
// it implicitly extend every type that
2594+
// conforms to `IFoo`? The difference is
2595+
// significant, and we need to make a choice
2596+
// sooner or later.
2597+
//
2598+
auto targetType = GetTargetType(extDeclRef);
2599+
return calcThisType(targetType);
2600+
}
2601+
else
2602+
{
2603+
return nullptr;
2604+
}
2605+
}
2606+
2607+
RefPtr<Type> SemanticsVisitor::calcThisType(Type* type)
2608+
{
2609+
if( auto declRefType = as<DeclRefType>(type) )
2610+
{
2611+
return calcThisType(declRefType->declRef);
2612+
}
2613+
else
2614+
{
2615+
return type;
2616+
}
2617+
}
2618+
25272619
RefPtr<Type> SemanticsVisitor::findResultTypeForConstructorDecl(ConstructorDecl* decl)
25282620
{
25292621
// We want to look at the parent of the declaration,
@@ -2538,27 +2630,16 @@ namespace Slang
25382630
parent = genericParent->ParentDecl;
25392631
}
25402632

2541-
// Now look at the type of the parent (or grandparent).
2542-
if (auto aggTypeDecl = as<AggTypeDecl>(parent))
2543-
{
2544-
// We are nested in an aggregate type declaration,
2545-
// so the result type of the initializer will just
2546-
// be the surrounding type.
2547-
return DeclRefType::Create(
2548-
getSession(),
2549-
makeDeclRef(aggTypeDecl));
2550-
}
2551-
else if (auto extDecl = as<ExtensionDecl>(parent))
2552-
{
2553-
// We are nested inside an extension, so the result
2554-
// type needs to be the type being extended.
2555-
return extDecl->targetType.type;
2556-
}
2557-
else
2633+
// The result type for a constructor is whatever `This` would
2634+
// refer to in the body of the outer declaration.
2635+
//
2636+
auto thisType = calcThisType(makeDeclRef(parent));
2637+
if( !thisType )
25582638
{
25592639
getSink()->diagnose(decl, Diagnostics::initializerNotInsideType);
2560-
return nullptr;
2640+
thisType = getSession()->getErrorType();
25612641
}
2642+
return thisType;
25622643
}
25632644

25642645
void SemanticsDeclHeaderVisitor::visitConstructorDecl(ConstructorDecl* decl)

0 commit comments

Comments
 (0)