-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[glsl-in] fix missing side effects from sequence expressions #8787
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
f6eacdf
984dc4f
72598bf
07f2b31
c15c7e3
6c23acc
3976fef
edcd4e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -511,20 +511,43 @@ impl ParsingContext<'_> { | |
| }) | ||
| } | ||
|
|
||
| // shader grammar :- | ||
| // expression : | ||
| // assignment_expression | ||
| // expression COMMA assignment_expression | ||
| pub fn parse_expression( | ||
| &mut self, | ||
| frontend: &mut Frontend, | ||
| ctx: &mut Context, | ||
| stmt: &mut StmtContext, | ||
| ) -> Result<Handle<HirExpr>> { | ||
| let mut exprs = Vec::new(); | ||
| let mut expr = self.parse_assignment(frontend, ctx, stmt)?; | ||
| exprs.push(expr); | ||
|
|
||
| while let TokenValue::Comma = self.expect_peek(frontend)?.value { | ||
| self.bump(frontend)?; | ||
| expr = self.parse_assignment(frontend, ctx, stmt)?; | ||
| exprs.push(expr); | ||
| } | ||
|
|
||
| Ok(expr) | ||
| if exprs.len() == 1 { | ||
| Ok(expr) | ||
| } else { | ||
| let mut meta = stmt.hir_exprs[exprs[0]].meta; | ||
| for &e in &exprs[1..] { | ||
| meta.subsume(stmt.hir_exprs[e].meta); | ||
| } | ||
|
Comment on lines
+538
to
+540
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for handling the spans correctly |
||
| expr = stmt.hir_exprs.append( | ||
| HirExpr { | ||
| kind: HirExprKind::Sequence { exprs }, | ||
| meta, | ||
| }, | ||
| Default::default(), | ||
| ); | ||
|
|
||
| Ok(expr) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lots of non-functional changes here, see overall review comment |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // issue #6208 https://github.com/gfx-rs/wgpu/issues/6208 | ||
| # version 460 | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would it be preferred to adopt webgl cts tests, for example https://registry.khronos.org/webgl/sdk/tests/conformance/glsl/bugs/sequence-operator-evaluation-order.html? |
||
|
|
||
| void main() { | ||
| float a = 1.0; | ||
| float b = 0.25; | ||
| float c = 1.5; | ||
| int i = 20; | ||
|
|
||
| // tests for multiple expressions in first part (if it's a expression, not declaration)! | ||
| // also the third part! | ||
| for (i = 0, c-=1.0; i < 25; i++, b+=0.01) { | ||
| a -= 0.02; | ||
| } | ||
|
|
||
| // a, b and c should be all ~0.5! | ||
| // right now it only ever takes the last expression in the block... | ||
| // leading to infinite loops, lost devices and incorrect results. | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| fn main_1() { | ||
| var a: f32 = 1f; | ||
| var b: f32 = 0.25f; | ||
| var c: f32 = 1.5f; | ||
| var i: i32 = 20i; | ||
|
|
||
| i = 0i; | ||
| let _e9 = c; | ||
| c = (_e9 - 1f); | ||
| loop { | ||
| let _e12 = i; | ||
| if !((_e12 < 25i)) { | ||
| break; | ||
| } | ||
| { | ||
| let _e22 = a; | ||
| a = (_e22 - 0.02f); | ||
| } | ||
| continuing { | ||
| let _e16 = i; | ||
| i = (_e16 + 1i); | ||
| let _e19 = b; | ||
| b = (_e19 + 0.01f); | ||
| } | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| @fragment | ||
| fn main() { | ||
| main_1(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this can't be produced by the parser its more clear to say
unreachable!()