Bug 142388 – Add support for default constructor

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Miranda-fy constructor.

[Ryosuke Niwa]

Created attachment 248052 [details]
WIP

[Ryosuke Niwa]

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.

[Saam Barati]

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.

[Filip Pizlo]

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

[Ryosuke Niwa]

Created attachment 248362 [details]
WIP2

[Ryosuke Niwa]

Created attachment 248501 [details]
WIP3

[Ryosuke Niwa]

Created attachment 248626 [details]
Adds the feature

[WebKit Commit Bot]

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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Ryosuke Niwa]

Created attachment 248633 [details]
Skipped the test

[Radar WebKit Bug Importer]

<rdar://problem/20165846>

[Filip Pizlo]

Comment on attachment 248633 [details]
Skipped the test

LGTM

[WebKit Commit Bot]

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

[Ryosuke Niwa]

Created attachment 248790 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 248790 [details]
Patch for landing

Clearing flags on attachment: 248790

Committed r181611: <http://trac.webkit.org/changeset/181611>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.