_Comment made by: favila_
bq. Chromes complains with: "Not optimized. Bad value context for arguments value", looking further at the implementation of goog.string.buildString
Chrome complains about the variadic function dispatch code in the same way, see CLJS-916 plus patch.
bq. I think it might be safe to completely remove the macro since cljs.core/str would then do the same and the JIT is probably smart enough to figure this out (or even Closure when compiling).
The Closure compiler is not smart enough to remove the intermediate array, which is why I filed CLJS-801 (which this ticket rolled back). I don't think JITs can do it either.
I am beginning to wonder if we should ignore the Safari 6.0.5 problem in CLJS-847 that started all this string mess. To recap:
# CLJS-801 is accepted, which removes {{\[123, x].join('')}} in the str macro case in favor of {{''\+123\+(cljs.core/str$arity$1 x)}} style code, which the closure compiler can precompute. At this time, the one-arg cljs.core/str function (not macro) calls toString on its argument.
# CLJS-847 is filed. On Safari 6.0.5 at higher JIT levels, calling toString on some things (possibly only unboxed numbers? definitely not strings) throws a TypeError. This is unquestionably a bug in Safari. David fixes by making one-arg cljs.core/str function call js-str instead of toString. js-str uses string-concat {{''+x}}.
# However, this breaks for objects that define valueOf (issue in current ticket), because in js {{''+x}} is the same as {{''+x.valueOf().toString()}} not {{''+x.toString()}}.
# David considers using {{String()}} and variations but rejects because of performance hit.
# David rolls back CLJS-801 from the string-concat back to the array-join style to fix.
# Nikita and I point out that rolling back CLJS-801 only fixes the str *macro*, not the string *function*, which still uses {{js-str}} and hence string-concat.
# David fixes the str *function* to use goog.string.buildString, which has the behavior of array.join. Behavior is now correct even on Safari 6.0.5.
# Thomas points out that buildString uses arguments in a way unoptimizable by v8, and now the str function (not macro) has a performance regression. He suggests using {{\[].join()}} directly.
So, there's a lot of back and forth on this issue, and it's all because of a bug in Safari 6.0.5 which no one has been able to reproduce first-hand because Safari 6.0.5 is old and rare. For some perspective, Safari 6.0.x was only available on Lion and Mountain Lion between July 25,2012 and June 11,2013. Before July 25,2012 Lion used Safari 5.1.x and there was no Mountain Lion. On June 11, 2013, both Lion and Mountain Lion switched to Safari 6.1.x which does not suffer from the toString TypeError bug (I checked--I have an iMac with Lion on it). The only machines on Safari 6.0.5 are (Mountain) Lion machines which used software updates until late 2012-early 2013 and then stopped. I can't imagine this is a large number of people.
It is theoretically possible for me to run Safari 6.0.x on my Lion machine to actually test this, but I can't find a way to downgrade from 6.1.x.
I think the options are:
# Use array.join() for all stringification and take the performance hit (which we should quantify). Include a comment that this is only for Safari 6.0.x (only confirmed second-hand on 6.0.4 and 6.0.5) for future generations, who are going to think this is weird.
# Use CLJS-801 and toString (status quo before CLJS-847), and ignore this problem for Safari 6.0.x.
# Use CLJS-801, but add a {{number?}} check (with comment) to {{cljs.core/str$arity$1}} for Safari 6.0.5. The number case should use js-str, and the rest toString. I think this will work, but again we have no way to test--we really need to get our hands on a Safari 6.0.x browser.
Of course we should benchmark these approaches but my hunch is that 2 is faster than 3 is faster than 1.
