Skip to content

Conversation

@csyonghe
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented May 13, 2025

CLA assistant check
All committers have signed the CLA.

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.

This proposal looks great. I've walked through the plan as documented here and it seems like all the points are covered and it should be implementable on top of the current compiler codebase (rather than requiring any significant rearchitecture or overhauls).

This would be a really nice feature to get into the language ASAP, so that we can give our users (and ourselves) more confidence in the robustness of code that uses interface-typed parameters/variables/etc.

Copy link
Contributor

@juliusikkala juliusikkala left a comment

Choose a reason for hiding this comment

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

I like this proposal a lot, I've already encountered a few situations where these would've been helpful. I especially appreciate making some the default behaviour for 2026.

1. If a function returns `some IFoo`, then all return statements must return values of exactly the same type.
1. If a function contains a `out some IFoo` parameter, then all values assignmened to the parameter must have exactly the same type.
1. Two different var decls of `some IFoo` type are considered to have **different** types.
1. A `dyn IFoo` type is valid only if `IFoo` is an interface type with `dyn` qualifier.
Copy link
Contributor

@ArielG-NV ArielG-NV May 19, 2025

Choose a reason for hiding this comment

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

This line reads a bit confusingly (given the example provided later in the doc dyn IFoo d = f; and dyn IFoo v1;).

For clarification, this means that given the following:

dyn interface IDyn{};
some interface ISome{};
struct ISomeImpl : ISome {};
  1. I cannot do (in a function's scope):
    dyn ISome dynVar;

  2. I can do (in a function's scope):
    dyn IDyn dynVar;

  3. I can do (in a function's scope):

some ISome someVar = ISomeImpl();
dyn ISome dynVar = someVar;
  1. I can do (in a function's scope):
dyn ISome dynVar = ISomeImpl();

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think some can be used on interface type, from the proposal, some is supposed to be used only on var decl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The understanding is correct, although I would comment that IInterface() is never a valid expression. You can construct an interface typed value using the interface type.

Comment on lines +47 to +49
Any type that conforms to one or more `dyn` interface is subject to these restrictions:

1. The type itself must be an ordinary data type, meaning that it cannot contain any fields that are opaque or non-copyable or unsized.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this section intentionally omitting unless language version is 2025 or -enable-experimental-dynamic-dispatch flag is present:?
(I ask since this PR is a breaking change without the clause)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we allow opaque type in an interface that can be used in dynamic dispatch for now.
So I guess this is intentionally omitting.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I think my memory is correct. Look at this line.

Another big down side is that our dynamic dispatch logic can only handle ordinary data types. If a type implementing an interface contains opaque types such as Texture2D, the dynamic dispatch pass will fail and give user confusing error messages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this applies in all cases regardless of language version or experiemental flag setting. We can never support opaque types in dynamic dispatch.

@Dolkar Dolkar mentioned this pull request May 20, 2025
@csyonghe csyonghe merged commit ed9f9b7 into shader-slang:main May 27, 2025
1 check passed
{
out = f; // error: type mismatch.
out.computeFoo(1.0); // OK, calling mutable methods on a some type.
dyn IFoo d = f; // OK, assigning `some IFoo` to `dyn IFoo`.
Copy link
Contributor

@ArielG-NV ArielG-NV May 29, 2025

Choose a reason for hiding this comment

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

This should error, 1. A `dyn IFoo` type is valid only if `IFoo` is an interface type with `dyn` qualifier..

There are no exceptions to this rule, interface types are not automatically assigned dyn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants