Make StringBuffer and interpolation behave the same.

Make StringBuffer and interpolation behave the same.

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

[ahe, 2012-12-20 15:22:58]

In my opinion this is a win-win-win situation:

* Fail-fast behavior means better error message for the user

* StringBuffer and interpolation behaves the same

* It is easier to optimize

Cheers,
Peter

[srdjan, 2012-12-20 16:40:29]

I do not like the change.

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
File dart/sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
dart/sdk/lib/core/string_buffer.dart:57: String str = '$obj';
For the VM, string interpolation is slower than calling toString and I do not
see benefits. if 'obj' does not have a toString method that returns String, than
we throw an illegal argument error when interpolating. We can to the same
explicitly here.

[Ivan Posva, 2012-12-20 16:42:41]

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
File dart/sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
dart/sdk/lib/core/string_buffer.dart:57: String str = '$obj';
I do not see the purpose of this change. Please explain this particular line.

[ahe, 2012-12-20 17:10:36]

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
File dart/sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
dart/sdk/lib/core/string_buffer.dart:57: String str = '$obj';
On 2012/12/20 16:40:29, srdjan wrote:
> For the VM, string interpolation is slower than calling toString and I do not
> see benefits. if 'obj' does not have a toString method that returns String,
than
> we throw an illegal argument error when interpolating. We can to the same
> explicitly here.

The purpose of this change is to get sane semantics. Once we agree on the
semantics, both teams can optimize this function.  I have a CL with a faster
version for dart2js and will move this class to a VM specific patch file.

As far as I can tell, the current version will be harder for you to optimize.
The problem is the surprising behavior that happens if str is a non-empty list.

[sra1, 2012-12-20 19:07:41]

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
File dart/sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
dart/sdk/lib/core/string_buffer.dart:57: String str = '$obj';
On 2012/12/20 16:40:29, srdjan wrote:
> For the VM, string interpolation is slower than calling toString and I do not
> see benefits. if 'obj' does not have a toString method that returns String,
than
> we throw an illegal argument error when interpolating. We can to the same
> explicitly here.

obj.toString() should almost never be called explicitly.
Interpolation is clearer at the source level and the guarantees it provides
gives more robust applications.

If
  '$obj'
is slower than
  temp = obj.toString();
  if (temp is! String) throw ...
then fix the VM.

'$obj' is cleaner and guarantees a string.

Not only is preferring interpolation better style and better practice, it is
also better for compiled code size.  If, in the core library, we have a piece of
code that does a check, and another piece of code that does the same check, that
makes every application compiled to JavaScript bigger.  Presumably a VM snapshot
is a little bigger too.

[Lasse Reichstein Nielsen, 2012-12-20 23:21:57]

lgtm

https://codereview.chromium.org/11618037/diff/1/dart/tests/language/string_in...
File dart/tests/language/string_interpolation_and_buffer.dart (right):

https://codereview.chromium.org/11618037/diff/1/dart/tests/language/string_in...
dart/tests/language/string_interpolation_and_buffer.dart:37: return 'GOOD';
Use 'Success', matching 'Error'. Or 'Good'/'Bad'. The dissonance in the current
version is distracting!

[ahe, 2012-12-21 11:23:38]

Thank you for your suggestions and comments.

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
File dart/sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/11618037/diff/1/dart/sdk/lib/core/string_buff...
dart/sdk/lib/core/string_buffer.dart:57: String str = '$obj';
On 2012/12/20 16:40:29, srdjan wrote:
> For the VM, string interpolation is slower than calling toString and I do not
> see benefits. if 'obj' does not have a toString method that returns String,
than
> we throw an illegal argument error when interpolating. We can to the same
> explicitly here.

I have changed the code to use an explicit check as I have a follow-up CL that
moves this code to a VM-specific patch file.

I think the VM team understands that string interpolation needs to be fast, and
my change represents what is, not what should be.

https://codereview.chromium.org/11618037/diff/1/dart/tests/language/string_in...
File dart/tests/language/string_interpolation_and_buffer.dart (right):

https://codereview.chromium.org/11618037/diff/1/dart/tests/language/string_in...
dart/tests/language/string_interpolation_and_buffer.dart:37: return 'GOOD';
On 2012/12/20 23:21:57, Lasse Reichstein Nielsen wrote:
> Use 'Success', matching 'Error'. Or 'Good'/'Bad'. The dissonance in the
current
> version is distracting!

Done.