These objects were also being referenced from the CodeBlock. By changing this to just retain pointers to FunctionBodyNodes these classes can be restricted to use during parsing.
Created attachment 34633 [details] The Patch.
Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/JavaScriptCore.exp Sending JavaScriptCore/bytecode/CodeBlock.cpp Sending JavaScriptCore/bytecode/CodeBlock.h Sending JavaScriptCore/bytecompiler/BytecodeGenerator.cpp Sending JavaScriptCore/bytecompiler/BytecodeGenerator.h Sending JavaScriptCore/interpreter/Interpreter.cpp Sending JavaScriptCore/jit/JITOpcodes.cpp Sending JavaScriptCore/jit/JITStubs.cpp Sending JavaScriptCore/jit/JITStubs.h Sending JavaScriptCore/parser/Grammar.y Sending JavaScriptCore/parser/NodeConstructors.h Sending JavaScriptCore/parser/Nodes.cpp Sending JavaScriptCore/parser/Nodes.h Sending JavaScriptCore/runtime/JSFunction.h Sending WebCore/ChangeLog Sending WebCore/inspector/JavaScriptDebugServer.cpp Sending WebKitTools/Scripts/run-javascriptcore-tests Transmitting file data .................. Committed revision 47089. Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.def Sending JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore_debug.def Transmitting file data ... Committed revision 47090.
Hi there, You commit https://trac.webkit.org/changeset/47089 JavaScriptCore: Restrict use of FuncDeclNode & FuncExprNode to the parser. https://bugs.webkit.org/show_bug.cgi?id=28209 seems to break the fast/js tests for Qt 64bit linux. The tests breaking are: fast/js/const.html expected fast/js/exception-thrown-from-function-with-lazy-activation.html fast/js/function-call-register-allocation.html fast/js/function-dot-arguments-and-caller.html fast/js/named-function-expression.html fast/js/static-scope-object.html plugins/netscape-invoke-default.html Cheers, Kenneth
Here you have the diffs of the failing tests fast/js/const.html -PASS f() is f +FAIL f() should be function g() { g="FAIL"; return g; } (of type function). Was FAIL (of type string). fast/js/exception-thrown-from-function-with-lazy-activation.html -PASS: args[0] should be 1 and is. +FAIL: args[0] should be 1 but instead is undefined. fast/js/function-call-register-allocation.html -PASS: Recursion did not run out of stack space. +FAIL: Recursion threw an exception: ReferenceError: Can't find variable: f fast/js/function-dot-arguments-and-caller.html -PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS,PASS fast/js/named-function-expression.html -PASS var z = 10; var x = eval('(function Named(a,b){ return (!!Named) ? (a + b + z) : -999; })'); x(4,5) is 19 +FAIL var z = 10; var x = eval('(function Named(a,b){ return (!!Named) ? (a + b + z) : -999; })'); x(4,5) should -PASS var ctr = 3; var x = (function Named(a,b){ if(--ctr) return 2 * Named(a,b); else return a + b; }); x(5,6) is 44 +FAIL var ctr = 3; var x = (function Named(a,b){ if(--ctr) return 2 * Named(a,b); else return a + b; }); x(5,6) should be 44. Threw exception ReferenceError: Can't find variable: Named fast/js/static-scope-object.html -PASS namedFunctionExpression() is globalObject +FAIL namedFunctionExpression() should be the global object. Threw exception ReferenceError: Can't find variable: f plugins/netscape-invoke-default.html This tests that it's possible to call NPN_InvokeDefault on a JavaScript object. If this test succeeded, the text "SUCCESS" should be shown below. -SUCCESS +FAILURE
Gah, sorry, it looks like I've broken the interpreter, can repro on Mac with the JIT disabled. Will investigate, & will roll the change out if I can't fix this quickly.
I can't tell if this is still pending commit. But I think it's not supposed to be, or?
I'm not sure I understand your question. :-) This patch is committed, currently pending decommit.
Comment on attachment 34633 [details] The Patch. Ok. Removing the r+ to remove it from the to-be-committed list.
Hmmm, is removing the r+ flag a good idea? :-/ Are we not losing the information from bugzilla as to who reviewed the patch, and which patches have been approved? What problem is this solving, does this related to the commit queue? If so, it seems the commit queue shouldn't care about this bug, since it was not added to the queue. If there is a problem, are we solving it in the right way here? - does the review queue mechanism need to be smarter? (Apologies if I'm just misunderstanding something here). cheers, G.
The query that Maciej sent to the list months ago for listing which bugs need commit: https://bugs.webkit.org/buglist.cgi?query_format=advanced&short_desc_type=notregexp&short_desc=%5C%5BS60%5C%5D&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%2B&field0-1-0=noop&type0-1-0=equals&value0-1-0= Show's bugs if they have r+. Nah, the commit-queue is smart enough to only look for bugs which actually have a commit-queue+. It's just we humans who are confused by the r+ on an open bug, and wonder if said open bug should be considered for landing.
Ah, I see. Fair enough. :-)
Sorry about that, should be fixed now in r47183.