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

call_statement: should only be able to call a function that yields void? #1413

Closed
dneto0 opened this issue Feb 3, 2021 · 11 comments
Closed
Assignees
Labels
wgsl WebGPU Shading Language Issues
Projects
Milestone

Comments

@dneto0
Copy link
Contributor

dneto0 commented Feb 3, 2021

That can catch some programming errors.

If you really want to call a value-producing function at statement level, only for its side effects, then you can always capture its result in a const-declaration.

  const toss_it: T = CallForSideEffects();

Or

  const _ := CallForSideEffects();  // if we have type deduction

  _  := CallForSideEffects();  // even more aggressive throwaway with type deduction

  // Invent a void-constructor  explicitly for this purpose. 
  // It's a sink for values, producing void.
  void(CallForSideEffects());  
@dneto0 dneto0 added the wgsl WebGPU Shading Language Issues label Feb 3, 2021
@dneto0
Copy link
Contributor Author

dneto0 commented Feb 3, 2021

#1412 is a somewhat related grammar change.

@dneto0 dneto0 added this to the MVP milestone Feb 3, 2021
@dneto0 dneto0 changed the title call_statement: Limit to calling a function that yields void? call_statement: should only be able to call a function that yields void? Feb 4, 2021
@kvark
Copy link
Contributor

kvark commented Feb 6, 2021

Played with this in Naga, and I'm strongly in favor of this. 🚀

@dneto0 dneto0 added this to Needs Discussion in WGSL Mar 5, 2021
@dneto0
Copy link
Contributor Author

dneto0 commented Mar 5, 2021

I propose adding a void constructor builtin function:

    void(sink:T) -> void;

A call to the void function can appear as a statement in a function body. It evaluates its argument, and yields no value.

Also, add the rule that when a statement is a function calll, that function must have void return type.


edited to add the new validation rule

@dneto0 dneto0 self-assigned this Mar 5, 2021
@dneto0 dneto0 moved this from Needs Discussion to Needs Approval in WGSL Mar 5, 2021
@kvark
Copy link
Contributor

kvark commented Mar 8, 2021

Let's not mix this into the void(sink) suggestion, please file it separately?
I don't see why it's needed. Users can always just declare a constant to get a result of anything.

@ben-clayton
Copy link
Collaborator

I don't see why it's needed. Users can always just declare a constant to get a result of anything.

It's not uncommon to have compilers warn about unused variables. (void) expr is the typical way to tell a C++ compiler that an expression is intentionally unused.

@kvark
Copy link
Contributor

kvark commented Mar 8, 2021

Ok, we can add void(sink:T) once we agree on the warnings. It doesn't require void type anyway.

Btw, since #1413 (comment) was written I've remade the support for function calling in Naga, and now this proposal doesn't give us anything at all. We are now totally cool to support:

my_function();

@dneto0
Copy link
Contributor Author

dneto0 commented Mar 9, 2021

Let's not mix this into the void(sink) suggestion, please file it separately?

I filed #1499 for the void constructor idea.

@dneto0
Copy link
Contributor Author

dneto0 commented Mar 9, 2021

Discussed 2021-03-09

@kdashg kdashg moved this from Needs Approval to Resolved: Needs Specification Work in WGSL Mar 9, 2021
@dneto0
Copy link
Contributor Author

dneto0 commented Mar 31, 2021

Hey @jdashg you marked this as resolved-needs spec. But reading the notes, I'm not clear there was consensus to make this an error. I don't know what the consensus is.

@kvark
Copy link
Contributor

kvark commented Apr 1, 2021

I read the consensus as:

  • don't allow the statement calls into functions with return types
  • follow-up with the ignore function at some point

@kdashg
Copy link
Contributor

kdashg commented Apr 26, 2022

I think this is closed by #2138.

@kdashg kdashg closed this as completed Apr 26, 2022
WGSL automation moved this from Resolved: Needs Specification Work to Done Apr 26, 2022
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this issue Sep 6, 2022
This PR adds the `skipLibCheck` option into the `tsconfig.json` file and
turns it on. Adding the flag takes a local full rebuild of the CTS
using the dawn CTS runner from 14-16s down to 8-9s.
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

No branches or pull requests

4 participants