[webkit-reviews] review granted: [Bug 177044] WSL needs a way to verify that structs are not cyclic : [Attachment 321412] the patch

[bugzilla-daemon at webkit.org]

Myles C. Maxfield <mmaxfield at apple.com> has granted Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 177044: WSL needs a way to verify that structs are not cyclic
https://bugs.webkit.org/show_bug.cgi?id=177044

Attachment 321412: the patch

https://bugs.webkit.org/attachment.cgi?id=321412&action=review




--- Comment #2 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 321412
  --> https://bugs.webkit.org/attachment.cgi?id=321412
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321412&action=review

> Tools/WebGPUShadingLanguageRI/Checker.js:64
> +	   let shaderType = node;

This doesn’t seem right. Should this be shaderType = node.shaderType;?

> Tools/WebGPUShadingLanguageRI/Checker.js:85
>	   switch (node.shaderType) {

... and you can use it here?

> Tools/WebGPUShadingLanguageRI/RecursiveTypeChecker.js:44
> +	   this._visiting.doVisit(node, () => super.visitStructType(node));

So linked lists will work, but struct Foo { Foo x; int y; } won’t.

> Tools/WebGPUShadingLanguageRI/Test.js:4436
> +	   () => checkInt(program, callFunction(program, "foo", [], []), -511),

I thought callFunction() caught theTrapError?

> Tools/WebGPUShadingLanguageRI/Test.js:4463
> +}

Can we test linked lists?

> Tools/WebGPUShadingLanguageRI/Visitor.js:-98
> -	   Node.visit(node.type, this);

Why not? Because this type would be visited in some other part of the tree?