WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
121930
Implement prefixed-destructuring assignment
https://bugs.webkit.org/show_bug.cgi?id=121930
Summary
Implement prefixed-destructuring assignment
Oliver Hunt
Reported
2013-09-25 16:19:11 PDT
Implement prefixed-destructuring assignment
Attachments
Patch
(88.41 KB, patch)
2013-09-25 16:24 PDT
,
Oliver Hunt
no flags
Details
Formatted Diff
Diff
Patch
(88.00 KB, patch)
2013-09-25 16:50 PDT
,
Oliver Hunt
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
(534.15 KB, application/zip)
2013-09-25 17:40 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
(662.11 KB, application/zip)
2013-09-25 18:11 PDT
,
Build Bot
no flags
Details
Patch
(89.84 KB, patch)
2013-09-25 19:00 PDT
,
Oliver Hunt
mhahnenberg
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Oliver Hunt
Comment 1
2013-09-25 16:24:37 PDT
Created
attachment 212636
[details]
Patch
Early Warning System Bot
Comment 2
2013-09-25 16:34:22 PDT
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 3
2013-09-25 16:39:56 PDT
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
Comment 4
2013-09-25 16:50:57 PDT
Created
attachment 212641
[details]
Patch
Early Warning System Bot
Comment 5
2013-09-25 17:06:29 PDT
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 6
2013-09-25 17:12:54 PDT
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 7
2013-09-25 17:40:37 PDT
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
Comment 8
2013-09-25 17:40:39 PDT
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 9
2013-09-25 18:11:19 PDT
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
Comment 10
2013-09-25 18:11:21 PDT
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
Comment 11
2013-09-25 19:00:23 PDT
Created
attachment 212652
[details]
Patch
Mark Hahnenberg
Comment 12
2013-09-26 09:30:14 PDT
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
Comment 13
2013-09-26 09:51:23 PDT
Committed
r156464
: <
http://trac.webkit.org/changeset/156464
>
Ralph T
Comment 14
2013-09-26 12:16:49 PDT
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
Comment 15
2013-09-26 12:46:15 PDT
(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
Comment 16
2013-09-26 13:48:12 PDT
(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
Comment 17
2013-09-26 13:57:20 PDT
(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
Comment 18
2013-09-26 14:06:44 PDT
Whoops, I misspoke -- that GCC actually generates some error messages, the two versions I tried just spin.
WebKit Commit Bot
Comment 19
2013-09-26 14:22:25 PDT
Re-opened since this is blocked by
bug 121981
Timothy Hatcher
Comment 20
2013-09-26 14:25:03 PDT
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
Comment 21
2013-09-27 01:50:42 PDT
(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
Comment 22
2013-09-27 10:46:11 PDT
> 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
Comment 23
2013-10-01 18:59:04 PDT
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
Comment 24
2013-10-01 19:18:46 PDT
Rolled out in
http://trac.webkit.org/changeset/156757
Oliver Hunt
Comment 25
2013-10-02 12:09:44 PDT
Committed
r156785
: <
http://trac.webkit.org/changeset/156785
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug