Keep p5.strands transpiler output valid for comma-joined expressions - #9193
Open
harshiltewari2004 wants to merge 1 commit into
Open
harshiltewari2004 wants to merge 1 commit into
harshiltewari2004 wants to merge 1 commit into
Conversation
This file contains hidden or 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
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.
Resolves #9178
Changes
The transpiler assumed each call or assignment is its own statement. Comma-joined
code (as minifiers produce) breaks that assumption in three places, all in
src/strands/strands_transpiler.js:1. Assignments were rewritten into statements.
AssignmentExpressionturnedthe node into an
ExpressionStatementfor thebridgeandbridgeSwizzlepaths, so escodegen emitted a
;mid-expression (;,) and the callback failedto build. They are now rewritten as call expressions in place, via a new
replaceWithMethodCall()helper that the existing computed-member branch alsouses. This covers ternaries, call arguments and
x++(which routes through thesame handler). In ordinary statement position it also removes a stray empty
statement that was previously emitted.
2.
.set()in control flow dropped sibling expressions. The rewrite replacedthe whole enclosing statement, so in
a(), hook.set(v), b()the calls toa()and
b()disappeared from the output with no error. Only the.set()call isreplaced now.
3.
.begin()/.end()were only found as standalone statements. Withhook.end(), b();the generatedhook.set(...)was inserted after.end(),which produces a hook function that never assigns its result — a GLSL compile
error (
'HOOK_getColor': Function does not return a value). A newstatementCallsHookMethod()helper, shared by both lookups, also looks insidecomma expressions.
Tests
Four tests in a new
comma operator (#9178)suite intest/unit/webgl/p5.Shader.js, one per failure mode. Verified by stashing thesource change: all four fail without it, each for its own reason, and pass with
it. The full WebGL and WebGPU shader suites pass.
PR Checklist
npm run lintpasses