Bug 121930 – Implement prefixed-destructuring assignment

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

[Oliver Hunt]

Implement prefixed-destructuring assignment

[Oliver Hunt]

Created attachment 212636 [details]
Patch

[Early Warning System Bot]

Comment on attachment 212636 [details]
Patch

Attachment 212636 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/2187067

[Early Warning System Bot]

Comment on attachment 212636 [details]
Patch

Attachment 212636 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/2187069

[Oliver Hunt]

Created attachment 212641 [details]
Patch

[Early Warning System Bot]

Comment on attachment 212641 [details]
Patch

Attachment 212641 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/2186072

[Early Warning System Bot]

Comment on attachment 212641 [details]
Patch

Attachment 212641 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/2165087

[Build Bot]

Comment on attachment 212641 [details]
Patch

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

New failing tests:
sputnik/Conformance/13_Function_Definition/S13_A5.html

[Build Bot]

Created attachment 212643 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 212641 [details]
Patch

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

New failing tests:
sputnik/Conformance/13_Function_Definition/S13_A5.html

[Build Bot]

Created attachment 212647 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Oliver Hunt]

Created attachment 212652 [details]
Patch

[Mark Hahnenberg]

Comment on attachment 212652 [details]
Patch

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

r=me

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:398
>  }

Side note: the BytecodeGenerator constructor seems like it does waaay too much stuff. Maybe we should factor some of this functionality out into separate helper functions?

> Source/JavaScriptCore/parser/ASTBuilder.h:322
> +    ParameterNode* createFormalParameterList(DeconstructionPattern pattern) { return new (m_vm) ParameterNode(pattern); }
> +    ParameterNode* createFormalParameterList(ParameterNode* list, DeconstructionPattern pattern) { return new (m_vm) ParameterNode(list, pattern); }

It's sad that we can't do PassRefPtrs here, but hopefully move semantics will save us.

[Oliver Hunt]

Committed r156464: <http://trac.webkit.org/changeset/156464>

[Ralph T]

GCC 4.7.3 and 4.8.1 can no longer compile Parser.cpp on linux x86-64 with this change applied.

[Csaba Osztrogonác]

(In reply to comment #14)
> GCC 4.7.3 and 4.8.1 can no longer compile Parser.cpp on linux x86-64 with this change applied.
Who cares? 
Apple doesn't use GCC (and EWS) ...

[Oliver Hunt]

(In reply to comment #15)
> (In reply to comment #14)
> > GCC 4.7.3 and 4.8.1 can no longer compile Parser.cpp on linux x86-64 with this change applied.
> Who cares? 
> Apple doesn't use GCC (and EWS) ...

Wow, way to be helpful Ossy, that patch had been up for the EWS bots all night and they hadn't run it.  I thought i had fixed the problems that caused the earlier failures.  Although I don't understand why there are Qt bots still running (or why the Qt port is even still in tree) given the stated intention to no longer use webkit.

(In reply to comment #14)
> GCC 4.7.3 and 4.8.1 can no longer compile Parser.cpp on linux x86-64 with this change applied.

I fixed one issue that was causing MSVC to have problems compiling, what are the failures you're seeing in GCC?  There shouldn't be anything in there that isn't part of the C++11 subset that we now require as part of webkit but it's possible I used something unintentionally

[Ralph T]

(In reply to comment #16)
> (In reply to comment #14)
> > GCC 4.7.3 and 4.8.1 can no longer compile Parser.cpp on linux x86-64 with this change applied.
> 
> I fixed one issue that was causing MSVC to have problems compiling, what are the failures you're seeing in GCC?  There shouldn't be anything in there that isn't part of the C++11 subset that we now require as part of webkit but it's possible I used something unintentionally

GCC doesn't terminate and gradually consumes more and more memory. I don't know exactly what's going on inside. You can see the output on any of the GCC bots, for example: https://webkit-queues.appspot.com/results/2574002 (it terminated GCC on Parser.cpp).

The change is pretty big and I'm unfamiliar with this part of WebKit; let me know if there's anything I can help test.

[Ralph T]

Whoops, I misspoke -- that GCC actually generates some error messages, the two versions I tried just spin.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 121981

[Timothy Hatcher]

This caused a massive leak that was killing the buildbot.

All the leaks are BindingNodes.

http://build.webkit.org/builders/Apple%20MountainLion%20%28Leaks%29/builds/7094/steps/layout-test/logs/stdio

Leak: 0x7fb2d14d13f0  size=64  zone: DefaultMallocZone_0x10f090000   JSC::BindingNode  C++  JavaScriptCore
	0x0f9c43e0 0x00000001 0x00000001 0x60000000 	.C.............`
	0x0000000d 0x0000001e 0x00000001 0x0000000d 	................
	0x0000001b 0x00000001 0x0000000d 0x0000001e 	................
	0x00000001 0x00007fb2 0xd2023800 0x00007fb2 	.........8......
	Call stack: [thread 0x7fff77210180]: | 0x2 | start | main DumpRenderTree.mm:955 | dumpRenderTree(int, char const**) DumpRenderTree.mm:917 | runTestingServerLoop() DumpRenderTree.mm:862 | runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) DumpRenderTree.mm:1411 | CFRunLoopRunSpecific | __CFRunLoopRun | __CFRunLoopDoSources0 | __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ | MultiplexerSource::perform() | RunloopBlockContext::perform() | CFArrayApplyFunction | __block_global_1 | ___withDelegateAsync_block_invoke_0 | ___delegate_didFinishLoading_block_invoke_0 | -[NSURLConnectionInternal _withActiveConnectionAndDelegate:] | -[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:] | __65-[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:]_block_invoke_0 | -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] WebCoreResourceHandleAsDelegate.mm:234 | WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double) ResourceLoader.cpp:490 | WebCore::SubresourceLoader::didFinishLoading(double) SubresourceLoader.cpp:285 | WebCore::CachedRawResource::finishLoading(WebCore::ResourceBuffer*) CachedRawResource.cpp:95 | WebCore::CachedResource::finishLoading(WebCore::ResourceBuffer*) CachedResource.cpp:386 | WebCore::CachedResource::checkNotify() CachedResource.cpp:369 | WebCore::DocumentLoader::notifyFinished(WebCore::CachedResource*) DocumentLoader.cpp:346 | WebCore::DocumentLoader::finishedLoading(double) DocumentLoader.cpp:409 | WebCore::DocumentWriter::end() DocumentWriter.cpp:243 | WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter*) DecodedDataDocumentParser.cpp:60 | WebCore::HTMLDocumentParser::append(WTF::PassRefPtr<WTF::StringImpl>) HTMLDocumentParser.cpp:744 | WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode) HTMLDocumentParser.cpp:237 | WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) HTMLDocumentParser.cpp:536 | WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode, WebCore::PumpSession&) HTMLDocumentParser.cpp:292 | WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() HTMLDocumentParser.cpp:272 | WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element>, WTF::TextPosition const&) HTMLScriptRunner.cpp:183 | WebCore::HTMLScriptRunner::runScript(WebCore::Element*, WTF::TextPosition const&) HTMLScriptRunner.cpp:314 | WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) ScriptElement.cpp:246 | WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&) ScriptElement.cpp:315 | WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&) ScriptController.cpp:158 | WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld*) ScriptController.cpp:142 | WebCore::JSMainThreadExecState::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) JSMainThreadExecState.h:62 | JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) Completion.cpp:83 | JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) Interpreter.cpp:854 | JSC::ProgramExecutable::initializeGlobalProperties(JSC::VM&, JSC::ExecState*, JSC::JSScope*) Executable.cpp:425 | JSC::JSGlobalObject::createProgramCodeBlock(JSC::ExecState*, JSC::ProgramExecutable*, JSC::JSObject**) JSGlobalObject.cpp:723 | JSC::CodeCache::getProgramCodeBlock(JSC::VM&, JSC::ProgramExecutable*, JSC::SourceCode const&, JSC::JSParserStrictness, JSC::DebuggerMode, JSC::ProfilerMode, JSC::ParserError&) CodeCache.cpp:120 | JSC::UnlinkedProgramCodeBlock* JSC::CodeCache::getCodeBlock<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable>(JSC::VM&, JSC::ProgramExecutable*, JSC::SourceCode const&, JSC::JSParserStrictness, JSC::DebuggerMode, JSC::ProfilerMode, JSC::ParserError&) CodeCache.cpp:92 | WTF::PassRefPtr<JSC::ProgramNode> JSC::parse<JSC::ProgramNode>(JSC::VM*, JSC::SourceCode const&, JSC::FunctionParameters*, JSC::Identifier const&, JSC::JSParserStrictness, JSC::JSParserMode, JSC::ParserError&) Parser.h:1118 | WTF::PassRefPtr<JSC::ProgramNode> JSC::Parser<JSC::Lexer<unsigned char> >::parse<JSC::ProgramNode>(JSC::ParserError&) Parser.h:1052 | JSC::Parser<JSC::Lexer<unsigned char> >::parseInner() Parser.cpp:120 | JSC::ASTBuilder::SourceElements JSC::Parser<JSC::Lexer<unsigned char> >::parseSourceElements<(JSC::SourceElementsMode)0, JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:176 | JSC::ASTBuilder::Statement JSC::Parser<JSC::Lexer<unsigned char> >::parseStatement<JSC::ASTBuilder>(JSC::ASTBuilder&, JSC::Identifier const*&, unsigned int*) Parser.cpp:886 | JSC::ASTBuilder::Statement JSC::Parser<JSC::Lexer<unsigned char> >::parseExpressionOrLabelStatement<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1070 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1218 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseAssignmentExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1245 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseConditionalExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1304 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseBinaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1343 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseUnaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1827 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseMemberExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1765 | JSC::ASTBuilder::Arguments JSC::Parser<JSC::Lexer<unsigned char> >::parseArguments<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1694 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseAssignmentExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1245 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseConditionalExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1304 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseBinaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1343 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseUnaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1827 | JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseMemberExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:1732 | bool JSC::Parser<JSC::Lexer<unsigned char> >::parseFunctionInfo<(JSC::FunctionRequirements)0, false, JSC::ASTBuilder>(JSC::ASTBuilder&, JSC::Identifier const*&, JSC::ASTBuilder::FormalParameterList&, JSC::ASTBuilder::FunctionBody&, unsigned int&, unsigned int&, int&, unsigned int&) Parser.cpp:946 | JSC::ASTBuilder::FormalParameterList JSC::Parser<JSC::Lexer<unsigned char> >::parseFormalParameters<JSC::ASTBuilder>(JSC::ASTBuilder&) Parser.cpp:908 | JSC::ASTBuilder::DeconstructionPattern JSC::Parser<JSC::Lexer<unsigned char> >::parseDeconstructionPattern<(JSC::Parser<JSC::Lexer<unsigned char> >::DeconstructionKind)1, JSC::ASTBuilder>(JSC::ASTBuilder&, int) Parser.cpp:431 | JSC::ASTBuilder::DeconstructionPattern JSC::Parser<JSC::Lexer<unsigned char> >::createBindingPattern<(JSC::Parser<JSC::Lexer<unsigned char> >::DeconstructionKind)1, JSC::ASTBuilder>(JSC::ASTBuilder&, JSC::Identifier const&, int) Parser.cpp:360 | JSC::ASTBuilder::createBindingLocation(JSC::JSTokenLocation const&, JSC::Identifier const&, JSC::JSTextPosition const&, JSC::JSTextPosition const&, JSC::JSTextPosition const&) ASTBuilder.h:655 | JSC::BindingNode::create(JSC::VM*, JSC::Identifier const&, JSC::JSTextPosition const&, JSC::JSTextPosition const&, JSC::JSTextPosition const&) NodeConstructors.h:858 | JSC::DeconstructionPatternNode::operator new(unsigned long) Nodes.h:1539 | WTF::fastMalloc(unsigned long) FastMalloc.cpp:287 | malloc | malloc_zone_malloc

[Csaba Osztrogonác]

(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > GCC 4.7.3 and 4.8.1 can no longer compile Parser.cpp on linux x86-64 with this change applied.
> > Who cares? 
> > Apple doesn't use GCC (and EWS) ...
> 
> Wow, way to be helpful Ossy, that patch had been up for the EWS bots all night and they hadn't run it.  I thought i had fixed the problems that caused the earlier failures.

If you had clicked on the purple bubbles, you would have seen 
that EWS bots can't apply the patch, that's why they don't run.
( Otherwise watching the bots after landing isn't forbidden. :) )
And because of this breakage, one more breakage landed in the
tree, because EWS don't run .. - https://bugs.webkit.org/show_bug.cgi?id=121983

> Although I don't understand why there are Qt bots still running (or why the Qt port is even still in tree) given the stated intention to no longer use webkit.

How is it related to me or this bug? I don't work on Qt port, I am not the Qt
port and Qt port isn't me. If you would like to know more about the future of
the Qt port, I suggest you should follow this thread on webkit-dev mailing
list: Changes in QtWebKit development.

( Otherwise Qt buildbots and EWS bots are still the fastest bots
  and they notice very quickly if something goes wrong. )

Just out of curiosity if the contributing rules are burden for 
you, why don't you change "all platforms" to "Apple Mac" by fiat?

"Keeping the tree green
... 
Your change must at least compile on all platforms.
..."

/ http://www.webkit.org/coding/contributing.html /

[Oliver Hunt]

> How is it related to me or this bug? I don't work on Qt port, I am not the Qt
> port and Qt port isn't me. If you would like to know more about the future of
> the Qt port, I suggest you should follow this thread on webkit-dev mailing
> list: Changes in QtWebKit development.

Okay, in that case you don't actually do any work on webkit, you just make unhelpful comments in bugzilla.  People other than you actually worked to help me diagnose the problem with the gcc build.

By actually contributing other people helped webkit get better.

On the other hand, your comments did nothing to help anyone.

[Filip Pizlo]

This regressed string-fasta by 50%.  I'm going to roll it out.

Please run performance tests next time you make such large changes.  It's very painful to have to find the performance regressions so long after the fact.

[Filip Pizlo]

Rolled out in http://trac.webkit.org/changeset/156757

[Oliver Hunt]

Committed r156785: <http://trac.webkit.org/changeset/156785>