Skip to content

Commit 08f90ca

Browse files
rakudramacommit-bot@chromium.org
rakudrama
authored and
[email protected]
committed
[dart2js] Use 'num(ber)' wherever possible instead of 'double'
JSDouble represents doubles that are not integers. It is usually wrong to use JSDouble, so use JSNumber instead where we mean 'all double values'. This fixes #44818 by not mistakenly pretending that division cannot have an integral result. Will follow up with a CL to rename JSDouble. Fixed: 44818 Change-Id: Ic324df2ee1f2c7434bc0b064c0dbd7b6800ad93b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183360 Commit-Queue: Stephen Adams <[email protected]> Reviewed-by: Sigmund Cherem <[email protected]>
1 parent 680f5e6 commit 08f90ca

23 files changed

+61
-135
lines changed

‎pkg/compiler/lib/src/inferrer/abstract_value_domain.dart

-8
Original file line numberDiff line numberDiff line change
@@ -391,14 +391,6 @@ abstract class AbstractValueDomain {
391391
/// number or `null` at runtime.
392392
AbstractBool isNumberOrNull(covariant AbstractValue value);
393393

394-
/// Returns an [AbstractBool] that describes whether [value] is a non-integer
395-
/// number at runtime.
396-
AbstractBool isDouble(covariant AbstractValue value);
397-
398-
/// Returns an [AbstractBool] that describes whether [value] is a non-integer
399-
/// number or `null` at runtime.
400-
AbstractBool isDoubleOrNull(covariant AbstractValue value);
401-
402394
/// Returns an [AbstractBool] that describes whether [value] is a JavaScript
403395
/// bool at runtime.
404396
AbstractBool isBoolean(covariant AbstractValue value);

‎pkg/compiler/lib/src/inferrer/powersets/powerset_bits.dart

-4
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,6 @@ class PowersetBitsDomain {
244244
return AbstractBool.Maybe;
245245
}
246246

247-
AbstractBool isDoubleOrNull(int value) => isDouble(excludeNull(value));
248-
249-
AbstractBool isDouble(int value) => isOther(value);
250-
251247
AbstractBool isNumberOrNull(int value) => isNumber(excludeNull(value));
252248

253249
AbstractBool isNumber(int value) => isOther(value);

‎pkg/compiler/lib/src/inferrer/powersets/powersets.dart

-11
Original file line numberDiff line numberDiff line change
@@ -435,17 +435,6 @@ class PowersetDomain implements AbstractValueDomain {
435435
AbstractBool.strengthen(_powersetBitsDomain.isTruthy(value._powersetBits),
436436
_abstractValueDomain.isTruthy(value._abstractValue));
437437

438-
@override
439-
AbstractBool isDoubleOrNull(covariant PowersetValue value) =>
440-
AbstractBool.strengthen(
441-
_powersetBitsDomain.isDoubleOrNull(value._powersetBits),
442-
_abstractValueDomain.isDoubleOrNull(value._abstractValue));
443-
444-
@override
445-
AbstractBool isDouble(covariant PowersetValue value) =>
446-
AbstractBool.strengthen(_powersetBitsDomain.isDouble(value._powersetBits),
447-
_abstractValueDomain.isDouble(value._abstractValue));
448-
449438
@override
450439
AbstractBool isNumberOrNull(covariant PowersetValue value) =>
451440
AbstractBool.strengthen(

‎pkg/compiler/lib/src/inferrer/powersets/wrapped.dart

-8
Original file line numberDiff line numberDiff line change
@@ -318,14 +318,6 @@ class WrappedAbstractValueDomain implements AbstractValueDomain {
318318
AbstractBool isTruthy(covariant WrappedAbstractValue value) =>
319319
_abstractValueDomain.isTruthy(value._abstractValue);
320320

321-
@override
322-
AbstractBool isDoubleOrNull(covariant WrappedAbstractValue value) =>
323-
_abstractValueDomain.isDoubleOrNull(value._abstractValue);
324-
325-
@override
326-
AbstractBool isDouble(covariant WrappedAbstractValue value) =>
327-
_abstractValueDomain.isDouble(value._abstractValue);
328-
329321
@override
330322
AbstractBool isNumberOrNull(covariant WrappedAbstractValue value) =>
331323
_abstractValueDomain.isNumberOrNull(value._abstractValue);

‎pkg/compiler/lib/src/inferrer/trivial.dart

-6
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,6 @@ class TrivialAbstractValueDomain implements AbstractValueDomain {
225225
@override
226226
AbstractBool isTruthy(AbstractValue value) => AbstractBool.Maybe;
227227

228-
@override
229-
AbstractBool isDoubleOrNull(AbstractValue value) => AbstractBool.Maybe;
230-
231-
@override
232-
AbstractBool isDouble(AbstractValue value) => AbstractBool.Maybe;
233-
234228
@override
235229
AbstractBool isNumberOrNull(AbstractValue value) => AbstractBool.Maybe;
236230

‎pkg/compiler/lib/src/inferrer/type_system.dart

-7
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,6 @@ class TypeSystem {
165165
getConcreteTypeFor(_abstractValueDomain.positiveIntType);
166166
}
167167

168-
TypeInformation doubleTypeCache;
169-
TypeInformation get doubleType {
170-
if (doubleTypeCache != null) return doubleTypeCache;
171-
return doubleTypeCache =
172-
getConcreteTypeFor(_abstractValueDomain.doubleType);
173-
}
174-
175168
TypeInformation numTypeCache;
176169
TypeInformation get numType {
177170
if (numTypeCache != null) return numTypeCache;

‎pkg/compiler/lib/src/inferrer/typemasks/flat_type_mask.dart

+6-15
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ class FlatTypeMask implements TypeMask {
157157
}
158158
}
159159

160-
bool isSingleImplementationOf(ClassEntity cls, JClosedWorld closedWorld) {
160+
bool _isSingleImplementationOf(ClassEntity cls, JClosedWorld closedWorld) {
161161
// Special case basic types so that, for example, JSString is the
162162
// single implementation of String.
163163
// The general optimization is to realize there is only one class that
@@ -179,10 +179,6 @@ class FlatTypeMask implements TypeMask {
179179
cls == commonElements.jsUInt32Class ||
180180
cls == commonElements.jsUInt31Class;
181181
}
182-
if (containsOnlyDouble(closedWorld)) {
183-
return cls == closedWorld.commonElements.doubleClass ||
184-
cls == commonElements.jsDoubleClass;
185-
}
186182
return false;
187183
}
188184

@@ -200,10 +196,10 @@ class FlatTypeMask implements TypeMask {
200196
FlatTypeMask flatOther = other;
201197
ClassEntity otherBase = flatOther.base;
202198
// If other is exact, it only contains its base.
203-
// TODO(herhut): Get rid of isSingleImplementationOf.
199+
// TODO(herhut): Get rid of _isSingleImplementationOf.
204200
if (flatOther.isExact) {
205201
return (isExact && base == otherBase) ||
206-
isSingleImplementationOf(otherBase, closedWorld);
202+
_isSingleImplementationOf(otherBase, closedWorld);
207203
}
208204
// If other is subclass, this has to be subclass, as well. Unless
209205
// flatOther.base covers all subtypes of this. Currently, we only
@@ -228,23 +224,18 @@ class FlatTypeMask implements TypeMask {
228224
@override
229225
bool containsOnlyInt(JClosedWorld closedWorld) {
230226
CommonElements commonElements = closedWorld.commonElements;
231-
return base == closedWorld.commonElements.intClass ||
227+
return base == commonElements.intClass ||
232228
base == commonElements.jsIntClass ||
233229
base == commonElements.jsPositiveIntClass ||
234230
base == commonElements.jsUInt31Class ||
235231
base == commonElements.jsUInt32Class;
236232
}
237233

238-
@override
239-
bool containsOnlyDouble(JClosedWorld closedWorld) {
240-
return base == closedWorld.commonElements.doubleClass ||
241-
base == closedWorld.commonElements.jsDoubleClass;
242-
}
243-
244234
@override
245235
bool containsOnlyNum(JClosedWorld closedWorld) {
246236
return containsOnlyInt(closedWorld) ||
247-
containsOnlyDouble(closedWorld) ||
237+
base == closedWorld.commonElements.doubleClass ||
238+
base == closedWorld.commonElements.jsDoubleClass ||
248239
base == closedWorld.commonElements.numClass ||
249240
base == closedWorld.commonElements.jsNumberClass;
250241
}

‎pkg/compiler/lib/src/inferrer/typemasks/forwarding_type_mask.dart

-5
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,6 @@ abstract class ForwardingTypeMask implements TypeMask {
5252
return forwardTo.containsOnlyInt(closedWorld);
5353
}
5454

55-
@override
56-
bool containsOnlyDouble(JClosedWorld closedWorld) {
57-
return forwardTo.containsOnlyDouble(closedWorld);
58-
}
59-
6055
@override
6156
bool containsOnlyNum(JClosedWorld closedWorld) {
6257
return forwardTo.containsOnlyNum(closedWorld);

‎pkg/compiler/lib/src/inferrer/typemasks/masks.dart

-11
Original file line numberDiff line numberDiff line change
@@ -627,17 +627,6 @@ class CommonMasks implements AbstractValueDomain {
627627
return value.containsOnlyNum(_closedWorld);
628628
}
629629

630-
@override
631-
AbstractBool isDouble(TypeMask value) {
632-
return AbstractBool.trueOrMaybe(
633-
value.containsOnlyDouble(_closedWorld) && !value.isNullable);
634-
}
635-
636-
@override
637-
AbstractBool isDoubleOrNull(TypeMask value) {
638-
return AbstractBool.trueOrMaybe(value.containsOnlyDouble(_closedWorld));
639-
}
640-
641630
@override
642631
AbstractBool isBoolean(TypeMask value) {
643632
return AbstractBool.trueOrMaybe(

‎pkg/compiler/lib/src/inferrer/typemasks/type_mask.dart

-1
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,6 @@ abstract class TypeMask implements AbstractValue {
354354
bool get isValue;
355355

356356
bool containsOnlyInt(JClosedWorld closedWorld);
357-
bool containsOnlyDouble(JClosedWorld closedWorld);
358357
bool containsOnlyNum(JClosedWorld closedWorld);
359358
bool containsOnlyBool(JClosedWorld closedWorld);
360359
bool containsOnlyString(JClosedWorld closedWorld);

‎pkg/compiler/lib/src/inferrer/typemasks/union_type_mask.dart

-5
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,6 @@ class UnionTypeMask implements TypeMask {
358358
return disjointMasks.every((mask) => mask.containsOnlyInt(closedWorld));
359359
}
360360

361-
@override
362-
bool containsOnlyDouble(JClosedWorld closedWorld) {
363-
return disjointMasks.every((mask) => mask.containsOnlyDouble(closedWorld));
364-
}
365-
366361
@override
367362
bool containsOnlyNum(JClosedWorld closedWorld) {
368363
return disjointMasks.every((mask) {

‎pkg/compiler/lib/src/js_backend/codegen_listener.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,7 @@ class CodegenEnqueuerListener extends EnqueuerListener {
271271
registerInstantiation(_commonElements.jsUInt32Class);
272272
registerInstantiation(_commonElements.jsUInt31Class);
273273
registerInstantiation(_commonElements.jsNumberClass);
274-
} else if (cls == _commonElements.doubleClass ||
275-
cls == _commonElements.jsDoubleClass) {
274+
} else if (cls == _commonElements.jsDoubleClass) {
276275
registerInstantiation(_commonElements.jsDoubleClass);
277276
registerInstantiation(_commonElements.jsNumberClass);
278277
} else if (cls == _commonElements.boolClass ||
@@ -281,7 +280,8 @@ class CodegenEnqueuerListener extends EnqueuerListener {
281280
} else if (cls == _commonElements.nullClass ||
282281
cls == _commonElements.jsNullClass) {
283282
registerInstantiation(_commonElements.jsNullClass);
284-
} else if (cls == _commonElements.numClass ||
283+
} else if (cls == _commonElements.doubleClass ||
284+
cls == _commonElements.numClass ||
285285
cls == _commonElements.jsNumberClass) {
286286
registerInstantiation(_commonElements.jsIntClass);
287287
registerInstantiation(_commonElements.jsPositiveIntClass);

‎pkg/compiler/lib/src/js_backend/resolution_listener.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,7 @@ class ResolutionEnqueuerListener extends EnqueuerListener {
373373
_addInterceptors(_commonElements.jsUInt32Class, impactBuilder);
374374
_addInterceptors(_commonElements.jsUInt31Class, impactBuilder);
375375
_addInterceptors(_commonElements.jsNumberClass, impactBuilder);
376-
} else if (cls == _commonElements.doubleClass ||
377-
cls == _commonElements.jsDoubleClass) {
376+
} else if (cls == _commonElements.jsDoubleClass) {
378377
_addInterceptors(_commonElements.jsDoubleClass, impactBuilder);
379378
_addInterceptors(_commonElements.jsNumberClass, impactBuilder);
380379
} else if (cls == _commonElements.boolClass ||
@@ -383,7 +382,8 @@ class ResolutionEnqueuerListener extends EnqueuerListener {
383382
} else if (cls == _commonElements.nullClass ||
384383
cls == _commonElements.jsNullClass) {
385384
_addInterceptors(_commonElements.jsNullClass, impactBuilder);
386-
} else if (cls == _commonElements.numClass ||
385+
} else if (cls == _commonElements.doubleClass ||
386+
cls == _commonElements.numClass ||
387387
cls == _commonElements.jsNumberClass) {
388388
_addInterceptors(_commonElements.jsIntClass, impactBuilder);
389389
_addInterceptors(_commonElements.jsPositiveIntClass, impactBuilder);

‎pkg/compiler/lib/src/js_backend/specialized_checks.dart

+2-4
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ class SpecializedChecks {
5757
return IsTestSpecialization.bool;
5858
}
5959

60-
if (element == commonElements.jsDoubleClass ||
61-
element == commonElements.doubleClass ||
60+
if (element == commonElements.doubleClass ||
6261
element == commonElements.jsNumberClass ||
6362
element == commonElements.numClass) {
6463
return IsTestSpecialization.num;
@@ -166,8 +165,7 @@ class SpecializedChecks {
166165
return commonElements.specializedAsBool;
167166
}
168167

169-
if (element == commonElements.jsDoubleClass ||
170-
element == commonElements.doubleClass) {
168+
if (element == commonElements.doubleClass) {
171169
if (legacy) return commonElements.specializedAsDoubleLegacy;
172170
if (nullable) return commonElements.specializedAsDoubleNullable;
173171
return commonElements.specializedAsDouble;

‎pkg/compiler/lib/src/ssa/interceptor_simplifier.dart

-2
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor
133133
}
134134
} else if (_abstractValueDomain.isIntegerOrNull(type).isDefinitelyTrue) {
135135
return _commonElements.jsIntClass;
136-
} else if (_abstractValueDomain.isDoubleOrNull(type).isDefinitelyTrue) {
137-
return _commonElements.jsDoubleClass;
138136
} else if (_abstractValueDomain.isBooleanOrNull(type).isDefinitelyTrue) {
139137
return _commonElements.jsBoolClass;
140138
} else if (_abstractValueDomain.isStringOrNull(type).isDefinitelyTrue) {

‎pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart

+2-15
Original file line numberDiff line numberDiff line change
@@ -404,11 +404,6 @@ class UnaryNegateSpecializer extends InvokeDynamicSpecializer {
404404
.isDefinitelyTrue) {
405405
return closedWorld.abstractValueDomain.intType;
406406
}
407-
if (operand
408-
.isDoubleOrNull(closedWorld.abstractValueDomain)
409-
.isDefinitelyTrue) {
410-
return closedWorld.abstractValueDomain.doubleType;
411-
}
412407
return closedWorld.abstractValueDomain.numType;
413408
}
414409
return super.computeTypeFromInputTypes(instruction, results, closedWorld);
@@ -492,14 +487,6 @@ abstract class BinaryArithmeticSpecializer extends InvokeDynamicSpecializer {
492487
return closedWorld.abstractValueDomain.intType;
493488
}
494489
if (left.isNumberOrNull(closedWorld.abstractValueDomain).isDefinitelyTrue) {
495-
if (left
496-
.isDoubleOrNull(closedWorld.abstractValueDomain)
497-
.isDefinitelyTrue ||
498-
right
499-
.isDoubleOrNull(closedWorld.abstractValueDomain)
500-
.isDefinitelyTrue) {
501-
return closedWorld.abstractValueDomain.doubleType;
502-
}
503490
return closedWorld.abstractValueDomain.numType;
504491
}
505492
return super.computeTypeFromInputTypes(instruction, results, closedWorld);
@@ -621,7 +608,7 @@ class DivideSpecializer extends BinaryArithmeticSpecializer {
621608
GlobalTypeInferenceResults results, JClosedWorld closedWorld) {
622609
HInstruction left = instruction.inputs[1];
623610
if (left.isNumberOrNull(closedWorld.abstractValueDomain).isDefinitelyTrue) {
624-
return closedWorld.abstractValueDomain.doubleType;
611+
return closedWorld.abstractValueDomain.numType;
625612
}
626613
return super.computeTypeFromInputTypes(instruction, results, closedWorld);
627614
}
@@ -630,7 +617,7 @@ class DivideSpecializer extends BinaryArithmeticSpecializer {
630617
HInstruction newBuiltinVariant(HInvokeDynamic instruction,
631618
GlobalTypeInferenceResults results, JClosedWorld closedWorld) {
632619
return new HDivide(instruction.inputs[1], instruction.inputs[2],
633-
closedWorld.abstractValueDomain.doubleType);
620+
closedWorld.abstractValueDomain.numType);
634621
}
635622

636623
@override

‎pkg/compiler/lib/src/ssa/nodes.dart

-19
Original file line numberDiff line numberDiff line change
@@ -1207,12 +1207,6 @@ abstract class HInstruction implements Spannable {
12071207
AbstractBool isNumberOrNull(AbstractValueDomain domain) =>
12081208
domain.isNumberOrNull(instructionType);
12091209

1210-
AbstractBool isDouble(AbstractValueDomain domain) =>
1211-
domain.isDouble(instructionType);
1212-
1213-
AbstractBool isDoubleOrNull(AbstractValueDomain domain) =>
1214-
domain.isDoubleOrNull(instructionType);
1215-
12161210
AbstractBool isBoolean(AbstractValueDomain domain) =>
12171211
domain.isBoolean(instructionType);
12181212

@@ -4223,19 +4217,6 @@ AbstractBool _typeTest(
42234217
return AbstractBool.False;
42244218
}
42254219

4226-
if (expression.isDouble(abstractValueDomain).isDefinitelyTrue) {
4227-
if (dartTypes.isSubtype(commonElements.doubleType, interface)) {
4228-
return AbstractBool.True;
4229-
}
4230-
if (interface == commonElements.intType) {
4231-
// We let the JS semantics decide for that check. Currently the code we
4232-
// emit will return true for a double that can be represented as a 31-bit
4233-
// integer and for -0.0.
4234-
return AbstractBool.Maybe;
4235-
}
4236-
return AbstractBool.False;
4237-
}
4238-
42394220
if (expression.isNumber(abstractValueDomain).isDefinitelyTrue) {
42404221
if (dartTypes.isSubtype(commonElements.numType, interface)) {
42414222
return AbstractBool.True;

‎pkg/compiler/lib/src/ssa/ssa_tracer.dart

-2
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,6 @@ class HInstructionStringifier implements HVisitor<String> {
154154
prefix = 'b';
155155
} else if (instruction.isInteger(_abstractValueDomain).isDefinitelyTrue) {
156156
prefix = 'i';
157-
} else if (instruction.isDouble(_abstractValueDomain).isDefinitelyTrue) {
158-
prefix = 'd';
159157
} else if (instruction.isNumber(_abstractValueDomain).isDefinitelyTrue) {
160158
prefix = 'n';
161159
} else if (_abstractValueDomain

‎pkg/compiler/lib/src/ssa/types_propagation.dart

-3
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,6 @@ class SsaTypePropagator extends HBaseVisitor implements OptimizationPhase {
138138
right.isInteger(abstractValueDomain).isDefinitelyTrue) {
139139
return abstractValueDomain.intType;
140140
}
141-
if (left.isDouble(abstractValueDomain).isDefinitelyTrue) {
142-
return abstractValueDomain.doubleType;
143-
}
144141
return abstractValueDomain.numType;
145142
}
146143

‎pkg/compiler/test/codegen/data/tdiv1.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ int foo5(bool param1, bool param2) {
8585
@pragma('dart2js:noInline')
8686
/*member: foo_regress_37502:function(param1, param2) {
8787
var a = param1 ? 1.2 : 12.3;
88-
return C.JSInt_methods.gcd$1(C.JSDouble_methods.$tdiv(a, param2 ? 3.14 : 2.81), 2);
88+
return C.JSInt_methods.gcd$1(C.JSNumber_methods.$tdiv(a, param2 ? 3.14 : 2.81), 2);
8989
}*/
9090
foo_regress_37502(param1, param2) {
9191
var a = param1 ? 1.2 : 12.3;

‎pkg/compiler/test/codegen/data_2/tdiv1.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,11 @@ int foo5(bool param1, bool param2) {
101101
@pragma('dart2js:noInline')
102102
/*spec.member: foo_regress_37502:function(param1, param2) {
103103
var a = H.boolConversionCheck(param1) ? 1.2 : 12.3;
104-
return C.JSInt_methods.gcd$1(C.JSDouble_methods.$tdiv(a, H.boolConversionCheck(param2) ? 3.14 : 2.81), 2);
104+
return C.JSInt_methods.gcd$1(C.JSNumber_methods.$tdiv(a, H.boolConversionCheck(param2) ? 3.14 : 2.81), 2);
105105
}*/
106106
/*prod.member: foo_regress_37502:function(param1, param2) {
107107
var a = param1 ? 1.2 : 12.3;
108-
return C.JSInt_methods.gcd$1(C.JSDouble_methods.$tdiv(a, param2 ? 3.14 : 2.81), 2);
108+
return C.JSInt_methods.gcd$1(C.JSNumber_methods.$tdiv(a, param2 ? 3.14 : 2.81), 2);
109109
}*/
110110
foo_regress_37502(param1, param2) {
111111
var a = param1 ? 1.2 : 12.3;

0 commit comments

Comments
 (0)