[glsl-new] Handle local vars scopes #126
Conversation
| @@ -37,6 +38,9 @@ impl fmt::Display for ErrorKind { | |||
| ErrorKind::UnknownVariable(meta, val) => { | |||
| write!(f, "Unknown variable {} at {:?}", val, meta) | |||
| } | |||
| ErrorKind::VariableAlreadyDeclared(val) => { | |||
There was a problem hiding this comment.
who forbids this? I think, if you encounter a variable with the same name, it can just replace the current one in the scope map
There was a problem hiding this comment.
At least glslang barfs on this: http://shader-playground.timjones.io/53f99a8f0acb42b00c1b20a6888878d2
There was a problem hiding this comment.
Will try checking if the spec has anything to say on this ...
There was a problem hiding this comment.
Naga IR doesn't really care about names, other than for entry points (since they connect to the API). So IR has no limitations on what the names are, might as well treat them as debug labels.
The limitation you are talking about comes from GLSL, and the front-end should assume it has valid GLSL on the input, you don't need to go into the forest of validating incoming GLSL sources :) So you don't need to worry about overlaps.
There was a problem hiding this comment.
Where would the GLSL source be validated? Is naga always supposed to be used with a validator in front of it?
There was a problem hiding this comment.
Naga has one and only place in its pipeline where the data is guaranteed to be valid: that's the IR:
- front-end inputs are assumed to be valid. We don't care if they actually are, as long as the front-end doesn't crash or hangs (which is a universal requirement to the code base).
- front-end output IR has to be valid, and we are testing this, and any clients will be testing this, using the validator in of Validation function for the IR #59 . Any breaking of this contract is a bug in the front-end.
- similarly, backends can assume the IR is valid
- similarly, backend's produced low-level shaders have to be valid, and we are either testing, or will be testing this
So Naga doesn't care if your GLSL is valid or not. Just like a Vulkan driver doesn't care if the use of the API is valid or not. Unlike the Vulkan driver, Naga can't afford to behave strangely though.
- Check loacl vars before globals - Rename add_scope > push_scope - Remove unnecessary ref in lookup functions
I think this was the biggest remaining issue I needed to figure out - the rest should more or less be just work 😊
Probably also a good time to start figure out some proper tests.