Jesse Ruderman Reporter
	Description • 14 years ago

(function(){ print(8>>1); })();

./js       4
./js -m    0

	Alan Pierce Assignee
	Comment 1 • 14 years ago

Easy one-character fix.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 2 • 14 years ago

Comment on attachment 463073 [details] [diff] [review]
Patch

r=me but we should really just be calling JSOpBinaryTryConstantFold() here, as per bug 578517 comment #13

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 3 • 14 years ago

And by "here" I mean at the top of the function.

	Chris Leary [:cdleary] (not checking bugmail)
	Comment 4 • 14 years ago

My mistake, thanks for picking up the patch, Alan. I'm learning that I need to write more tests than just a few sanity checks for my ops.

	Alan Pierce Assignee
	Comment 5 • 14 years ago

Added a few test cases and made it call JSOpBinaryTryConstantFold instead. We need to go through ValueToECMAInt32 for shifts, so I reworked JSOpBinaryTryConstantFold a bit.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 6 • 14 years ago

(In reply to comment #4)

No worries - can't catch everything, this is what the fuzzer is for ;)

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 7 • 14 years ago

Comment on attachment 463084 [details] [diff] [review]
Patch, v2

Sorry Alan, forgot that the folder needed changes. Thanks for taking.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 8 • 14 years ago

Comment on attachment 463084 [details] [diff] [review]
Patch, v2


>+    /* TODO: Check for conversion failure. */

Conversion failure isn't possible on a primitive, IIRC.

	Brendan Eich [:brendan]
	Comment 9 • 14 years ago

Indeed, primitives convert infallibly (may result in NaN -> 0, -0 -> 0, e.g.).

The L and R locals stand out much better, even though prevailing style avoids capitalizing local and member names. Maybe that would help in general, although it starts to get shout-y. "l" is obviously a bad local name because it looks like a 1 in too many fonts.

/be

	Alan Pierce Assignee
	Comment 10 • 14 years ago

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/c649f828e197