| Summary: | Yarr JIT should include annotations with dumpDisassembly=true | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||
| Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, saam, webkit-bug-importer, ysuzuki | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Michael Saboff
2018-08-08 13:46:03 PDT
Created attachment 346794 [details]
Patch
Attachment 346794 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/yarr/YarrDisassembler.h:96: The parameter name "vectorOrder" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/yarr/YarrDisassembler.h:98: The parameter name "vectorOrder" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 2 in 10 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 346794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346794&action=review r=me > Source/JavaScriptCore/yarr/YarrDisassembler.h:77 > + enum VectorOrder { enum class? > Source/JavaScriptCore/yarr/YarrDisassembler.h:109 > + unsigned m_indentLevel; Making `m_identLevel { 0 };` looks nice to me instead of initializing it in a constructor. > Source/JavaScriptCore/yarr/YarrJIT.cpp:45 > +// friend void jitCompile(VM*, YarrCodeBlock&, const String& pattern, unsigned& numSubpatterns, const char*& error, bool ignoreCase, bool multiline); Let's remove this. (In reply to Yusuke Suzuki from comment #4) > Comment on attachment 346794 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=346794&action=review > > r=me > > > Source/JavaScriptCore/yarr/YarrDisassembler.h:77 > > + enum VectorOrder { > > enum class? Done. > > Source/JavaScriptCore/yarr/YarrDisassembler.h:109 > > + unsigned m_indentLevel; > > Making `m_identLevel { 0 };` looks nice to me instead of initializing it in > a constructor. Done. > > Source/JavaScriptCore/yarr/YarrJIT.cpp:45 > > +// friend void jitCompile(VM*, YarrCodeBlock&, const String& pattern, unsigned& numSubpatterns, const char*& error, bool ignoreCase, bool multiline); > > Let's remove this. Removed. This was a holdover when I was checking if this was dead. Committed r234713: <https://trac.webkit.org/changeset/234713> |