glsl-in: Update initializer list type when parsing #2066
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jimblandy
approved these changes
Sep 25, 2022
Previously when parsing an initializer list, the type passed to `parse_initializer` was not updated to reflect the child type, this caused implicit conversions to not work and nested initializer lists to not be allowed.
JCapucho
force-pushed
the
glsl-nested-init-list
branch
from
e442553
to
6621a41
Compare
|
@jimblandy thanks for the suggestion, I've also took some time to improve the documentation, could you give it another look. |
jimblandy
approved these changes
Oct 5, 2022
Comment on lines
+27
to
+29
| /// Does not check if the index is valid and returns the same type | ||
| /// when indexing out-of-bounds a struct or indexing a non indexable | ||
| /// type. |
There was a problem hiding this comment.
You're counting on validation to complain if there are too many components - correct? (It does, in naga::valid::compose::validate_compose.)
This PR is still an improvement over the status quo, but tolerating bad indices here does mean that if someone supplies too many values in the initializer, you're going to be passing meaningless types to implicit_conversion, and the error messages will be pretty confusing. That is, you'll get weird GLSL front end errors, instead of the nice "too many components" error from the validator, because you'll never make it that far.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously when parsing an initializer list, the type passed to
parse_initializerwas not updated to reflect the child type, this caused implicit conversions to not work and nested initializer lists to not be allowed.