-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Add friendly FES error for dimension mismatch on shared variable assi… #8823
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: dev-2.0
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { swizzleTrap, primitiveConstructorNode, variableNode, arrayAccessNode, a | |
| import { BaseType, NodeType, OpCode } from './ir_types'; | ||
| import { getNodeDataFromID, createNodeData, getOrCreateNode } from './ir_dag'; | ||
| import { recordInBasicBlock } from './ir_cfg'; | ||
| import { dimensionMismatchError } from './strands_FES'; | ||
| export class StrandsNode { | ||
| constructor(id, dimension, strandsContext) { | ||
| this.id = id; | ||
|
|
@@ -56,6 +57,13 @@ export class StrandsNode { | |
|
|
||
| // For varying variables, we need both assignment generation AND a way to reference by identifier | ||
| if (this._originalIdentifier) { | ||
| if(value?.isStrandsNode && value.dimension!==this._originalDimension){ | ||
|
Contributor
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. Similar to before, in an if statement above here we create |
||
| dimensionMismatchError( | ||
| this._originalDimension, | ||
| value.dimension, | ||
| this._originalIdentifier | ||
| ); | ||
| } | ||
| // Create a variable node for the target (the varying variable) | ||
| const { id: targetVarID } = variableNode( | ||
| this.strandsContext, | ||
|
|
@@ -108,6 +116,13 @@ export class StrandsNode { | |
|
|
||
| // For varying variables, create swizzle assignment | ||
| if (this._originalIdentifier) { | ||
| if(value?.isStrandsNode && value.dimension!==swizzlePattern.length){ | ||
|
Contributor
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. Looks like the same pattern applies here too. |
||
| dimensionMismatchError( | ||
| swizzlePattern.length, | ||
| value.dimension, | ||
| `${this._originalIdentifier}.${swizzlePattern}` | ||
| ); | ||
| } | ||
| // Create a variable node for the target with swizzle | ||
| const { id: targetVarID } = variableNode( | ||
| this.strandsContext, | ||
|
|
||
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.
There's a chance that the value is not yet a strands node, but in the if statement below on line 689 we create a strands node if it isn't one already. We could possibly move this check to happen after that, and then check for dimensions, to make sure we don't accidentally miss a scenario that should trigger a warning if e.g. the user assigned a plain number instead of a strands node?