Bug 206157 – [JSC] Consistently use "var" in builtin JS

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Consistently use "var" in builtin JS

[Yusuke Suzuki]

Created attachment 387493 [details]
Patch

[Yusuke Suzuki]

Created attachment 387494 [details]
Patch

[Mark Lam]

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.

[Mark Lam]

(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.

[Mark Lam]

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.

[Yusuke Suzuki]

Comment on attachment 387494 [details]
Patch

Thanks, fixed "let" => "var" changes in comments.

[Yusuke Suzuki]

Committed r254418: <https://trac.webkit.org/changeset/254418>

[Radar WebKit Bug Importer]

<rdar://problem/58516383>

[Joseph Pecoraro]

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