Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synthesize conformance for generic requirements. #5111

Merged
merged 5 commits into from
Sep 19, 2024

Conversation

csyonghe
Copy link
Collaborator

Closes #5035.

This PR adds the ability to synthesize generic interface requirements.
For example,

interface IBase
{
    static float get();
}
interface IBar : IBase
{
    float derivedMethod();
}
interface IFoo
{
     void method<T:IBar>();
}
struct FooImpl : IFoo
{
    // method here doesn't match IFoo.method exactly in that here we require
    // T:IBase and in IFoo.method we require T:IBar.
    // the compiler should be able to synthesize a
    // void method<T:IBar>() { method<T>(); }
    // to satisfy the requirement.
      void method<T:IBase>() { .. }
}

The compiler already has the ability to synthesize requirements based on what is provided, but it currently gives up for all generic requirements. This PR completes this missing piece and allows us to synthesize for generic requirements as well.

@csyonghe csyonghe added the pr: non-breaking PRs without breaking changes label Sep 19, 2024
jkwak-work
jkwak-work previously approved these changes Sep 19, 2024
Copy link
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@@ -2614,7 +2614,7 @@ namespace Slang
{
// We are referring to a bunch of declarations, each of which might be generic
LookupResult result;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this line?
It doesn't seem to be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Comment on lines +6 to +23
interface IBase
{
static float get();
}
interface IBar : IBase
{
float derivedMethod();
}

struct Bar : IBar
{
static float get() { return 1.0f; }
float derivedMethod() { return 2.0f; }
}

interface ITestInterface<Real : IFloat>
{
Real sample<T : IBar>(T t);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious if it would work when I have another method like sample<T:IBar2>(T t) where IBar2 inherits from IBase too.
It seems to be working as expected.

interface IBar2 : IBase
{
    float derivedMethod2();
}

struct Bar2 : IBar2
{
    static float get() { return 1.0f; }
    float derivedMethod2() { return 3.0f; }
}

interface ITestInterface<Real : IFloat>
{
    Real sample<T : IBar>(T t);
    Real sample<T : IBar2>(T t); // <=== This seems to be synthesized as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in this case, both requirements can be synthesized from a single implementation that is Real sample<T:IBae>().

tangent-vector
tangent-vector previously approved these changes Sep 19, 2024
Copy link
Contributor

@tangent-vector tangent-vector left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach itself looks good.

I have some questions about whether the semantics being implemented here are the ideal ones, but that's likely to be a detailed and longer-term discussion.

The current approach basically takes a requirement of the form:

X someFunc<T : IFoo, U : IBar>(T t, U u);

and synthesizes a body that invokes someFunc<T,Y>(t,u). However, it would also be intuitively reasonable to synthesize a body that simply invokes someFunc(t,u).

By not performing explicit specialization in the body, we would make it possible for a non-generic method to satisfy a generic requirement. For the above example of someFunc, a possible satisfying method might be:

X someFunc( IFoo t, IBar u ) { ... }

While it might at first glance seem obvious that generic requirements can only be satisfied by generic methods, the above example becomes less obvious in the presence of some and any:

// requirement:
X someFunc( some IFoo t, some IBar u );

// satisfying method:
X someFunc( any IFoo t, any IFoo u );

In this latter case it is more immediately obvious why a programmer might expect the method to satisfy the requirement. If we end up making some just be syntactic sugar for generics (as the equivalent feature works in Rust), then we seemingly can't avoid those sorts of cases.

The same basic problem can/will arise for class hierarchies, where a method with a parameter of a base class should in principle be able to satisfy a generic requirement where the type parameter is constrained to be a subclass:

// req:
void anotherFunc<T : BaseClass>(T thing);

// satisfying:
void anotherFunc(BaseClass thing);

I think we should land this as implemented here, but we probably need to step back and think carefully about what the desired semantics for synthesis should be in the longer run.

@csyonghe
Copy link
Collaborator Author

I thought about this problem and cannot conclude if allowing such synthesis is what the user want. It can introduce tricky scenarios where the sythesis can fail if the generic arguments cannot be inferred from actual arguments. E.g.:

interface IFoo
{
     X<n> method<int n>(); // n is not referenced by any parameter therefore not inferrable.
}
struct Foo : IFoo
{
    X<n> method<uint n>(){} // synthesis will fail, because `method();` is not resolvable.
}

So for the time being, I think it is safer to say we don't synthesis for non-matching generic requirements.

@csyonghe csyonghe merged commit 26ca9c5 into shader-slang:master Sep 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support synthesizing generic adaptor methods for interface requirements.
3 participants