fix #30094, assert should work with a function

fix #30094, assert should work with a function

R=vsm@google.com

Committed: https://github.com/dart-lang/sdk/commit/418d11fbd6800f8e3d8e3e3e4239f03ce9372a0e

[Jennifer Messerly, 2017-07-07 00:10:15]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[vsm, 2017-07-07 12:38:47]

https://codereview.chromium.org/2971243003/diff/1/pkg/dev_compiler/tool/input...
File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
(right):

https://codereview.chromium.org/2971243003/diff/1/pkg/dev_compiler/tool/input...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart:705: if
(JS('bool', '!#', condition)) throwAssertionError(message);
If the static type of condition is `dynamic`, we would need to test for a
function at runtime, right?

[vsm, 2017-07-07 17:45:54]

On 2017/07/07 12:38:47, vsm wrote:
>
https://codereview.chromium.org/2971243003/diff/1/pkg/dev_compiler/tool/input...
> File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
> (right):
> 
>
https://codereview.chromium.org/2971243003/diff/1/pkg/dev_compiler/tool/input...
> pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart:705: if
> (JS('bool', '!#', condition)) throwAssertionError(message);
> If the static type of condition is `dynamic`, we would need to test for a
> function at runtime, right?

LGTM with that comment.

[Jennifer Messerly, 2017-07-07 23:43:17]

good catch! the existing test case was missing a bunch of cases.

I've expanded test tests, and fixed DDC to support a "static assert" when the
type is boolean and a "dynamic assert" for when we need our dcall/dtest
semantics. The new behavior should also correctly assert when the value is not a
boolean.

https://codereview.chromium.org/2971243003/diff/10001/pkg/dev_compiler/tool/i...
File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
(right):

https://codereview.chromium.org/2971243003/diff/10001/pkg/dev_compiler/tool/i...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart:706: if
(condition == null) _throwBooleanConversionError();
Now has correct handling for null

https://codereview.chromium.org/2971243003/diff/10001/pkg/dev_compiler/tool/i...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart:716: return
assert_(dtest(value), message);
now correctly handles all cases where a non-boolean is involved

https://codereview.chromium.org/2971243003/diff/10001/pkg/dev_compiler/tool/i...
File pkg/dev_compiler/tool/input_sdk/private/js_helper.dart (right):

https://codereview.chromium.org/2971243003/diff/10001/pkg/dev_compiler/tool/i...
pkg/dev_compiler/tool/input_sdk/private/js_helper.dart:869: (message != null ?
Error.safeToString(message) : "is not true");
this uses Error.safeToString as dart2js does

VM/dart2js do not appear to have a separate impl type for asserts with messages
compared to other asserts.

https://codereview.chromium.org/2971243003/diff/10001/tests/language_strong/a...
File tests/language_strong/assert_with_message_test.dart (right):

https://codereview.chromium.org/2971243003/diff/10001/tests/language_strong/a...
tests/language_strong/assert_with_message_test.dart:55:
Expect.isTrue(e.toString().contains("is not true"));
this test now matches VM behavior

https://codereview.chromium.org/2971243003/diff/10001/tests/language_strong/a...
tests/language_strong/assert_with_message_test.dart:85:
Expect.isTrue(e.toString().contains("Instance of 'ToString'"));
same here

https://codereview.chromium.org/2971243003/diff/10001/tests/language_strong/a...
File tests/language_strong/assertion_test.dart (right):

https://codereview.chromium.org/2971243003/diff/10001/tests/language_strong/a...
tests/language_strong/assertion_test.dart:10: testTrue() {
this test also matches VM behavior

[Jennifer Messerly, 2017-07-07 23:43:29]

PTAL :)

[vsm, 2017-07-10 13:03:12]

lgtm!

[Jennifer Messerly, 2017-07-14 05:51:59]

Description was changed from

==========
fix #30094, assert should work with a function
==========

to

==========
fix #30094, assert should work with a function

R=vsm@google.com

Committed:
https://github.com/dart-lang/sdk/commit/418d11fbd6800f8e3d8e3e3e4239f03ce9372a0e
==========

[Jennifer Messerly, 2017-07-14 05:52:00]

Committed patchset #2 (id:10001) manually as
418d11fbd6800f8e3d8e3e3e4239f03ce9372a0e (presubmit successful).