On Dec 21 2008, 6:02 am, Cameron McCormack <[email protected]> wrote: > Hi again. > > I'm trying to solve bug 434031, where in optimised compilation mode > certain assignments to object properties result in a property named > "NaN" being created. This seems to occur when the assignment is done in > a direct call function, where the object index is one of the function's > parameters. > > Here's the test case from the bug, whittled down a little further: > > function add(_object, _key, _value) { > _object[_key] = _value; > } > > function f() { > add({}, 'c', 'd'); > } > > var o = { }; > add(o, 'a', 'b'); > java.lang.System.out.println(o.toSource()) > > This prints '({NaN:"b"})', but should print '{a:"b"}'. If we replace > add() with: > > function add(_object, _key, _value) { > var key = _key; > _object[key] = _value; > } > > then we get the desired output. > > When the object indexing is done via the local variable, this exercises > the second if-statement clause of the Token.GETVAR case in > Optimizer.rewriteForNumberVariables(Node,int): > > http://mxr.mozilla.org/js/source/js/rhino/src/org/mozilla/javascript/... > > That works, since isNumberVar() returns whether the flow analysis > determined that the variable would always be a number. In the first > clause of the if-statement, though, the one for when a function > parameter is used as the object index, there isn't any similar check. > > So, a few questions: > > * What are the exact conditions under which a function will be > determined to be a direct call function, and what assumptions > are then made? > > * Why are isNumberVar() and setIsNumberVar() on OptFunctionNode > restricted for use with non-parameter locals? > > * Why aren't the values in the varTypes array in > Block.runFlowAnalyzes() that correspond to parameters used? > > * Does the flow analyser in Block actually assign any value other than > AnyType to function parameters? > > Thanks, > > Cameron > > -- > Cameron McCormack ≝http://mcc.id.au/
I believe what's going on is that the direct call optimization has runtime conversions for the parameter to fit either number or object, and so code in rewriteForNumberVariables optimistically tries to use a number context. This works in most cases handled by rewriteForNumberVariables because it's dealing with cases where the value is needed as a number. But the use as an index to an object may have both numbers and objects, and so the code shouldn't consider that to be a number context. I think the real bug here is a missing call to convertParameter() for the index expression in the Token.SETELEM case. This call will convert the parameter to an object type. I've checked in a fix and updated the bug. This fix is suboptimal in that if a number is passed in as a parameter, then the number will be converted to an object rather than using that number value in a call to the special OptRuntime.setObjectIndex. But for the non-parameter case OptRuntime.setObjectIndex should still be used if the index is a number. A historical note: the type and data flow analysis and the direct call optimizations were written by different people at different times, so they are not as integrated as they could be. Thanks for your work diagnosing the problem! --Norris _______________________________________________ dev-tech-js-engine-rhino mailing list [email protected] https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
