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

Remove the void type #1460

Merged
merged 2 commits into from
Mar 29, 2021
Merged

Remove the void type #1460

merged 2 commits into from
Mar 29, 2021

Conversation

kvark
Copy link
Contributor

@kvark kvark commented Feb 23, 2021

Closes #1219
Closes #743

Basically, there is no utility right now for this type. It's obvious from looking at a function signature if a return statement is required, without the type.
If we ever feel that the type is needed, we can bring it back.

@kvark kvark requested a review from dneto0 February 23, 2021 20:12
@kvark kvark added the wgsl WebGPU Shading Language Issues label Feb 23, 2021
@kvark kvark added this to Needs Approval in WGSL Feb 23, 2021
wgsl/index.bs Outdated Show resolved Hide resolved
wgsl/index.bs Outdated Show resolved Hide resolved
wgsl/index.bs Outdated Show resolved Hide resolved
wgsl/index.bs Outdated Show resolved Hide resolved
wgsl/index.bs Outdated Show resolved Hide resolved
wgsl/index.bs Outdated Show resolved Hide resolved
@dneto0
Copy link
Contributor

dneto0 commented Mar 5, 2021

A wise person once said:

#1413

#1413 introduces a use case for being able to write down the void type.

@kvark
Copy link
Contributor Author

kvark commented Mar 8, 2021

I don't think #1413 (comment) presents a strong case for keeping void. It's niche, and trivial to do with the means we have, without new constructs or the void type.

@litherum
Copy link
Contributor

I think there's another option: keep the concept of the void type in the spec, but forbid authors from spelling it inside WGSL programs. We keep the philosophical type system simplicity that @dneto0 desired during today's office hours call, while removing the concept of the void type from every shader author's brain.

@kvark
Copy link
Contributor Author

kvark commented Mar 24, 2021

@dneto0 this is rebased now, please have another look!

@github-actions
Copy link
Contributor

Previews, as seen at the time of posting this comment:
WebGPU | IDL
WGSL
6fe2a28

@github-actions
Copy link
Contributor

Previews, as seen at the time of posting this comment:
WebGPU | IDL
WGSL
ab06bdd

@kvark
Copy link
Contributor Author

kvark commented Mar 29, 2021

Thank you for reviewing and rebasing!

@dneto0 dneto0 merged commit 2390912 into gpuweb:main Mar 29, 2021
WGSL automation moved this from Needs Approval to Done Mar 29, 2021
@kvark kvark deleted the void branch March 29, 2021 20:08
ben-clayton added a commit to ben-clayton/Tint that referenced this pull request Apr 13, 2021
Was removed with:
gpuweb/gpuweb#1460

Bug: tint:677
Change-Id: If08cb450189c6158561051ef6e8f2439c60bc010
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47140
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
ben-clayton added a commit to ben-clayton/webgpu-samples that referenced this pull request Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wgsl WebGPU Shading Language Issues
Projects
WGSL
Done
Development

Successfully merging this pull request may close these issues.

Considering making ARROW function_type_decl optional for void functions Follow-up: Using the void type
4 participants