Opened 10 years ago

Closed 8 years ago

#9828 closed defect (patchwelcome)

Variable named 'yield' causes problems in shrinksafe

Reported by: Bryan Forbes Owned by: Richard Backhouse
Priority: low Milestone: future
Component: ShrinkSafe Version: 1.3.2
Keywords: needsreview Cc: Bryan Forbes, Adam Peller
Blocked By: Blocking:

Description

Running this code through shrinksafe:

{
	compute: function(yield, other){
		return yield + other;
	}
}

changes it to this:

compute:
function(_1,_2){
return _2+_2;
};

Changing "yield" to something like "yield2" or removing the second argument fixes the problem.

Change History (14)

comment:1 Changed 10 years ago by dante

I made a quick test to see myself:

var result = 0;
(function(){

	var fn = function(yield, foo){
		return yield + foo;
	}
	
	result = fn(1, 2);

})();

outputs

var result=0;
(function(){
var fn=function(fn,_1){
return _1+_1;
};
result=fn(1,2);
})();

notice the 'fn' being "yielded" to the previous var? weeeird.

should I check in a failing unit test? (or just return; from the test, but have it in place?)

comment:2 Changed 10 years ago by dante

Summary: Variable named "yield" causes problems in shrinksafeVariable named 'yield' causes problems in shrinksafe

comment:3 Changed 10 years ago by Adam Peller

Milestone: 1.3.3tbd

Yield is a language feature of JavaScript? 1.7, which Rhino implements

https://developer.mozilla.org/en/New_in_JavaScript_1.7

but is not a reserved word, even in ECMAScript 5

http://bugs.ecmascript.org/ticket/413

It sounds like Rhino ought to behave if the version is set properly to something less than 1.7, but I'd suggest we avoid using terms like 'let' or 'yield' in our code.

I'm not certain this is worth addressing in 1.3.3.

comment:4 Changed 10 years ago by dante

@peller - suggestions here? Could be invalid/cantfix, and document the shortcoming in Shrinksafe ... Is there a way to force rhino to use a lower version of JS at runtime/calltime? The behavior now currently is not 'safe', just need to decide if there is something we can do about it or if people should be pointed to the updated documentation. This would be a "fun" bug to track down if you were using the variable 'yield' in your code anywhere.

comment:5 Changed 10 years ago by dante

Cc: Adam Peller added
Milestone: tbdfuture

comment:6 Changed 10 years ago by Adam Peller

Owner: changed from dante to Richard Backhouse

comment:7 Changed 9 years ago by Adam Peller

Milestone: future1.5

comment:8 Changed 9 years ago by Richard Backhouse

This appears to be a bug in Rhino. When I run the test case using Rhino 1.7R2 the result is

compute: function(_1,_2){ return _1+_2; };

So it would seem that it war a parser bug that has been fixed between R1 and R2

comment:9 Changed 9 years ago by Adam Peller

We never upgraded Rhino in util to 1.7R2. Should we do so and close this bug? We should add a test case while we're at it.

comment:10 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:11 Changed 8 years ago by bill

Milestone: 1.6future

(sadly) punting seemingly abandoned ticket and meta tickets to future

comment:12 Changed 8 years ago by Colin Snover

Priority: highblocker

Bulk update of open ticket priorities.

comment:13 Changed 8 years ago by ben hockey

Keywords: needsreview added
Priority: blockerlow

comment:14 Changed 8 years ago by bill

Resolution: patchwelcome
Status: newclosed

I think closure will be the compressor of choice going forwards, and we don't have anyone working on shrinksafe, so closing this ticket.

Note: See TracTickets for help on using tickets.