[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