Bug 135179

Summary: Array.concat() should work on runtime arrays too
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, fpizlo, ggaren, mhahnenberg, mmirman, msaboff, oliver, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch.
mark.lam: review-
patch minus tabs ggaren: review+

Description Mark Lam 2014-07-22 15:01:51 PDT 
Patch coming.
Comment 1 Mark Lam 2014-07-22 15:02:25 PDT 
<rdar://problem/17544620>
Comment 2 Mark Lam 2014-07-22 15:34:36 PDT 
Created attachment 235319 [details]
the patch.
Comment 3 WebKit Commit Bot 2014-07-22 15:36:19 PDT 
Attachment 235319 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jsc.cpp:288:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:289:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:290:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/jsc.cpp:301:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:361:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:361:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/JavaScriptCore/jsc.cpp:362:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:363:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:365:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:366:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:367:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:373:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:373:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jsc.cpp:374:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:374:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/JavaScriptCore/jsc.cpp:375:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:377:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:378:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:378:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/JavaScriptCore/jsc.cpp:379:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:380:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:381:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:382:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/jsc.cpp:383:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:2:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:3:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:4:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:6:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:7:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:9:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:10:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/regexp-matches-array.js:11:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:2:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:3:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:5:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:6:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:7:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:9:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/runtime-array.js:10:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 41 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Mark Lam 2014-07-22 15:37:20 PDT 
Comment on attachment 235319 [details]
the patch.

Curses.  Will remove tabs.
Comment 5 Mark Lam 2014-07-22 15:47:46 PDT 
Created attachment 235321 [details]
patch minus tabs
Comment 6 Geoffrey Garen 2014-07-22 21:02:41 PDT 
Comment on attachment 235321 [details]
patch minus tabs

r=me
Comment 7 Mark Lam 2014-07-22 21:20:38 PDT 
Thanks.  Landed in r171390: <http://trac.webkit.org/r171390>.
Comment 8 Mark Hahnenberg 2014-07-23 07:21:36 PDT 
(In reply to comment #7)
> Thanks.  Landed in r171390: <http://trac.webkit.org/r171390>.

Did you look at the RegExpArray stuff too?
Comment 9 Brent Fulgham 2014-07-23 08:54:36 PDT 
This introduced a build failure on Windows, since NO_RETURN_DUE_TO_CRASH is undefined.

1. I landed a build fix for this: <http://trac.webkit.org/changeset/171393>
2. I of course screwed up the change and had to land a follow-up: <http://trac.webkit.org/changeset/171395>
Comment 10 Mark Lam 2014-07-23 09:56:17 PDT 
(In reply to comment #8)
> Did you look at the RegExpArray stuff too?

I believe you mean RegExpMatchesArray.  Yes, we looked at it, and also added a test at Source/JavaScriptCore/tests/stress/regexp-matches-array.js for it.
Comment 11 Mark Hahnenberg 2014-07-23 10:25:37 PDT 
(In reply to comment #10)
> (In reply to comment #8)
> > Did you look at the RegExpArray stuff too?
> 
> I believe you mean RegExpMatchesArray.  Yes, we looked at it, and also added a test at Source/JavaScriptCore/tests/stress/regexp-matches-array.js for it.

Sweet! Just curious if there was also an issue there.