470609 - TM: convert String objects into primitive strings when invoking String functions on them

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

Attachment: v1

We used to do this explicitly for match(), but we can easily do this for all String functions. This only works for String objects atm. It might be cleaner to use an imacro for the conversion, and then properly support scripted toString() implementations of arbitrary objects.

(If anyone wants to grab this and further improve it, go ahead.)

Comment 1

[Andreas Gal :gal]

This is not a blocker for 1.9.1 imo

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 354016 [details] [diff] [review]
v1

I like the change.  I'm not a peer on JS though!

Comment 3

[Jeff Walden [:Waldo]]

Dangit brendan, rectify this!  Now!  :-P

If I knew we were still using despot for this I'd do it now (think peers have that power) since I know you've expressed surprise at this before, but I'm not sure if despot is relevant or not any more.

Comment 4

[Andreas Gal :gal]

Comment on attachment 354016 [details] [diff] [review]
v1

You wrote the code I changed so I guess you must be qualified at some level :)

Comment 5

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/4acb47a25eb5

Comment 6

[Andreas Gal :gal]

Backed out. Makes tinderboxes orange.

http://hg.mozilla.org/tracemonkey/rev/083a9f80c6ce

Comment 7

[Brendan Eich [:brendan]]

Obviously by peer-duck-typing, jorendorff is a peer. Problem is we are still using despot and jorendorff was only webmonkey, not cvsuser. I made him cvsuser (sue me, Reed :-P) just to be able to add him as peer.

I also moved graydon from member to peer. I'd like to add dmandelin, gal, and danderson but they won't be cvsusers in any future I can foresee, and shouldn't have to go through the hassles. Ah, homebrew tools (despot is still useful, but too tied to CVS).

/be

Comment 8

[Bob Clary [:bc] (inactive)]

(In reply to comment #5)
> http://hg.mozilla.org/tracemonkey/rev/4acb47a25eb5

with jit enabled, this crashes 

<http://test.bclary.com/tests/mozilla.org/js/js-test-driver-standards.html?test=ecma%2FString%2F15.5.4.5-1.js;language=type;text/javascript>

and 

<http://test.bclary.com/tests/mozilla.org/js/js-test-driver-standards.html?test=js1_2%2FString%2Fslice.js;language=type;text/javascript>

Comment 9

[Chris Leary [:cdleary] (not checking bugmail)]

It looks like this was backed out, unmarking as fixed-in-tracemonkey.

Ping @jorendorff.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Obsolete with the removal of tracejit.