Skip to content

Commit 99366e7

Browse files
Tim Foleycsyonghe
Tim Foley
andauthored
Fix an issue with explicit enum tag types (shader-slang#1495)
The basic problem here was that in a declaration like: ```hlsl enum Color : uint { Red, Orange, ... } ``` The `: uint` bit is represented as an `InheritanceDecl`, because that is what we use to represent the syntactic form of inheritance clauses like that. At the point where we parse the `InheritanceDecl` we don't yet know whether it represents a base interface or a "tag type" like `uint` in this case. The root problem that is then created is: an `enum` type is *not* a subtype of its "tag type," and treating it like a subtype can create problems. The main problem that arises is that looking in a type like `Color` will find both the members of color *and* the members of `uint`. In the case of things like `__init` declarations, that creates a problem where the `Color` type has two different `__init`s that take a `uint`: * The one it inherits from `uint` via that `InheritanceDecl` (even though it shouldn't) * The one it gets via an extension just for conforming to `__EnumType` (a non-user-exposed `interface` in the standard library) Because both of those `__init`s are inherited, neither is preferred over the other one and they create an ambiguity if somebody tries to write: ```hlsl uint u = ...; Colorc = Color(u); ``` The solution used in this PR is to add a compiler-internal modifier to the `InheritanceDecl` that introduces a "tag type" to an `enum`, in an early phase of checking (one of the ones that occurs before it is legal to enumerate the bases of a type). Then the lookup process is modified to ignore `InheritanceDecl`s with that modifier when doing lookup in super-types (since the declaration does *not* indicate a subtype/supertype relationship). This appears to get the basic feature working again, although it is possible that there are other parts of the compiler that use `InheritanceDecl`s and mistakently assume that all `InheritanceDecl`s introduce subtype/supertype relationships. We probably need to do a significant audit of the code to start being more clear about the nature of the relationships such declarations introduce. Such steps are left to future changes. Co-authored-by: Yong He <yonghe@outlook.com>
1 parent 2bfe62a commit 99366e7

File tree

5 files changed

+58
-0
lines changed

5 files changed

+58
-0
lines changed

source/slang/slang-ast-modifier.h

+3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ class ExportedModifier : public Modifier { SLANG_CLASS(ExportedModifier)};
2929
class ConstExprModifier : public Modifier { SLANG_CLASS(ConstExprModifier)};
3030
class GloballyCoherentModifier : public Modifier { SLANG_CLASS(GloballyCoherentModifier)};
3131

32+
/// A modifier that indicates an `InheritanceDecl` should be ignored during name lookup (and related checks).
33+
class IgnoreForLookupModifier : public Modifier { SLANG_CLASS(IgnoreForLookupModifier) };
34+
3235
// A modifier that marks something as an operation that
3336
// has a one-to-one translation to the IR, and thus
3437
// has no direct definition in the high-level language.

source/slang/slang-check-decl.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -2985,6 +2985,16 @@ namespace Slang
29852985
// For now we will just be harsh and require it
29862986
// to be one of a few builtin types.
29872987
validateEnumTagType(tagType, tagTypeInheritanceDecl->loc);
2988+
2989+
// Note: The `InheritanceDecl` that introduces a tag
2990+
// type isn't actually representing a super-type of
2991+
// the `enum`, and things like name lookup need to
2992+
// know to ignore that "inheritance" relationship.
2993+
//
2994+
// We add a modifier to the `InheritanceDecl` to ensure
2995+
// that it can be detected and ignored by such steps.
2996+
//
2997+
addModifier(tagTypeInheritanceDecl, m_astBuilder->create<IgnoreForLookupModifier>());
29882998
}
29892999
decl->tagType = tagType;
29903000

source/slang/slang-lookup.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,14 @@ static void _lookUpMembersInSuperTypeDeclImpl(
528528
{
529529
ensureDecl(semantics, inheritanceDeclRef.getDecl(), DeclCheckState::CanUseBaseOfInheritanceDecl);
530530

531+
// Some things that are syntactically `InheritanceDecl`s don't actually
532+
// represent a subtype/supertype relationship, and thus we shouldn't
533+
// include members from the base type when doing lookup in the
534+
// derived type.
535+
//
536+
if(inheritanceDeclRef.getDecl()->hasModifier<IgnoreForLookupModifier>())
537+
continue;
538+
531539
auto baseType = getSup(astBuilder, inheritanceDeclRef);
532540
if( auto baseDeclRefType = as<DeclRefType>(baseType) )
533541
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// explicit-tag-type.slang
2+
3+
// Test that the underlying "tag type" of an `enum` can be set.
4+
5+
//TEST(compute):COMPARE_COMPUTE:
6+
7+
enum Channel : uint
8+
{
9+
Red,
10+
Green,
11+
Blue,
12+
Alpha,
13+
}
14+
15+
uint test(uint val)
16+
{
17+
Channel channel = Channel(val);
18+
uint u = uint(channel) + 1;
19+
channel = Channel(u & 0x3);
20+
return uint(channel);
21+
}
22+
23+
//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=outputBuffer
24+
RWStructuredBuffer<uint> outputBuffer;
25+
26+
[numthreads(4, 1, 1)]
27+
void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
28+
{
29+
uint tid = dispatchThreadID.x;
30+
uint inVal = tid;
31+
uint outVal = test(inVal);
32+
outputBuffer[tid] = outVal;
33+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
1
2+
2
3+
3
4+
0

0 commit comments

Comments
 (0)