diff options
| author | Theresa Foley <tfoleyNV@users.noreply.github.com> | 2021-11-03 22:44:21 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2021-11-03 22:44:21 -0700 |
| commit | af0f26d54ce39b6d7203646abd6e970b8113584c (patch) | |
| tree | a45f7fec689ce19b5e4178443361c772f31685c5 /source/slang/slang-lookup.cpp | |
| parent | 4b7e674fa9e9f8e325009183bec1ddf377b99bf1 (diff) | |
Fix an infinite-recursion bug in type-checking (#2004)
Fixes #1990
The underlying problem here is in the `ExtractExistentialType` AST node class.
An "existential" in current Slang is typically a value of interface type. When such a value is used in an operation, the type-checker "opens" the extistential so that subsequent type-checking steps can work with the (statically unknown) specific type of the value stored inside. The `ExtractExistentialType` AST node represents the type of an existential that has been "opened" in this way.
When the front-end performs lookup "into" a value with one of these types, it nees to use a reference to the original interface declaration with a "this-type substitution" that refers to the "opened" type (a this-type substitution tells the compiler the concrete type it should use in place of `This` in signatures within the interface; it allows compiler to "see" the right associated type definitions to use in a context).
Prior to this change, the implementation would store the specialized reference to the original interface declaration in the `ExtractExistentialType` node as part of its state. The catch there is that the specialized interface reference indirectly refers to the `ExtractExistentialType` AST node itself, creating a circularity. As soon as the front-end performs any operation that tries to recurse over that structure, it would go into an infinite loop.
The fix here sounds kind of like a hack, but seems to be pretty nice in practice. Instead of always storing the specialized interface reference, we instead store the few values that are needed to construct it, and then create and cache the actual reference on-demand. The on-demand created fields are not considered part of the state of the AST node for any kind of recursion or serialization, so they avoid the original problem.
A single test case was added that represents the original bug, and confirms the fix.
Diffstat (limited to 'source/slang/slang-lookup.cpp')
| -rw-r--r-- | source/slang/slang-lookup.cpp | 8 |
1 files changed, 7 insertions, 1 deletions
diff --git a/source/slang/slang-lookup.cpp b/source/slang/slang-lookup.cpp index 20670e7b3..e3093a3db 100644 --- a/source/slang/slang-lookup.cpp +++ b/source/slang/slang-lookup.cpp @@ -613,7 +613,13 @@ static void _lookUpMembersInSuperTypeImpl( } else if (auto extractExistentialType = as<ExtractExistentialType>(superType)) { - _lookUpMembersInSuperTypeDeclImpl(astBuilder, name, leafType, superType, leafIsSuperWitness, extractExistentialType->interfaceDeclRef, request, ioResult, inBreadcrumbs); + // We want lookup to be performed on the underlying interface type of the existential, + // but we need to have a this-type substitution applied to ensure that the result of + // lookup will have a comparable substitution applied (allowing things like associated + // types, etc. used in the signature of a method to resolve correctly). + // + auto interfaceDeclRef = extractExistentialType->getSpecializedInterfaceDeclRef(); + _lookUpMembersInSuperTypeDeclImpl(astBuilder, name, leafType, superType, leafIsSuperWitness, interfaceDeclRef, request, ioResult, inBreadcrumbs); } else if( auto thisType = as<ThisType>(superType) ) { |
