[CFE] Encapsulate int operations in separate classes for VM and JS

This refactoring achieves the following:

- Collect the integer operation semantics into one place instead of
having it sprinkled all over the constant evaluator with many
"if (targetingJavaScript)" tests.

- Avoid emitting the internal JavaScriptIntConstant node, which is
serialized as normal DoubleConstant nodes, thus achieving parity
between direct consumers and consumers seeing output that has been
serialized.

- Implement the JavaScript semantics for unsigned right shift.

- Fix truncating divide with a result outside int64 range incorrectly
clamping the result for JS targets.

- Fix positive hex constants >= 2^63 through int.fromEnvironment
producing negative values in dart2js.

- Clarify in Kernel documentation how numeric constants are represented
on VM vs JS targets.

Change-Id: If30bb2c2c77c54eff120b611b059c2ec726c99a0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116525
Reviewed-by: Mayank Patke <fishythefish@google.com>
This commit is contained in:
Aske Simon Christensen 2019-09-16 13:10:59 +00:00 committed by commit-bot@chromium.org
parent eb1a375749
commit 7799f424f4
6 changed files with 305 additions and 246 deletions

View file

@ -495,7 +495,19 @@ Future expectNotNull(String code, String expectedNotNull) async {
var actualNotNull = collector.notNullExpressions
// ConstantExpressions print the table offset - we want to compare
// against the underlying constant value instead.
.map((e) => (e is ConstantExpression ? e.constant : e).toString())
.map((e) {
if (e is ConstantExpression) {
Constant c = e.constant;
if (c is DoubleConstant &&
c.value.isFinite &&
c.value.truncateToDouble() == c.value) {
// Print integer values as integers
return BigInt.from(c.value).toString();
}
return c.toString();
}
return e.toString();
})
// Filter out our own NotNull annotations. The library prefix changes
// per test, so just filter on the suffix.
.where((s) => !s.endsWith('::_NotNull {}'))

View file

@ -60,11 +60,12 @@ import '../fasta_codes.dart'
templateConstEvalInvalidStringInterpolationOperand,
templateConstEvalInvalidSymbolName,
templateConstEvalKeyImplementsEqual,
templateConstEvalNegativeShift,
templateConstEvalNonConstantLiteral,
templateConstEvalNonConstantVariableGet,
templateConstEvalZeroDivisor;
import 'constant_int_folder.dart';
part 'constant_collection_builders.dart';
Component transformComponent(Component component, ConstantsBackend backend,
@ -116,22 +117,6 @@ void transformLibraries(
}
}
class JavaScriptIntConstant extends DoubleConstant {
final BigInt bigIntValue;
JavaScriptIntConstant(int value) : this.fromBigInt(new BigInt.from(value));
JavaScriptIntConstant.fromDouble(double value)
: bigIntValue = new BigInt.from(value),
super(value);
JavaScriptIntConstant.fromBigInt(this.bigIntValue)
: super(bigIntValue.toDouble());
JavaScriptIntConstant.fromUInt64(int value)
: this.fromBigInt(new BigInt.from(value).toUnsigned(64));
DartType getType(TypeEnvironment types) => types.intType;
String toString() => '$bigIntValue';
}
class ConstantsTransformer extends Transformer {
final ConstantsBackend backend;
final ConstantEvaluator constantEvaluator;
@ -499,6 +484,7 @@ class ConstantsTransformer extends Transformer {
class ConstantEvaluator extends RecursiveVisitor<Constant> {
final ConstantsBackend backend;
final NumberSemantics numberSemantics;
ConstantIntFolder intFolder;
Map<String, String> environmentDefines;
final bool errorOnUnevaluatedConstant;
final CoreTypes coreTypes;
@ -552,6 +538,7 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
"No 'environmentDefines' passed to the constant evaluator but the "
"ConstantsBackend does not support unevaluated constants.");
}
intFolder = new ConstantIntFolder.forSemantics(this, numberSemantics);
primitiveEqualCache = <Class, bool>{
coreTypes.boolClass: true,
coreTypes.doubleClass: false,
@ -795,14 +782,12 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
Constant visitIntLiteral(IntLiteral node) {
// The frontend ensures that integer literals are valid according to the
// target representation.
return targetingJavaScript
? canonicalize(new JavaScriptIntConstant.fromUInt64(node.value))
: canonicalize(new IntConstant(node.value));
return canonicalize(intFolder.makeIntConstant(node.value, unsigned: true));
}
@override
Constant visitDoubleLiteral(DoubleLiteral node) {
return canonicalize(makeDoubleConstant(node.value));
return canonicalize(new DoubleConstant(node.value));
}
@override
@ -824,14 +809,6 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
result = runInsideContext(constant.expression, () {
return _evaluateSubexpression(constant.expression);
});
} else if (targetingJavaScript) {
if (constant is DoubleConstant) {
double value = constant.value;
// TODO(askesc, fishythefish): Handle infinite integers.
if (value.isFinite && value.truncateToDouble() == value) {
result = new JavaScriptIntConstant.fromDouble(value);
}
}
}
// If there were already constants in the AST then we make sure we
// re-canonicalize them. After running the transformer we will therefore
@ -1330,112 +1307,14 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
other.getType(typeEnvironment)));
}
}
} else if (receiver is IntConstant || receiver is JavaScriptIntConstant) {
} else if (intFolder.isInt(receiver)) {
if (arguments.length == 0) {
switch (op) {
case 'unary-':
if (targetingJavaScript) {
BigInt value = (receiver as JavaScriptIntConstant).bigIntValue;
if (value == BigInt.zero) {
return canonicalize(new DoubleConstant(-0.0));
}
return canonicalize(new JavaScriptIntConstant.fromBigInt(-value));
}
int value = (receiver as IntConstant).value;
return canonicalize(new IntConstant(-value));
case '~':
if (targetingJavaScript) {
BigInt value = (receiver as JavaScriptIntConstant).bigIntValue;
return canonicalize(new JavaScriptIntConstant.fromBigInt(
(~value).toUnsigned(32)));
}
int value = (receiver as IntConstant).value;
return canonicalize(new IntConstant(~value));
}
return canonicalize(intFolder.foldUnaryOperator(node, op, receiver));
} else if (arguments.length == 1) {
final Constant other = arguments[0];
if (other is IntConstant || other is JavaScriptIntConstant) {
if ((op == '<<' || op == '>>' || op == '>>>')) {
Object receiverValue = receiver is IntConstant
? receiver.value
: (receiver as JavaScriptIntConstant).bigIntValue;
int otherValue = other is IntConstant
? other.value
: (other as JavaScriptIntConstant).bigIntValue.toInt();
if (otherValue < 0) {
return report(
node.arguments.positional.first,
// TODO(askesc): Change argument types in template to
// constants.
templateConstEvalNegativeShift.withArguments(
op, '${receiverValue}', '${otherValue}'));
}
}
if ((op == '%' || op == '~/')) {
Object receiverValue = receiver is IntConstant
? receiver.value
: (receiver as JavaScriptIntConstant).bigIntValue;
int otherValue = other is IntConstant
? other.value
: (other as JavaScriptIntConstant).bigIntValue.toInt();
if (otherValue == 0) {
return report(
node.arguments.positional.first,
// TODO(askesc): Change argument type in template to constant.
templateConstEvalZeroDivisor.withArguments(
op, '${receiverValue}'));
}
}
switch (op) {
case '|':
case '&':
case '^':
int receiverValue = receiver is IntConstant
? receiver.value
: (receiver as JavaScriptIntConstant)
.bigIntValue
.toUnsigned(32)
.toInt();
int otherValue = other is IntConstant
? other.value
: (other as JavaScriptIntConstant)
.bigIntValue
.toUnsigned(32)
.toInt();
return evaluateBinaryBitOperation(
op, receiverValue, otherValue, node);
case '<<':
case '>>':
case '>>>':
bool negative = false;
int receiverValue;
if (receiver is IntConstant) {
receiverValue = receiver.value;
} else {
BigInt bigIntValue =
(receiver as JavaScriptIntConstant).bigIntValue;
receiverValue = bigIntValue.toUnsigned(32).toInt();
negative = bigIntValue.isNegative;
}
int otherValue = other is IntConstant
? other.value
: (other as JavaScriptIntConstant).bigIntValue.toInt();
return evaluateBinaryShiftOperation(
op, receiverValue, otherValue, node,
negativeReceiver: negative);
default:
num receiverValue = receiver is IntConstant
? receiver.value
: (receiver as DoubleConstant).value;
num otherValue = other is IntConstant
? other.value
: (other as DoubleConstant).value;
return evaluateBinaryNumericOperation(
op, receiverValue, otherValue, node);
}
if (intFolder.isInt(other)) {
return canonicalize(
intFolder.foldBinaryOperator(node, op, receiver, other));
} else if (other is DoubleConstant) {
if ((op == '|' || op == '&' || op == '^') ||
(op == '<<' || op == '>>' || op == '>>>')) {
@ -1447,11 +1326,9 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
typeEnvironment.intType,
other.getType(typeEnvironment)));
}
num receiverValue = receiver is IntConstant
? receiver.value
: (receiver as DoubleConstant).value;
return evaluateBinaryNumericOperation(
op, receiverValue, other.value, node);
num receiverValue = (receiver as PrimitiveConstant<num>).value;
return canonicalize(evaluateBinaryNumericOperation(
op, receiverValue, other.value, node));
}
return report(
node,
@ -1475,17 +1352,15 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
if (arguments.length == 0) {
switch (op) {
case 'unary-':
return canonicalize(makeDoubleConstant(-receiver.value));
return canonicalize(new DoubleConstant(-receiver.value));
}
} else if (arguments.length == 1) {
final Constant other = arguments[0];
if (other is IntConstant || other is DoubleConstant) {
final num value = (other is IntConstant)
? other.value
: (other as DoubleConstant).value;
return evaluateBinaryNumericOperation(
op, receiver.value, value, node);
final num value = (other as PrimitiveConstant<num>).value;
return canonicalize(
evaluateBinaryNumericOperation(op, receiver.value, value, node));
}
return report(
node,
@ -1627,10 +1502,7 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
final Constant receiver = _evaluateSubexpression(node.receiver);
if (receiver is StringConstant && node.name.name == 'length') {
if (targetingJavaScript) {
return canonicalize(new JavaScriptIntConstant(receiver.value.length));
}
return canonicalize(new IntConstant(receiver.value.length));
return canonicalize(intFolder.makeIntConstant(receiver.value.length));
} else if (shouldBeUnevaluated) {
return unevaluated(node,
new PropertyGet(extract(receiver), node.name, node.interfaceTarget));
@ -1708,7 +1580,12 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
for (int i = 0; i < node.expressions.length; i++) {
Constant constant = _evaluateSubexpression(node.expressions[i]);
if (constant is PrimitiveConstant<Object>) {
String value = constant.toString();
String value;
if (constant is DoubleConstant && intFolder.isInt(constant)) {
value = new BigInt.from(constant.value).toString();
} else {
value = constant.toString();
}
Object last = concatenated.last;
if (last is StringBuffer) {
last.write(value);
@ -1784,16 +1661,17 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
return boolConstant;
} else if (target.enclosingClass == coreTypes.intClass) {
int intValue = value != null ? int.tryParse(value) : null;
intValue ??= defaultValue is IntConstant
? defaultValue.value
: defaultValue is JavaScriptIntConstant
? defaultValue.bigIntValue.toInt()
: null;
if (intValue == null) return nullConstant;
if (targetingJavaScript) {
return canonicalize(new JavaScriptIntConstant(intValue));
Constant intConstant;
if (intValue != null) {
bool negated = value.startsWith('-');
intConstant =
intFolder.makeIntConstant(intValue, unsigned: !negated);
} else if (intFolder.isInt(defaultValue)) {
intConstant = defaultValue;
} else {
intConstant = nullConstant;
}
return canonicalize(new IntConstant(intValue));
return canonicalize(intConstant);
} else if (target.enclosingClass == coreTypes.stringClass) {
value ??=
defaultValue is StringConstant ? defaultValue.value : null;
@ -1945,8 +1823,7 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
}
bool hasPrimitiveEqual(Constant constant) {
// TODO(askesc, fishythefish): Make sure the correct class is inferred
// when we clean up JavaScript int constant handling.
if (intFolder.isInt(constant)) return true;
DartType type = constant.getType(typeEnvironment);
return !(type is InterfaceType && !classHasPrimitiveEqual(type.classNode));
}
@ -1970,18 +1847,6 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
BoolConstant makeBoolConstant(bool value) =>
value ? trueConstant : falseConstant;
DoubleConstant makeDoubleConstant(double value) {
if (targetingJavaScript) {
// Convert to an integer when possible (matching the runtime behavior
// of `is int`).
if (value.isFinite && !identical(value, -0.0)) {
int i = value.toInt();
if (value == i.toDouble()) return new JavaScriptIntConstant(i);
}
}
return new DoubleConstant(value);
}
bool isSubtype(Constant constant, DartType type) {
DartType constantType = constant.getType(typeEnvironment);
if (targetingJavaScript) {
@ -2095,88 +1960,26 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
}
}
Constant evaluateBinaryBitOperation(String op, int a, int b, TreeNode node) {
int result;
switch (op) {
case '|':
result = a | b;
break;
case '&':
result = a & b;
break;
case '^':
result = a ^ b;
break;
}
if (targetingJavaScript) {
return canonicalize(new JavaScriptIntConstant(result));
}
return canonicalize(new IntConstant(result));
}
Constant evaluateBinaryShiftOperation(String op, int a, int b, TreeNode node,
{negativeReceiver: false}) {
int result;
switch (op) {
case '<<':
result = a << b;
break;
case '>>':
if (targetingJavaScript) {
if (negativeReceiver) {
const int signBit = 0x80000000;
a -= (a & signBit) << 1;
}
result = a >> b;
} else {
result = a >> b;
}
break;
case '>>>':
// TODO(fishythefish): Implement JS semantics for `>>>`.
result = b >= 64 ? 0 : (a >> b) & ((1 << (64 - b)) - 1);
break;
}
if (targetingJavaScript) {
return canonicalize(new JavaScriptIntConstant(result.toUnsigned(32)));
}
return canonicalize(new IntConstant(result));
}
/// Binary operation between two operands, at least one of which is a double.
Constant evaluateBinaryNumericOperation(
String op, num a, num b, TreeNode node) {
num result;
switch (op) {
case '+':
result = a + b;
break;
return new DoubleConstant(a + b);
case '-':
result = a - b;
break;
return new DoubleConstant(a - b);
case '*':
result = a * b;
break;
return new DoubleConstant(a * b);
case '/':
result = a / b;
break;
return new DoubleConstant(a / b);
case '~/':
result = a ~/ b;
break;
if (b == 0) {
return report(
node, templateConstEvalZeroDivisor.withArguments(op, '$a'));
}
return intFolder.truncatingDivide(a, b);
case '%':
result = a % b;
break;
}
if (result is int) {
if (targetingJavaScript) {
return canonicalize(new JavaScriptIntConstant(result));
}
return canonicalize(new IntConstant(result.toSigned(64)));
}
if (result is double) {
return canonicalize(makeDoubleConstant(result));
return new DoubleConstant(a % b);
}
switch (op) {

View file

@ -0,0 +1,238 @@
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'dart:core' hide MapEntry;
import 'package:kernel/ast.dart';
import 'package:kernel/target/targets.dart';
import 'constant_evaluator.dart';
import '../fasta_codes.dart'
show
templateConstEvalInvalidMethodInvocation,
templateConstEvalNegativeShift,
templateConstEvalZeroDivisor;
abstract class ConstantIntFolder {
final ConstantEvaluator evaluator;
ConstantIntFolder(this.evaluator);
factory ConstantIntFolder.forSemantics(
ConstantEvaluator evaluator, NumberSemantics semantics) {
if (semantics == NumberSemantics.js) {
return new JsConstantIntFolder(evaluator);
} else {
return new VmConstantIntFolder(evaluator);
}
}
bool isInt(Constant constant);
Constant makeIntConstant(int value, {bool unsigned: false});
Constant foldUnaryOperator(
MethodInvocation node, String op, covariant Constant operand);
Constant foldBinaryOperator(MethodInvocation node, String op,
covariant Constant left, covariant Constant right);
Constant truncatingDivide(num left, num right);
void _checkOperands(MethodInvocation node, String op, num left, num right) {
if ((op == '<<' || op == '>>' || op == '>>>') && right < 0) {
evaluator.report(node,
templateConstEvalNegativeShift.withArguments(op, '$left', '$right'));
}
if ((op == '%' || op == '~/') && right == 0) {
evaluator.report(
node, templateConstEvalZeroDivisor.withArguments(op, '$left'));
}
}
}
class VmConstantIntFolder extends ConstantIntFolder {
VmConstantIntFolder(ConstantEvaluator evaluator) : super(evaluator);
@override
bool isInt(Constant constant) => constant is IntConstant;
@override
IntConstant makeIntConstant(int value, {bool unsigned: false}) {
return new IntConstant(value);
}
@override
Constant foldUnaryOperator(
MethodInvocation node, String op, IntConstant operand) {
switch (op) {
case 'unary-':
return new IntConstant(-operand.value);
case '~':
return new IntConstant(~operand.value);
default:
return evaluator.report(
node,
templateConstEvalInvalidMethodInvocation.withArguments(
op, operand));
}
}
@override
Constant foldBinaryOperator(
MethodInvocation node, String op, IntConstant left, IntConstant right) {
int a = left.value;
int b = right.value;
_checkOperands(node, op, a, b);
switch (op) {
case '+':
return new IntConstant(a + b);
case '-':
return new IntConstant(a - b);
case '*':
return new IntConstant(a * b);
case '/':
return new DoubleConstant(a / b);
case '~/':
return new IntConstant(a ~/ b);
case '%':
return new IntConstant(a % b);
case '|':
return new IntConstant(a | b);
case '&':
return new IntConstant(a & b);
case '^':
return new IntConstant(a ^ b);
case '<<':
return new IntConstant(a << b);
case '>>':
return new IntConstant(a >> b);
case '>>>':
int result = b >= 64 ? 0 : (a >> b) & ((1 << (64 - b)) - 1);
return new IntConstant(result);
case '<':
return evaluator.makeBoolConstant(a < b);
case '<=':
return evaluator.makeBoolConstant(a <= b);
case '>=':
return evaluator.makeBoolConstant(a >= b);
case '>':
return evaluator.makeBoolConstant(a > b);
default:
return evaluator.report(node,
templateConstEvalInvalidMethodInvocation.withArguments(op, left));
}
}
@override
Constant truncatingDivide(num left, num right) {
return new IntConstant(left ~/ right);
}
}
class JsConstantIntFolder extends ConstantIntFolder {
JsConstantIntFolder(ConstantEvaluator evaluator) : super(evaluator);
static bool _valueIsInteger(double value) {
return value.isFinite && value.truncateToDouble() == value;
}
static int _truncate32(int value) => value & 0xFFFFFFFF;
static int _toUint32(double value) {
return new BigInt.from(value).toUnsigned(32).toInt();
}
@override
bool isInt(Constant constant) {
return constant is DoubleConstant && _valueIsInteger(constant.value);
}
@override
DoubleConstant makeIntConstant(int value, {bool unsigned: false}) {
double doubleValue = value.toDouble();
assert(doubleValue.toInt() == value);
if (unsigned) {
const double twoTo64 = 18446744073709551616.0;
if (value < 0) doubleValue += twoTo64;
}
return new DoubleConstant(doubleValue);
}
@override
Constant foldUnaryOperator(
MethodInvocation node, String op, DoubleConstant operand) {
switch (op) {
case 'unary-':
return new DoubleConstant(-operand.value);
case '~':
int intValue = _toUint32(operand.value);
return new DoubleConstant(_truncate32(~intValue).toDouble());
default:
return evaluator.report(
node,
templateConstEvalInvalidMethodInvocation.withArguments(
op, operand));
}
}
@override
Constant foldBinaryOperator(MethodInvocation node, String op,
DoubleConstant left, DoubleConstant right) {
double a = left.value;
double b = right.value;
_checkOperands(node, op, a, b);
switch (op) {
case '+':
return new DoubleConstant(a + b);
case '-':
return new DoubleConstant(a - b);
case '*':
return new DoubleConstant(a * b);
case '/':
return new DoubleConstant(a / b);
case '~/':
return truncatingDivide(a, b);
case '%':
return new DoubleConstant(a % b);
case '|':
return new DoubleConstant((_toUint32(a) | _toUint32(b)).toDouble());
case '&':
return new DoubleConstant((_toUint32(a) & _toUint32(b)).toDouble());
case '^':
return new DoubleConstant((_toUint32(a) ^ _toUint32(b)).toDouble());
case '<<':
int ai = _toUint32(a);
return new DoubleConstant(_truncate32(ai << b.toInt()).toDouble());
case '>>':
int ai = _toUint32(a);
if (a < 0) {
const int signBit = 0x80000000;
ai -= (ai & signBit) << 1;
}
return new DoubleConstant(_truncate32(ai >> b.toInt()).toDouble());
case '>>>':
int ai = _toUint32(a);
return new DoubleConstant(_truncate32(ai >> b.toInt()).toDouble());
case '<':
return evaluator.makeBoolConstant(a < b);
case '<=':
return evaluator.makeBoolConstant(a <= b);
case '>=':
return evaluator.makeBoolConstant(a >= b);
case '>':
return evaluator.makeBoolConstant(a > b);
default:
return evaluator.report(node,
templateConstEvalInvalidMethodInvocation.withArguments(op, left));
}
}
@override
Constant truncatingDivide(num left, num right) {
double result = (left / right).truncateToDouble();
return new DoubleConstant(result == 0.0 ? 0.0 : result);
}
}

View file

@ -304,6 +304,7 @@ ff
fff
ffff
fffff
ffffffff
ffi
file's
filenames

View file

@ -6076,6 +6076,7 @@ class BoolConstant extends PrimitiveConstant<bool> {
DartType getType(TypeEnvironment types) => types.boolType;
}
/// An integer constant on a non-JS target.
class IntConstant extends PrimitiveConstant<int> {
IntConstant(int value) : super(value);
@ -6086,6 +6087,7 @@ class IntConstant extends PrimitiveConstant<int> {
DartType getType(TypeEnvironment types) => types.intType;
}
/// A double constant on a non-JS target or any numeric constant on a JS target.
class DoubleConstant extends PrimitiveConstant<double> {
DoubleConstant(double value) : super(value);

View file

@ -1,7 +1,7 @@
// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// SharedOptions=-Da=1 -Db=-12 -Dc=0x123 -Dd=-0x1234 -De=+0x112296 -Df=-9007199254740991 -Dg=9007199254740991
// SharedOptions=-Da=1 -Db=-12 -Dc=0x123 -Dd=-0x1234 -De=+0x112296 -Df=-9007199254740991 -Dg=9007199254740991 -Dh=-0x8000000000000000 -Di=0x8000000000000000 -Dj=0xDEADBEEFCAFE0000
import "package:expect/expect.dart";
@ -13,4 +13,7 @@ main() {
Expect.equals(0x112296, const int.fromEnvironment('e'));
Expect.equals(-9007199254740991, const int.fromEnvironment('f'));
Expect.equals(9007199254740991, const int.fromEnvironment('g'));
Expect.equals(-0x8000000000000000, const int.fromEnvironment('h'));
Expect.equals(0x8000000000000000, const int.fromEnvironment('i'));
Expect.equals(0xDEADBEEFCAFE0000, const int.fromEnvironment('j'));
}