[webkit-reviews] review granted: [Bug 177303] WSL should have some post-instantiation type checking : [Attachment 321450] the patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 21 14:28:55 PDT 2017


Keith Miller <keith_miller at apple.com> has granted Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 177303: WSL should have some post-instantiation type checking
https://bugs.webkit.org/show_bug.cgi?id=177303

Attachment 321450: the patch

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




--- Comment #5 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 321450
  --> https://bugs.webkit.org/attachment.cgi?id=321450
the patch

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

r=me with some nits.

> Tools/ChangeLog:26
> +	   Finally, this removes the shader type checker's incomplete
reimplementation of isPrimitive. That calls
> +	   isPrimitive now.

I don't understand what this is saying. What is "That"? isPrimitive was/is a
property. AFAICT, there is no isPrimitive function?

> Tools/WebGPUShadingLanguageRI/LateChecker.js:31
> +    constructor()
> +    {
> +	   super();
> +    }

Nit: you can remove this. Unless you want to be really sure that no arguments
are passed to Visitor.

> Tools/WebGPUShadingLanguageRI/LateChecker.js:52
> +	       if (!(node.returnType.type instanceof StructType))
> +		   throw new WTypeError(node.origin.originString, "Vertex
shader " + node.name + " must return a struct.");
> +	       assertPrimitive(node.returnType);

Nit: you could probably move this and the one below out of the switch.


More information about the webkit-reviews mailing list