Bug 142388 - Add support for default constructor
Summary: Add support for default constructor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on: 142430 142436 142574 142626 142627
Blocks: 140491
  Show dependency treegraph
 
Reported: 2015-03-06 00:34 PST by Ryosuke Niwa
Modified: 2015-03-16 21:03 PDT (History)
10 users (show)

See Also:


Attachments
WIP (10.21 KB, patch)
2015-03-06 00:35 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
WIP2 (19.40 KB, patch)
2015-03-10 15:00 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
WIP3 (21.93 KB, patch)
2015-03-12 01:41 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Adds the feature (23.77 KB, patch)
2015-03-13 17:31 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (526.06 KB, application/zip)
2015-03-13 18:30 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (560.69 KB, application/zip)
2015-03-13 18:34 PDT, Build Bot
no flags Details
Skipped the test (24.60 KB, patch)
2015-03-13 19:07 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (24.44 KB, patch)
2015-03-16 20:09 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2015-03-06 00:34:54 PST
Miranda-fy constructor.
Comment 1 Ryosuke Niwa 2015-03-06 00:35:43 PST
Created attachment 248052 [details]
WIP
Comment 2 Ryosuke Niwa 2015-03-06 00:43:55 PST
Comment on attachment 248052 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=248052&action=review

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1772
> +            SourceCode source = makeSource(ASCIILiteral("(function () { return this; })"));
> +            executable = FunctionExecutable::fromGlobalCode(name, m_globalObject->globalExec(), m_globalObject->debugger(), source, &exception);

Parsing code here is kind of ugly. I wonder if I can easily add a subclass of FunctionExecutable or not.
Comment 3 Saam Barati 2015-03-10 12:53:26 PDT
Comment on attachment 248052 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=248052&action=review

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:90
> +    , m_isMiranda(false)

What about using a more intuitive/descriptive name here?
I kind of like: m_isBaseClassDefaultConstructor

I had to read parsing code to figure out what isMiranda represents.
Comment 4 Filip Pizlo 2015-03-10 13:12:02 PDT
(In reply to comment #3)
> Comment on attachment 248052 [details]
> WIP
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248052&action=review
> 
> > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:90
> > +    , m_isMiranda(false)
> 
> What about using a more intuitive/descriptive name here?
> I kind of like: m_isBaseClassDefaultConstructor
> 
> I had to read parsing code to figure out what isMiranda represents.

"Miranda" is sort of a term of art in Java parsing. I think it carries more specific meaning than isBasrClassDefaultConstructor and its shorter.
Comment 5 Ryosuke Niwa 2015-03-10 15:00:31 PDT
Created attachment 248362 [details]
WIP2
Comment 6 Ryosuke Niwa 2015-03-12 01:41:17 PDT
Created attachment 248501 [details]
WIP3
Comment 7 Ryosuke Niwa 2015-03-13 17:31:51 PDT
Created attachment 248626 [details]
Adds the feature
Comment 8 WebKit Commit Bot 2015-03-13 17:35:13 PDT
Attachment 248626 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:109:  Use 'WTF::move()' instead of 'std::move()'.  [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/builtins/BuiltinExecutables.cpp:104:  Use 'WTF::move()' instead of 'std::move()'.  [runtime/wtf_move] [4]
Total errors found: 2 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Build Bot 2015-03-13 18:30:48 PDT
Comment on attachment 248626 [details]
Adds the feature

Attachment 248626 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5439830750134272

New failing tests:
js/class-syntax-default-constructor.html
Comment 10 Build Bot 2015-03-13 18:30:52 PDT
Created attachment 248629 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 11 Build Bot 2015-03-13 18:34:06 PDT
Comment on attachment 248626 [details]
Adds the feature

Attachment 248626 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5259734181478400

New failing tests:
js/class-syntax-default-constructor.html
Comment 12 Build Bot 2015-03-13 18:34:10 PDT
Created attachment 248630 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 13 Ryosuke Niwa 2015-03-13 19:07:33 PDT
Created attachment 248633 [details]
Skipped the test
Comment 14 Radar WebKit Bug Importer 2015-03-14 21:56:20 PDT
<rdar://problem/20165846>
Comment 15 Filip Pizlo 2015-03-16 14:35:10 PDT
Comment on attachment 248633 [details]
Skipped the test

LGTM
Comment 16 WebKit Commit Bot 2015-03-16 19:56:15 PDT
Comment on attachment 248633 [details]
Skipped the test

Rejecting attachment 248633 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 248633, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
hangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/TestExpectations
Hunk #1 succeeded at 69 with fuzz 1.
patching file LayoutTests/js/class-syntax-default-constructor-expected.txt
patching file LayoutTests/js/class-syntax-default-constructor.html
patching file LayoutTests/js/script-tests/class-syntax-default-constructor.js

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Filip Pizlo']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/5673560722898944
Comment 17 Ryosuke Niwa 2015-03-16 20:09:17 PDT
Created attachment 248790 [details]
Patch for landing
Comment 18 WebKit Commit Bot 2015-03-16 21:03:27 PDT
Comment on attachment 248790 [details]
Patch for landing

Clearing flags on attachment: 248790

Committed r181611: <http://trac.webkit.org/changeset/181611>
Comment 19 WebKit Commit Bot 2015-03-16 21:03:31 PDT
All reviewed patches have been landed.  Closing bug.