[naga wgsl-in] Attempt automatic conversion for arguments to user defined function calls

[jamienicol]

Connections
Fixes #5523

Description
When lowering arguments for a user-defined function call, avoid concretizing the argument types. Instead make use of the existing try_automatic_conversions() machinery to attempt to convert each argument to the type expected by the function. This is straightforward as user-defined functions only have a single overload.

This additionally changes an argument type in the test parse_pointers() from ptr<private> to ptr<function>. The former is invalid code which is indeed caught by the validator, but the test only asserts that parsing succeeds, not validation. With this patch, this error is now caught during parsing which caused the test to fail.

Testing
Test suite passes. Added new tests.

Checklist

• Run cargo fmt.
• Run taplo format.
• Run cargo clippy. If applicable, add:
  • --target wasm32-unknown-unknown
  • --target wasm32-unknown-emscripten
• Run cargo xtask test to run tests.
• Add change to CHANGELOG.md. See simple instructions inside file.

[jamienicol]

There may be a discussion to be had regarding how to handle conversion errors, which can either be addressed here or in a follow up. Note the change made to the parse_pointers() test. The old code was invalid WGSL which would be caught by the validator with this error message:

error: Function [1] 'bar' is invalid
  ┌─ in.wgsl:2:1
  │  
2 │ ╭ fn bar() {
3 │ │     var x: f32 = 1.0;
4 │ │     let px = &x;
5 │ │     let py = foo(px);
  │ │              ^^^^^^^ invalid function call
  │ ╰─────────────────────^ naga::Function [1]
  │  
  = Call to [0] is invalid
  = Argument 0 value [1] doesn't match the type [1]

However, the test only ensures that parsing succeeds, not validation. With this patch we now report an error during parsing (hence why we had to fix the shader code for the test to still pass):

Could not parse WGSL:
error: automatic conversions cannot convert `ptr<f32>` to `ptr<f32>`
  ┌─ in.wgsl:5:18
  │
5 │     let py = foo(px);
  │                  ^^ a value of type ptr<f32> is required here

Arguably the new error is more helpful as tells you the required type (though not printing the pointer's address space is unhelpful). Though of course the validator's error message could be improved. Additionally the "automatic conversions cannot convert..." line is a bit misleading, as we shouldn't really be attempting to automatically convert a ptr.

FWIW I did try making us not attempt automatic conversions for non-abstract types, but that caused this test assertion to fail. Again because it expects an error during parsing, but by not attempting to convert non-abstract types that shader parses successfully (but still fails validation)

[jamienicol]

For comparison, tint's error messages seem a bit more intuitive.

With the pointer example from above:

fn foo(a: ptr<private, f32>) -> f32 { return *a; }
fn bar() {
    var x: f32 = 1.0;
    let px = &x;
    let py = foo(px);
}

Tint outputs:

error: type mismatch for argument 1 in call to foo, expected ptr<private, f32, read_write>, got ptr<function, f32, read_write>
    let py = foo(px);
                 ^^

But in a case where an abstract type cannot convert correctly:

fn foo(a: u32) {}
fn bar() {
    foo(0.0);
}

error: cannot convert value of type abstract-float to type u32
    foo(0.0);
        ^^^

Note the distinction between a completely mismatched type, and an abstract type that cannot be converted correctly

[ErichDonGubler]

This LGTM, but I'm rather inexperienced with this part of Naga. Will give somebody from @gfx-rs/naga a (timeboxed) chance to look this over; holler if you'd like that, please!

[ErichDonGubler]

todo: Ah, one thing: gotta CHANGELOG entry? 🙂 This sounds like a nice item to highlight there!

[jamienicol]

done 🙂

[ErichDonGubler]

question: Doesn't this apply automatic type conversion, not just type inference?

[jamienicol]

I might not understand the distinction in terminology correctly. It will convert from AbstractInt or AbstractFloat to concrete types. It won’t convert from an already concrete type. I guess “conversion” is still a better word as “inference” relates to variable declaration?

[ErichDonGubler]

I guess the term of art of "feasible automatic conversion", according to the WGSL spec.: https://www.w3.org/TR/WGSL/#feasible-automatic-conversion.

So, I'd say "type conversion" here would be uncontroversial, while "inference" probably refers to the wrong thing (i.e., figuring out what the type of a binding is when it's not specified given a RHS on assignment).

[teoxoy]

This looks ok to me but @jimblandy might want to take a look.