[webkit-reviews] review granted: [Bug 198059] WHLSL: Add an AST dumper : [Attachment 370362] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue May 21 18:17:23 PDT 2019
Myles C. Maxfield <mmaxfield at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 198059: WHLSL: Add an AST dumper
https://bugs.webkit.org/show_bug.cgi?id=198059
Attachment 370362: patch
https://bugs.webkit.org/attachment.cgi?id=370362&action=review
--- Comment #7 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 370362
--> https://bugs.webkit.org/attachment.cgi?id=370362
patch
View in context: https://bugs.webkit.org/attachment.cgi?id=370362&action=review
> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:264
> + Base::visit(constantExpression);
cool
> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:577
> + m_out.print(" && ");
Aren't parentheses required to make this non ambiguous?
> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:589
> + visit(logicalNotExpression.operand());
ditto
> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:609
> + m_out.print("(");
makeString() will make these easier to read.
> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h:44
> + void visit(AST::UnnamedType&) override;
These can all be private because they'll all only be called from the base
class.
> Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h:139
> + dataLogLn(toString(program));
Shouldn't we make a for realsies new logging channel and use WTFLog()?
More information about the webkit-reviews
mailing list