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

Support extension on generic type. #4968

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Conversation

csyonghe
Copy link
Collaborator

@csyonghe csyonghe commented Aug 30, 2024

Closes #3960.

This PR enables the following code:

extension<T:IFoo> T : IBar
{
     // Make every type that conforms to IFoo also conform to IBar.
}

Our existing type system has almost everything to support this, in that we already allow things like:

__generic<T:IFoo>
extension MyType<T> : IBar {}

Supporting extending T directly isn't that much different from the MyType<T> case. The one challenge we need to overcome is we shoudn't be considering the extension for every existing type in the linkage. Instead, we disallow general unconstraint extension such as extension<T> T {...} and require all T to be constraint with some interface IFoo. Then we can register this extension under the IFoo decl.

When considering whether extension<T:IFoo> T:IBar{} can be applied to some type X, we go through all known base facets of X and see if there are any facet corrsponding to IFoo. If so, we will try to join X with T, and if it is successful, then we add the extension to the facet list of X. However, do so effectively makes X conform to IBar, so we need to repeat the process and add any extesnion<T:IBar> T : IBaz to the facet list...

The next thing to consider is to make sure not to recurse indefeinitely when there is a "circle" of extensions such as:

extension<T:IFoo> T:IBar {}
extension<T:IBar> T:IFoo {}

Theoretically we can make this work, since it just means that every type that conforms to IFoo also conforms to IBar and vice versa. But this is currently not implemented in this PR and we will issue a diagnostic if such circular extension is detected.

@csyonghe csyonghe added the pr: non-breaking PRs without breaking changes label Aug 30, 2024
@jkwak-work
Copy link
Collaborator

jkwak-work commented Aug 30, 2024

The syntax at first look was very confusing and I thought it was a typo.
It feels strange to have < and > right after extension, because < and > have been always some kind of suffix to template keyword, function or struct name.
*EDIT: I guess it still is a suffix to extension.

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.

Comment on lines +12 to +13
__generic<T:IFoo>
extension T
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is little different from the repo example from the issue.
And even with this PR, we still cannot do the following, can we?

extension IFoo

It looks like __generic<T:IFoo> is a workaround for the limitation but I am not sure what the limitation exactly is.
In other words, why we cannot do extension IFoo here, instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The whole purpose of the change is to get rid of extension IFoo from the language, because it does not make a lot of sense syntactically: you can't really add a requirement to an interface, because that will make all existing types conforming to the interface no longer conforming.

What extension IFoo really means is extension<T:IFoo> T, and this PR implements proper support for this, which will replace all existing uses of extension IFoo. The currently implementation of the compiler trying to support extension IFoo in a very hacky way that is not correct in many cases. It is much cleaner to just remove all these logic from the compiler and ban it forever.

@csyonghe
Copy link
Collaborator Author

csyonghe commented Aug 30, 2024

As for syntax, the only place we can write <> without the __generic keyword is for it to appear after extension, because extension themselves don't really have a name, and you can think of it as:

extension omittedNameForExtensionDecl<T:IFoo> for T : IBar
{
}

Note that when we write extension T {}, T is not the name for the extension decl, it is the target type that this extension decl is extending.

@csyonghe csyonghe merged commit 24df551 into shader-slang:master Aug 30, 2024
10 of 12 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.

"Ambiguous reference to This" in extension method returning This
2 participants