Change _interpolateSingle to use the same value in errors as _interpolate.

Change _interpolateSingle to use the same value in errors as _interpolate.

R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=43942

[Lasse Reichstein Nielsen, 2015-02-12 13:11:09]

lrn@google.com changed reviewers:
+ iposva@chromium.org

[Lasse Reichstein Nielsen, 2015-02-12 13:11:10]

https://codereview.chromium.org/920793002/diff/1/runtime/lib/string_patch.dart
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/920793002/diff/1/runtime/lib/string_patch.dar...
runtime/lib/string_patch.dart:802: throw new ArgumentError(s);
Using "o", which we already know has a faulty toString, means that the error
can't be printed either. We used "s" in _interpolate below.

Using "s" reduces the problem (but using Error.safeToString(s)) would likely be
even better.

[Ivan Posva, 2015-02-20 06:43:48]

iposva@google.com changed reviewers:
+ iposva@google.com

[Ivan Posva, 2015-02-20 06:43:48]

LGTM for consistency, but we should really switch to using safeToString.

-Ivan

https://codereview.chromium.org/920793002/diff/1/runtime/lib/string_patch.dart
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/920793002/diff/1/runtime/lib/string_patch.dar...
runtime/lib/string_patch.dart:802: throw new ArgumentError(s);
On 2015/02/12 13:11:09, Lasse Reichstein Nielsen wrote:
> Using "o", which we already know has a faulty toString, means that the error
> can't be printed either. We used "s" in _interpolate below.
> 
> Using "s" reduces the problem (but using Error.safeToString(s)) would likely
be
> even better.

Since you know s is not a String, you could also use o for the safeToString
call. I am not sure how much it helps here to try to use s in the ArgumentError
as s is not the faulty argument.

[Lasse Reichstein Nielsen, 2015-02-23 08:10:15]

Committed patchset #1 (id:1) manually as 43942 (presubmit successful).

[sra1, 2015-02-24 16:57:02]

sra@google.com changed reviewers:
+ sra@google.com

[sra1, 2015-02-24 16:57:02]

https://codereview.chromium.org/920793002/diff/1/runtime/lib/string_patch.dart
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/920793002/diff/1/runtime/lib/string_patch.dar...
runtime/lib/string_patch.dart:802: throw new ArgumentError(s);
On 2015/02/12 13:11:09, Lasse Reichstein Nielsen wrote:
> Using "o", which we already know has a faulty toString, means that the error
> can't be printed either. We used "s" in _interpolate below.
> 
> Using "s" reduces the problem (but using Error.safeToString(s)) would likely
be
> even better.

It seems a bad idea to report the wrong irritant like this (e.g. null).

We would not worry about this if ArgumentError was robust and reported a bad
object for the 'message' , e.g.  "Instance of ${o.runtimeType} (with bad
toString() method)", or "unprintable object" if that fails too.

That would also help other places that throw errors with bad toString methods.

We could still report something special here.

If this throw was changed to

   throw _interpolationNotStringError(o, s);

then the helper could be called from _interpolate too to ensure consistency,
probably throwing something like ArgumentError.value(o, 'interpolated value',
'toString() method does not return a String')