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

[naga] Validation relies on ensure_blocks_return pass to catch non-void functions which don't return a value #7015

Open
jamienicol opened this issue Jan 28, 2025 · 1 comment
Labels
area: validation Issues related to validation, diagnostics, and error handling type: enhancement New feature or request

Comments

@jamienicol
Copy link
Contributor

jamienicol commented Jan 28, 2025

Description
Noticed this while working on #4395 / #7013. The problem was a non-void function which doesn't have a return statement. This typically gets caught by the validator here as a InvalidReturnType error. Note, however, that this just says "for each return statement, ensure that the type being returned matches the function's return type". It does not ensure that every control flow branch of a function eventually leads to a return statement.

This is fine as long as the front-end calls the ensure_block_returns() pass (modulo any bugs in that pass such as #4395), as we will then always have a return statement. But it feels very fragile for the validator to rely on a pass that a frontend may forget to call.

If we add a pass to the validator to check this properly, perhaps we can remove the ensure_block_returns pass altogether. This looks like it was added because SPIRV requires functions to end in a return statement (see gfx-rs/naga#301) but we handle that in the spv backend here. Though the other arguments given in that issue may still apply

@jamienicol jamienicol changed the title [naga] Validation relies on ensure_blocks_return pass to catch non-void functions which don't return [naga] Validation relies on ensure_blocks_return pass to catch non-void functions which don't return a value Jan 28, 2025
@teoxoy teoxoy added the area: validation Issues related to validation, diagnostics, and error handling label Jan 28, 2025
@jamienicol
Copy link
Contributor Author

jamienicol commented Jan 29, 2025

I initially wrote in the description that the HLSL frontend does not call ensure_block_returns, but there is no HLSL frontend 😆 . I've edited the description to keep it accurate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: validation Issues related to validation, diagnostics, and error handling type: enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

3 participants