Skip to content

Commit 23cbe39

Browse files
nshahancommit-bot@chromium.org
nshahan
authored and
[email protected]
committed
[ddc] Remove error handler from number parse helpers
This copies the logic from dart2js and brings the two patches closer together. There is no need to pass the error handler to the helper function shared between `parse()` and `tryParse()`. Update JS foreign function types in parseDouble to match the NNBD version. Fixes: #41613 Change-Id: I85d39f8ba6bd76459cf6263099983b8b96d3a669 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144304 Reviewed-by: Stephen Adams <[email protected]> Reviewed-by: Sigmund Cherem <[email protected]> Commit-Queue: Nicholas Shahan <[email protected]>
1 parent a934ec2 commit 23cbe39

File tree

7 files changed

+68
-67
lines changed

7 files changed

+68
-67
lines changed

‎pkg/dev_compiler/tool/dartdevc_nnbd_sdk_error_golden.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
ERROR|COMPILE_TIME_ERROR|CONST_CONSTRUCTOR_THROWS_EXCEPTION|lib/core/core.dart|3716|5|94|Const constructors can't throw exceptions.
2-
ERROR|COMPILE_TIME_ERROR|CONST_CONSTRUCTOR_THROWS_EXCEPTION|lib/core/core.dart|7902|5|97|Const constructors can't throw exceptions.
1+
ERROR|COMPILE_TIME_ERROR|CONST_CONSTRUCTOR_THROWS_EXCEPTION|lib/core/core.dart|3719|5|94|Const constructors can't throw exceptions.
2+
ERROR|COMPILE_TIME_ERROR|CONST_CONSTRUCTOR_THROWS_EXCEPTION|lib/core/core.dart|7908|5|97|Const constructors can't throw exceptions.
33
ERROR|COMPILE_TIME_ERROR|CONST_CONSTRUCTOR_THROWS_EXCEPTION|lib/core/core.dart|940|5|95|Const constructors can't throw exceptions.
44
ERROR|COMPILE_TIME_ERROR|CONST_CONSTRUCTOR_THROWS_EXCEPTION|lib/core/core.dart|973|5|94|Const constructors can't throw exceptions.
55
ERROR|STATIC_WARNING|ARGUMENT_TYPE_NOT_ASSIGNABLE|lib/io/io.dart|6334|51|12|The argument type 'Object' can't be assigned to the parameter type 'String'.
6-
ERROR|SYNTACTIC_ERROR|CONST_FACTORY|lib/core/core.dart|3714|3|5|Only redirecting factory constructors can be declared to be 'const'.
7-
ERROR|SYNTACTIC_ERROR|CONST_FACTORY|lib/core/core.dart|7900|3|5|Only redirecting factory constructors can be declared to be 'const'.
6+
ERROR|SYNTACTIC_ERROR|CONST_FACTORY|lib/core/core.dart|3717|3|5|Only redirecting factory constructors can be declared to be 'const'.
7+
ERROR|SYNTACTIC_ERROR|CONST_FACTORY|lib/core/core.dart|7906|3|5|Only redirecting factory constructors can be declared to be 'const'.
88
ERROR|SYNTACTIC_ERROR|CONST_FACTORY|lib/core/core.dart|938|3|5|Only redirecting factory constructors can be declared to be 'const'.
99
ERROR|SYNTACTIC_ERROR|CONST_FACTORY|lib/core/core.dart|971|3|5|Only redirecting factory constructors can be declared to be 'const'.

‎sdk/lib/_internal/js_dev_runtime/patch/core_patch.dart

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,19 +166,20 @@ class Expando<T> {
166166
static int _keyCount = 0;
167167
}
168168

169-
Null _kNull(_) => null;
170-
171169
@patch
172170
class int {
173171
@patch
174172
static int parse(String source,
175173
{int radix, @deprecated int onError(String source)}) {
176-
return Primitives.parseInt(source, radix, onError);
174+
var value = tryParse(source, radix: radix);
175+
if (value != null) return value;
176+
if (onError != null) return onError(source);
177+
throw new FormatException(source);
177178
}
178179

179180
@patch
180181
static int tryParse(String source, {int radix}) {
181-
return Primitives.parseInt(source, radix, _kNull);
182+
return Primitives.parseInt(source, radix);
182183
}
183184

184185
@patch
@@ -209,12 +210,15 @@ class double {
209210
@patch
210211
static double parse(String source,
211212
[@deprecated double onError(String source)]) {
212-
return Primitives.parseDouble(source, onError);
213+
var value = tryParse(source);
214+
if (value != null) return value;
215+
if (onError != null) return onError(source);
216+
throw new FormatException('Invalid double', source);
213217
}
214218

215219
@patch
216220
static double tryParse(String source) {
217-
return Primitives.parseDouble(source, _kNull);
221+
return Primitives.parseDouble(source);
218222
}
219223

220224
@JSExportName('is')

‎sdk/lib/_internal/js_dev_runtime/private/js_helper.dart

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,7 @@ class SyncIterable<E> extends IterableBase<E> {
7070
}
7171

7272
class Primitives {
73-
@NoInline()
74-
static int _parseIntError(String source, int handleError(String source)) {
75-
if (handleError == null) throw FormatException(source);
76-
return handleError(source);
77-
}
78-
79-
static int parseInt(
80-
@nullCheck String source, int _radix, int handleError(String source)) {
73+
static int parseInt(@nullCheck String source, int _radix) {
8174
var re = JS('', r'/^\s*[+-]?((0x[a-f0-9]+)|(\d+)|([a-z0-9]+))\s*$/i');
8275
// TODO(jmesserly): this isn't reified List<String>, but it's safe to use as
8376
// long as we use it locally and don't expose it to user code.
@@ -89,7 +82,7 @@ class Primitives {
8982
// TODO(sra): It might be that the match failed due to unrecognized U+0085
9083
// spaces. We could replace them with U+0020 spaces and try matching
9184
// again.
92-
return _parseIntError(source, handleError);
85+
return null;
9386
}
9487
String decimalMatch = match[decimalIndex];
9588
if (_radix == null) {
@@ -101,7 +94,7 @@ class Primitives {
10194
// Cannot fail because we know that the digits are all hex.
10295
return JS<int>('!', r'parseInt(#, 16)', source);
10396
}
104-
return _parseIntError(source, handleError);
97+
return null;
10598
}
10699
@notNull
107100
var radix = _radix;
@@ -138,7 +131,7 @@ class Primitives {
138131
for (int i = 0; i < digitsPart.length; i++) {
139132
int characterCode = digitsPart.codeUnitAt(i) | 0x20;
140133
if (characterCode > maxCharCode) {
141-
return _parseIntError(source, handleError);
134+
return null;
142135
}
143136
}
144137
}
@@ -147,17 +140,7 @@ class Primitives {
147140
return JS<int>('!', r'parseInt(#, #)', source, radix);
148141
}
149142

150-
@NoInline()
151-
static double _parseDoubleError(
152-
String source, double handleError(String source)) {
153-
if (handleError == null) {
154-
throw FormatException('Invalid double', source);
155-
}
156-
return handleError(source);
157-
}
158-
159-
static double parseDouble(
160-
@nullCheck String source, double handleError(String source)) {
143+
static double parseDouble(@nullCheck String source) {
161144
// Notice that JS parseFloat accepts garbage at the end of the string.
162145
// Accept only:
163146
// - [+/-]NaN
@@ -169,15 +152,15 @@ class Primitives {
169152
r'/^\s*[+-]?(?:Infinity|NaN|'
170153
r'(?:\.\d+|\d+(?:\.\d*)?)(?:[eE][+-]?\d+)?)\s*$/.test(#)',
171154
source)) {
172-
return _parseDoubleError(source, handleError);
155+
return null;
173156
}
174-
num result = JS('!', r'parseFloat(#)', source);
157+
var result = JS<double>('!', r'parseFloat(#)', source);
175158
if (result.isNaN) {
176159
var trimmed = source.trim();
177160
if (trimmed == 'NaN' || trimmed == '+NaN' || trimmed == '-NaN') {
178161
return result;
179162
}
180-
return _parseDoubleError(source, handleError);
163+
return null;
181164
}
182165
return result;
183166
}

‎sdk_nnbd/lib/_internal/js_dev_runtime/patch/core_patch.dart

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,19 +167,20 @@ class Expando<T extends Object> {
167167
static int _keyCount = 0;
168168
}
169169

170-
Null _kNull(_) => null;
171-
172170
@patch
173171
class int {
174172
@patch
175173
static int parse(String source,
176174
{int? radix, @deprecated int onError(String source)?}) {
177-
return Primitives.parseInt(source, radix, onError)!;
175+
var value = tryParse(source, radix: radix);
176+
if (value != null) return value;
177+
if (onError != null) return onError(source);
178+
throw new FormatException(source);
178179
}
179180

180181
@patch
181182
static int? tryParse(String source, {int? radix}) {
182-
return Primitives.parseInt(source, radix, _kNull);
183+
return Primitives.parseInt(source, radix);
183184
}
184185

185186
@patch
@@ -209,12 +210,15 @@ class double {
209210
@patch
210211
static double parse(String source,
211212
[@deprecated double onError(String source)?]) {
212-
return Primitives.parseDouble(source, onError)!;
213+
var value = tryParse(source);
214+
if (value != null) return value;
215+
if (onError != null) return onError(source);
216+
throw new FormatException('Invalid double', source);
213217
}
214218

215219
@patch
216220
static double? tryParse(String source) {
217-
return Primitives.parseDouble(source, _kNull);
221+
return Primitives.parseDouble(source);
218222
}
219223

220224
@JSExportName('is')

‎sdk_nnbd/lib/_internal/js_dev_runtime/private/js_helper.dart

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,7 @@ class SyncIterable<E> extends IterableBase<E> {
6868
}
6969

7070
class Primitives {
71-
@NoInline()
72-
static int? _parseIntError(
73-
String source, int? Function(String)? handleError) {
74-
if (handleError == null) throw FormatException(source);
75-
return handleError(source);
76-
}
77-
78-
static int? parseInt(@nullCheck String source, int? _radix,
79-
int? Function(String)? handleError) {
71+
static int? parseInt(@nullCheck String source, int? _radix) {
8072
var re = JS('', r'/^\s*[+-]?((0x[a-f0-9]+)|(\d+)|([a-z0-9]+))\s*$/i');
8173
// TODO(jmesserly): this isn't reified List<String>, but it's safe to use as
8274
// long as we use it locally and don't expose it to user code.
@@ -88,7 +80,7 @@ class Primitives {
8880
// TODO(sra): It might be that the match failed due to unrecognized U+0085
8981
// spaces. We could replace them with U+0020 spaces and try matching
9082
// again.
91-
return _parseIntError(source, handleError);
83+
return null;
9284
}
9385
String? decimalMatch = match[decimalIndex];
9486
if (_radix == null) {
@@ -100,7 +92,7 @@ class Primitives {
10092
// Cannot fail because we know that the digits are all hex.
10193
return JS<int>('!', r'parseInt(#, 16)', source);
10294
}
103-
return _parseIntError(source, handleError);
95+
return null;
10496
}
10597
@notNull
10698
var radix = _radix;
@@ -137,7 +129,7 @@ class Primitives {
137129
for (int i = 0; i < digitsPart.length; i++) {
138130
int characterCode = digitsPart.codeUnitAt(i) | 0x20;
139131
if (characterCode > maxCharCode) {
140-
return _parseIntError(source, handleError);
132+
return null;
141133
}
142134
}
143135
}
@@ -146,17 +138,7 @@ class Primitives {
146138
return JS<int>('!', r'parseInt(#, #)', source, radix);
147139
}
148140

149-
@NoInline()
150-
static double? _parseDoubleError(
151-
String source, double? Function(String)? handleError) {
152-
if (handleError == null) {
153-
throw FormatException('Invalid double', source);
154-
}
155-
return handleError(source);
156-
}
157-
158-
static double? parseDouble(
159-
@nullCheck String source, double? Function(String)? handleError) {
141+
static double? parseDouble(@nullCheck String source) {
160142
// Notice that JS parseFloat accepts garbage at the end of the string.
161143
// Accept only:
162144
// - [+/-]NaN
@@ -168,15 +150,15 @@ class Primitives {
168150
r'/^\s*[+-]?(?:Infinity|NaN|'
169151
r'(?:\.\d+|\d+(?:\.\d*)?)(?:[eE][+-]?\d+)?)\s*$/.test(#)',
170152
source)) {
171-
return _parseDoubleError(source, handleError);
153+
return null;
172154
}
173-
double result = JS('!', r'parseFloat(#)', source);
155+
var result = JS<double>('!', r'parseFloat(#)', source);
174156
if (result.isNaN) {
175157
var trimmed = source.trim();
176158
if (trimmed == 'NaN' || trimmed == '+NaN' || trimmed == '-NaN') {
177159
return result;
178160
}
179-
return _parseDoubleError(source, handleError);
161+
return null;
180162
}
181163
return result;
182164
}

‎tests/language/regress/regress41613_test.dart

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// @dart=2.7
6+
7+
// Requirements=nnbd-weak
8+
9+
import "package:expect/expect.dart";
10+
11+
main() {
12+
// In legacy Dart 2 and null safety weak mode the error handlers can
13+
// return null.
14+
Expect.equals(null, int.parse("foo", onError: (_) => null));
15+
Expect.equals(null, double.parse("foo", (_) => null));
16+
}

‎tests/language_2/regress/regress41613_test.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import "package:expect/expect.dart";
6+
7+
main() {
8+
// In legacy Dart 2 and null safety weak mode the error handlers can
9+
// return null.
10+
Expect.equals(null, int.parse("foo", onError: (_) => null));
11+
Expect.equals(null, double.parse("foo", (_) => null));
12+
}

0 commit comments

Comments
 (0)