Skip to content

Conversation

csyonghe
Copy link
Collaborator

@csyonghe csyonghe commented Aug 7, 2025

Closes #8061.

Along with the fix, also enhanced coercion/overload resolution to filter candidates based on the target type, allowing tests\language-feature\higher-order-functions\overloaded.slang to pass.

@csyonghe csyonghe requested a review from a team as a code owner August 7, 2025 01:24
@csyonghe csyonghe added the pr: non-breaking PRs without breaking changes label Aug 7, 2025
@csyonghe csyonghe requested a review from juliusikkala August 7, 2025 03:28
@csyonghe csyonghe enabled auto-merge August 7, 2025 08:01
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 had also started on this yesterday (minus the overload part), but got preoccupied with something else and luckily didn't get too far yet. I had stopped when I realized I'd need a function like the maybeExpandArgList you implemented 😄

It's good that we can remove isInVariadicGenerics. isKeywordAvailable is in general a really nice solution IMO, since now we can add new keywords without breaking changes.

One more thing I'd like to see tested, is this case:

Tuple<int, float> x = (2, 3.0f);

float arr[2];
int i = 0;
expand arr[i++] = float(each x);

or in general, expanding concrete tuples outside of function call parameters. I encountered a type-related error with that code even after allowing expand-each outside of variadic generics. I'm not on a PC with slangc currently, so can't check what it was or if this PR has already fixed it. Either way, I'd feel more comfortable with something like that being in the tests.

@csyonghe
Copy link
Collaborator Author

csyonghe commented Aug 7, 2025

@juliusikkala I don't think your code is valid though, because there is no overload of float.init that will take a generic type like each T without any constraints.

@csyonghe
Copy link
Collaborator Author

csyonghe commented Aug 7, 2025

Again, even with concrete types, the expression inside expand is checked before expansion, similar to other generic cases.

@juliusikkala
Copy link
Contributor

Ah ok, that's a bit unfortunate :( but if that's the desired behaviour, then LGTM

@csyonghe csyonghe added this pull request to the merge queue Aug 7, 2025
Merged via the queue into shader-slang:master with commit 7cd8130 Aug 7, 2025
19 checks passed
@csyonghe csyonghe deleted the expand branch August 7, 2025 17:49
szihs pushed a commit to szihs/slang that referenced this pull request Aug 19, 2025
Closes shader-slang#8061.

Along with the fix, also enhanced coercion/overload resolution to filter
candidates based on the target type, allowing
`tests\language-feature\higher-order-functions\overloaded.slang` to
pass.
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.

Expand statements not working with tuples, despite docs claiming it does
2 participants