...
Created attachment 454175 [details] Patch
Comment on attachment 454175 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454175&action=review > Source/WebGPU/WGSL/CompilationMessage.h:57 > +using Warning = CompilationMessage; > +using Error = CompilationMessage; Cute. > Source/WebGPU/WGSL/Parser.cpp:45 > +#define START_PARSE() \ Is this style of using #defines copied from anywhere? > Source/WebGPU/WGSL/Parser.cpp:168 > +Expected<AST::ShaderModule, Error> parse(const String& wgsl) > +{ > + Lexer lexer(wgsl); > + Parser parser(lexer); > + > + return parser.parseShader(); > +} Did you consider generating this code from a grammar file? > Source/WebGPU/WGSL/Parser.cpp:511 > + START_PARSE(); Is there a way to guarantee that parsing functions always start with a START_PARSE() macro?
Thanks for the review. (In reply to Myles C. Maxfield from comment #2) > Comment on attachment 454175 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=454175&action=review > > > Source/WebGPU/WGSL/CompilationMessage.h:57 > > +using Warning = CompilationMessage; > > +using Error = CompilationMessage; > > Cute. Thanks, I found it makes function signatures more readable. > > Source/WebGPU/WGSL/Parser.cpp:45 > > +#define START_PARSE() \ > > Is this style of using #defines copied from anywhere? The exact macros are different, but both JavaScriptCore/parser/Parser.cpp and the old WHLSL parser (which I used as inspiration) use a ton of macros as well. Regular C++ is just not the most pleasant language for writing this kind of code. > > Source/WebGPU/WGSL/Parser.cpp:168 > > +Expected<AST::ShaderModule, Error> parse(const String& wgsl) > > +{ > > + Lexer lexer(wgsl); > > + Parser parser(lexer); > > + > > + return parser.parseShader(); > > +} > > Did you consider generating this code from a grammar file? Yes, it would have been easier, but could have been an issue if the language later becomes more complicated. It would also have put a pretty low cap on the quality of the error messages that is achievable. As a result parser generators are rarely used for the major compilers for real-world languages. > > > Source/WebGPU/WGSL/Parser.cpp:511 > > + START_PARSE(); > > Is there a way to guarantee that parsing functions always start with a > START_PARSE() macro? Unfortunately I don't know of such a way. But forgetting the macro just gives you an error message as `_startOfElementPosition` is not defined.
Created attachment 454275 [details] Patch for landing Just fixed style nits.
Committed r291075 (248241@main): <https://commits.webkit.org/248241@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454275 [details].
<rdar://problem/90057433>