Summary: | [JSC] Consistently use "var" in builtin JS | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||
Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ews-watchlist, joepeck, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Yusuke Suzuki
2020-01-12 15:06:10 PST
Created attachment 387493 [details]
Patch
Created attachment 387494 [details]
Patch
Comment on attachment 387493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387493&action=review > Source/JavaScriptCore/builtins/RegExpPrototype.js:246 > + var n = chCode - 0x30; Is this problematic? It overwrites they outer chCode. (In reply to Mark Lam from comment #3) > Comment on attachment 387493 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=387493&action=review > > > Source/JavaScriptCore/builtins/RegExpPrototype.js:246 > > + var n = chCode - 0x30; > > Is this problematic? It overwrites they outer chCode. Nevermind. I misread that. It sets n, not chCode. Comment on attachment 387494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387494&action=review r=me if EWS bots are green. Please undo the changes in the comments. > Source/JavaScriptCore/builtins/RegExpPrototype.js:495 > + // 6. If flags contains "u", var unicodeMatching be true. > + // 7. Else, var unicodeMatching be false. I think you should undo these. The use of "let" here is in the English context, not the JS context. Also, this is a mirror of the text in the spec. > Source/JavaScriptCore/builtins/RegExpPrototype.js:498 > + // 8. If flags contains "y", var newFlags be flags. > + // 9. Else, var newFlags be the string that is the concatenation of flags and "y". Ditto. Please undo. > Source/JavaScriptCore/builtins/RegExpPrototype.js:513 > + // 13. If limit is undefined, var lim be 2^32-1; else var lim be ? ToUint32(limit). Ditto. Undo. > Source/JavaScriptCore/builtins/RegExpPrototype.js:547 > + // c. If z is null, var q be AdvanceStringIndex(S, q, unicodeMatching). Ditto. Undo. > Source/JavaScriptCore/builtins/RegExpPrototype.js:556 > + // iii. If e = p, var q be AdvanceStringIndex(S, q, unicodeMatching). Ditto. Undo. Comment on attachment 387494 [details]
Patch
Thanks, fixed "let" => "var" changes in comments.
Committed r254418: <https://trac.webkit.org/changeset/254418> You may want to add a style checker or something, or lexical variables will probably start creeping in again. Also, is there a bug for WebCore's builtins? A quick search found: • Source/WebCore/Modules/streams |