https://github.com/shader-slang/slang
Revision bc0d0f9e2a5a7297d8559a2ee1e3bd59454813cf authored by Tim Foley on 27 August 2020, 17:46:50 UTC, committed by GitHub on 27 August 2020, 17:46:50 UTC
* Clean up the way that lookup "through" a base type is encoded

In order to undestand this change, it is important to undestand how lookup through base interfaces works prior to this change. In order to understand *that* it helps to be reminded of how inheritance relationships get encoded in the AST.

Suppose the user writes:

    struct Base { int val; }
    struct Derived : Base { ... }
    ...
    Derived d = ...;
    int v = d.val;

The question is how an expression like `d.val` gets semantically checked, and how it is encoded into the IR after semantic checking. You might assume it gets checked and encoded so that we end up with:

    int v = ((Base) d).val;

and that seems like it should Just Work... so of course that isn't what Slang has been doing. Instead, we relied on the fact that the inheritance relationship `Derived : Base` is represented as an `InheritanceDecl` member of the `Derived` type, and we ended up checking the code into something like:

   int v = d.<anonymous>.val;

where `<anonymous>` stands in for the name of the `InheritanceDecl` that represents inheritance from `Base`. This design choice makes a limited amount of sense when you consider how inheritance would typically be lowered to a C-like output language:

    // struct Derived : Base { ... }
    //   =>
    struct Derived { Base base; ... }

The problem with that encoding is that it really doesn't make sense for almost any other scenario. In particular, if you have a generic type parameter `T` that was constrianed with `T : ISomething`, then the constraint isn't even technically a *member* of the type parameter `T`, so expressing thing as a member reference in the AST is completely incorrect. Unfortunately, by the time it was clear that we needed something better, a bunch of implementation work was done based on the existing representation.

This change tries to clean things up so that lookup of a super-type member through a value of a sub-type does the obvious thing: cast the value to the super-type and then look up the member (as in `((Base) d).val`).

The core of the change is that in lookup, instead of creating `Constraint` breadcrumbs whenever we are looking up in a super-type (with a reference to the `TypeConstraintDecl` being used) we instead use `SuperType` breadcrumbs (with a reference to a `SubtypeWitness`). Then when we create the expression from a `LookupResultItem`, we translate any `SuperType` breadcrumbs into `CastToSuperTypeExpr`s (an expression type that already existed).

This change also adds support for lookup through the `This` type in the context of an interface, and in order for that to work we need a new kind of subtype witness to represent the knowledge that a `This` type is a subtype of the enclosing interface. Making that work forces us to change the representation of `TransitiveSubtypeWitness` so that it takes a pair of subtype witnesses (and not one subtype witness plus one `TypeConstraintDecl`). For the most part this is a small change, but it raises the possibility that some pieces of the code aren't going to be robust against all possible shapes of subtype witnesses.

The IR lowering logic has relied on the weird `d.<anonymous>` representation in order to ensure that when looking up interface members we weren't always casting to the interface type (which would create a `makeExistential` instruction), and then calling using that. Basically, the IR lowering would ignore the `d.<anonymous>` part and just emit `d`, but we can't do that for `((Base) d)` or `((IThing) d)` because whehter or not we should actually perform the cast depends on context.

For now we solve that problem by adding specific logic to ignore up-casts to interface types when they appear in member expressions or method calls. A more robust solution might be needed down the line, but this seems to work in practice.

All of this work is cleanup that I found was needed in order to make `extension`s of `interface` types workable.

* fixup: disable an incorrect test
1 parent c936433
History
Tip revision: bc0d0f9e2a5a7297d8559a2ee1e3bd59454813cf authored by Tim Foley on 27 August 2020, 17:46:50 UTC
Clean up the way that lookup "through" a base type is encoded (#1519)
Tip revision: bc0d0f9
File Mode Size
docs
examples
external
extras
prelude
source
tests
tools
.editorconfig -rw-r--r-- 937 bytes
.gitattributes -rw-r--r-- 95 bytes
.gitignore -rw-r--r-- 759 bytes
.gitmodules -rw-r--r-- 774 bytes
.travis.yml -rw-r--r-- 2.1 KB
CODE_OF_CONDUCT.md -rw-r--r-- 3.1 KB
LICENSE -rw-r--r-- 1.1 KB
README.md -rw-r--r-- 7.4 KB
appveyor.yml -rw-r--r-- 4.0 KB
premake.bat -rw-r--r-- 120 bytes
premake5.lua -rw-r--r-- 38.6 KB
slang-com-helper.h -rw-r--r-- 4.8 KB
slang-com-ptr.h -rw-r--r-- 4.8 KB
slang-tag-version.h -rw-r--r-- 36 bytes
slang.h -rw-r--r-- 129.5 KB
slang.sln -rw-r--r-- 13.7 KB
test.bat -rw-r--r-- 1.4 KB
travis_build.sh -rw-r--r-- 460 bytes
travis_test.sh -rw-r--r-- 435 bytes

README.md

back to top