[dart2js] Improve null receiver guard removal near JS code

Null receiver guards (which look like "t1.toString;") are removed in more cases
where the following JS fragment would throw a TypeError on the same value.

Change-Id: I3872f00c90432077199542f4485b8e991f82fa21
Reviewed-on: https://dart-review.googlesource.com/c/89765
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
This commit is contained in:
Stephen Adams 2019-01-16 23:59:28 +00:00 committed by commit-bot@chromium.org
parent c3599a9d8c
commit 9cdce03e16
5 changed files with 136 additions and 109 deletions

View file

@ -38,15 +38,15 @@ class SpecialType {
}
/// Description of the exception behaviour of native code.
///
/// TODO(sra): Replace with something that better supports specialization on
/// first argument properties.
class NativeThrowBehavior {
static const NativeThrowBehavior NEVER = const NativeThrowBehavior._(0);
static const NativeThrowBehavior MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS =
const NativeThrowBehavior._(1);
static const NativeThrowBehavior MAY = const NativeThrowBehavior._(2);
static const NativeThrowBehavior MUST = const NativeThrowBehavior._(3);
static const NativeThrowBehavior NEVER = NativeThrowBehavior._(0);
static const NativeThrowBehavior MAY = NativeThrowBehavior._(1);
/// Throws only if first argument is null.
static const NativeThrowBehavior NULL_NSM = NativeThrowBehavior._(2);
/// Throws if first argument is null, then may throw.
static const NativeThrowBehavior NULL_NSM_THEN_MAY = NativeThrowBehavior._(3);
final int _bits;
const NativeThrowBehavior._(this._bits);
@ -55,43 +55,73 @@ class NativeThrowBehavior {
/// Does this behavior always throw a noSuchMethod check on a null first
/// argument before any side effect or other exception?
// TODO(sra): Extend NativeThrowBehavior with the concept of NSM guard
// followed by other potential behavior.
bool get isNullNSMGuard => this == MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS;
bool get isNullNSMGuard => this == NULL_NSM || this == NULL_NSM_THEN_MAY;
/// Does this behavior always act as a null noSuchMethod check, and has no
/// other throwing behavior?
bool get isOnlyNullNSMGuard =>
this == MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS;
bool get isOnlyNullNSMGuard => this == NULL_NSM;
/// Returns the behavior if we assume the first argument is not null.
NativeThrowBehavior get onNonNull {
if (this == MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS) return NEVER;
if (this == NULL_NSM) return NEVER;
if (this == NULL_NSM_THEN_MAY) return MAY;
return this;
}
String toString() {
if (this == NEVER) return 'never';
if (this == MAY) return 'may';
if (this == MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS) return 'null(1)';
if (this == MUST) return 'must';
if (this == NULL_NSM) return 'null(1)';
if (this == NULL_NSM_THEN_MAY) return 'null(1)+may';
return 'NativeThrowBehavior($_bits)';
}
/// Canonical list of marker values.
///
/// Added to make [NativeThrowBehavior] enum-like.
static const List<NativeThrowBehavior> values = const <NativeThrowBehavior>[
static const List<NativeThrowBehavior> values = [
NEVER,
MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS,
MAY,
MUST,
NULL_NSM,
NULL_NSM_THEN_MAY,
];
/// Index to this marker within [values].
///
/// Added to make [NativeThrowBehavior] enum-like.
int get index => values.indexOf(this);
/// Deserializer helper.
static NativeThrowBehavior _bitsToValue(int bits) {
switch (bits) {
case 0:
return NEVER;
case 1:
return MAY;
case 2:
return NULL_NSM;
case 3:
return NULL_NSM_THEN_MAY;
default:
return null;
}
}
/// Sequence operator.
NativeThrowBehavior then(NativeThrowBehavior second) {
if (this == NEVER) return second;
if (this == MAY) return MAY;
if (this == NULL_NSM_THEN_MAY) return NULL_NSM_THEN_MAY;
assert(this == NULL_NSM);
if (second == NEVER) return this;
return NULL_NSM_THEN_MAY;
}
/// Choice operator.
NativeThrowBehavior or(NativeThrowBehavior other) {
if (this == other) return this;
return MAY;
}
}
/// A summary of the behavior of a native element.
@ -190,21 +220,8 @@ class NativeBehavior {
behavior.codeTemplateText = codeTemplateText;
behavior.codeTemplate = js.js.parseForeignJS(codeTemplateText);
}
switch (throwBehavior) {
case 0:
behavior.throwBehavior = NativeThrowBehavior.NEVER;
break;
case 1:
behavior.throwBehavior =
NativeThrowBehavior.MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS;
break;
case 2:
behavior.throwBehavior = NativeThrowBehavior.MAY;
break;
case 3:
behavior.throwBehavior = NativeThrowBehavior.MUST;
break;
}
behavior.throwBehavior = NativeThrowBehavior._bitsToValue(throwBehavior);
assert(behavior.throwBehavior._bits == throwBehavior);
behavior.isAllocation = isAllocation;
behavior.useGvn = useGvn;
return behavior;
@ -319,12 +336,12 @@ class NativeBehavior {
/// indicated with 'no-static'. The flags 'effects' and 'depends' must be
/// used in unison (either both are present or none is).
///
/// The <throws-string> values are 'never', 'may', 'must', and 'null(1)'.
/// The default if unspecified is 'may'. 'null(1)' means that the template
/// expression throws if and only if the first template parameter is `null`
/// or `undefined`.
/// TODO(sra): Can we simplify to must/may/never and add null(1) by
/// inspection as an orthogonal attribute?
/// The <throws-string> values are 'never', 'may', 'null(1)', and
/// 'null(1)+may'. The default if unspecified is 'may'. 'null(1)' means
/// that the template expression throws if and only if the first template
/// parameter is `null` or `undefined`, and 'null(1)+may' throws if the
/// first argument is `null` / `undefined`, and then may throw for other
/// reasons.
///
/// <gvn-string> values are 'true' and 'false'. The default if unspecified
/// is 'false'.
@ -478,14 +495,14 @@ class NativeBehavior {
});
}
const throwsOption = const <String, NativeThrowBehavior>{
const throwsOption = <String, NativeThrowBehavior>{
'never': NativeThrowBehavior.NEVER,
'null(1)': NativeThrowBehavior.MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS,
'may': NativeThrowBehavior.MAY,
'must': NativeThrowBehavior.MUST
'null(1)': NativeThrowBehavior.NULL_NSM,
'null(1)+may': NativeThrowBehavior.NULL_NSM_THEN_MAY,
};
const boolOptions = const <String, bool>{'true': true, 'false': false};
const boolOptions = <String, bool>{'true': true, 'false': false};
SideEffects sideEffects =
processEffects(reportError, values['effects'], values['depends']);

View file

@ -131,26 +131,18 @@ class ThrowBehaviorVisitor extends js.BaseVisitor<NativeThrowBehavior> {
return visit(node);
}
// TODO(sra): Add [sequence] functionality to NativeThrowBehavior.
/// Returns the combined behavior of sequential execution of code having
/// behavior [first] followed by code having behavior [second].
static NativeThrowBehavior sequence(
NativeThrowBehavior first, NativeThrowBehavior second) {
if (first == NativeThrowBehavior.MUST) return first;
if (second == NativeThrowBehavior.MUST) return second;
if (second == NativeThrowBehavior.NEVER) return first;
if (first == NativeThrowBehavior.NEVER) return second;
// Both are one of MAY or MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS.
return NativeThrowBehavior.MAY;
return first.then(second);
}
// TODO(sra): Add [choice] functionality to NativeThrowBehavior.
/// Returns the combined behavior of a choice between two paths with behaviors
/// [first] and [second].
static NativeThrowBehavior choice(
NativeThrowBehavior first, NativeThrowBehavior second) {
if (first == second) return first; // Both paths have same behaviour.
return NativeThrowBehavior.MAY;
return first.or(second);
}
NativeThrowBehavior visit(js.Node node) {
@ -173,12 +165,16 @@ class ThrowBehaviorVisitor extends js.BaseVisitor<NativeThrowBehavior> {
return NativeThrowBehavior.NEVER;
}
NativeThrowBehavior visitArrayInitializer(js.ArrayInitializer node) {
return node.elements.map(visit).fold(NativeThrowBehavior.NEVER, sequence);
}
NativeThrowBehavior visitArrayHole(js.ArrayHole node) {
return NativeThrowBehavior.NEVER;
}
NativeThrowBehavior visitObjectInitializer(js.ObjectInitializer node) {
NativeThrowBehavior result = NativeThrowBehavior.NEVER;
for (js.Property property in node.properties) {
result = sequence(result, visit(property));
}
return result;
return node.properties.map(visit).fold(NativeThrowBehavior.NEVER, sequence);
}
NativeThrowBehavior visitProperty(js.Property node) {
@ -191,6 +187,17 @@ class ThrowBehaviorVisitor extends js.BaseVisitor<NativeThrowBehavior> {
}
NativeThrowBehavior visitCall(js.Call node) {
js.Expression target = node.target;
if (target is js.PropertyAccess && _isFirstInterpolatedProperty(target)) {
// #.f(...): Evaluate selector 'f', dereference, evaluate arguments, and
// finally call target.
NativeThrowBehavior result =
sequence(visit(target.selector), NativeThrowBehavior.NULL_NSM);
for (js.Expression argument in node.arguments) {
result = sequence(result, visit(argument));
}
return sequence(result, NativeThrowBehavior.MAY); // Target may throw.
}
return NativeThrowBehavior.MAY;
}
@ -242,7 +249,7 @@ class ThrowBehaviorVisitor extends js.BaseVisitor<NativeThrowBehavior> {
}
NativeThrowBehavior visitThrow(js.Throw node) {
return NativeThrowBehavior.MUST;
return sequence(visit(node.expression), NativeThrowBehavior.MAY);
}
NativeThrowBehavior visitPrefix(js.Prefix node) {
@ -269,6 +276,7 @@ class ThrowBehaviorVisitor extends js.BaseVisitor<NativeThrowBehavior> {
// global names are almost certainly not reference errors, e.g 'Array'.
switch (node.name) {
case 'Array':
case 'Math':
case 'Object':
return NativeThrowBehavior.NEVER;
default:
@ -277,20 +285,26 @@ class ThrowBehaviorVisitor extends js.BaseVisitor<NativeThrowBehavior> {
}
NativeThrowBehavior visitAccess(js.PropertyAccess node) {
// TODO(sra): We need a representation where the nsm guard behaviour is
// maintained when combined with other throwing behaviour.
js.Node receiver = node.receiver;
NativeThrowBehavior first = visit(receiver);
NativeThrowBehavior second = visit(node.selector);
if (receiver is js.InterpolatedExpression &&
receiver.isPositional &&
receiver.nameOrPosition == 0) {
first = NativeThrowBehavior.MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS;
if (_isFirstInterpolatedProperty(node)) {
first = NativeThrowBehavior.NULL_NSM;
} else {
first = NativeThrowBehavior.MAY;
}
return sequence(first, second);
}
bool _isFirstInterpolatedProperty(js.PropertyAccess node) {
js.Node receiver = node.receiver;
if (receiver is js.InterpolatedExpression &&
receiver.isPositional &&
receiver.nameOrPosition == 0) {
return true;
}
return false;
}
}

View file

@ -14,7 +14,7 @@ import '../elements/entities.dart';
import '../elements/types.dart';
import '../inferrer/abstract_value_domain.dart';
import '../inferrer/types.dart';
import '../js/js.dart' as js;
//import '../js/js.dart' as js;
import '../js_backend/allocator_analysis.dart' show JAllocatorAnalysis;
import '../js_backend/backend.dart';
import '../js_backend/native_data.dart' show NativeData;
@ -1846,31 +1846,6 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase {
if (foreign.inputs.length < 1) return false;
if (foreign.inputs.first != receiver) return false;
if (foreign.throwBehavior.isNullNSMGuard) return true;
// TODO(sra): Fix NativeThrowBehavior to distinguish MAY from
// throws-nsm-on-null-followed-by-MAY and remove all the code below.
// We look for a template of the form
//
// #.something -or- #.something()
//
// where # is substituted by receiver.
js.Template template = foreign.codeTemplate;
js.Node node = template.ast;
// #.something = ...
if (node is js.Assignment) {
js.Assignment assignment = node;
node = assignment.leftHandSide;
}
// #.something
if (node is js.PropertyAccess) {
js.PropertyAccess access = node;
if (access.receiver is js.InterpolatedExpression) {
js.InterpolatedExpression hole = access.receiver;
return hole.isPositional && hole.nameOrPosition == 0;
}
}
return false;
}
@ -1949,14 +1924,18 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase {
if (!instruction.usedBy.isEmpty) return false;
if (isTrivialDeadStore(instruction)) return true;
if (instruction.sideEffects.hasSideEffects()) return false;
if (instruction.canThrow(_abstractValueDomain) &&
instruction.onlyThrowsNSM() &&
hasFollowingThrowingNSM(instruction)) {
return true;
if (instruction.canThrow(_abstractValueDomain)) {
if (instruction.onlyThrowsNSM() && hasFollowingThrowingNSM(instruction)) {
// [instruction] is a null reciever guard that is followed by an
// instruction that fails the same way (by accessing a property of
// `null` or `undefined`).
return true;
}
return false;
}
return !instruction.canThrow(_abstractValueDomain) &&
instruction is! HParameterValue &&
instruction is! HLocalSet;
if (instruction is HParameterValue) return false;
if (instruction is HLocalSet) return false;
return true;
}
void visitGraph(HGraph graph) {

View file

@ -270,12 +270,11 @@ void main() {
testWithSideEffects(' returns:A|B|C; creates:A; ',
returns: ['A', 'B', 'C'], creates: ['A']);
test('throws:must', expectedThrows: NativeThrowBehavior.MUST);
test('throws:may', expectedThrows: NativeThrowBehavior.MAY);
test('throws:never', expectedThrows: NativeThrowBehavior.NEVER);
test('throws:null(1)',
expectedThrows:
NativeThrowBehavior.MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS);
test('throws:null(1)', expectedThrows: NativeThrowBehavior.NULL_NSM);
test('throws:null(1)+may',
expectedThrows: NativeThrowBehavior.NULL_NSM_THEN_MAY);
test('new:true', expectedNew: true);
test('new:false', expectedNew: false);

View file

@ -16,9 +16,9 @@ void test(String source, NativeThrowBehavior expectedThrowBehavior) {
void main() {
final MAY = NativeThrowBehavior.MAY;
final MUST = NativeThrowBehavior.MUST;
final NEVER = NativeThrowBehavior.NEVER;
final NULL_NSM = NativeThrowBehavior.MAY_THROW_ONLY_ON_FIRST_ARGUMENT_ACCESS;
final NULL_NSM = NativeThrowBehavior.NULL_NSM;
final NULL_NSM_THEN_MAY = NativeThrowBehavior.NULL_NSM_THEN_MAY;
test('0', NEVER);
test('void 0', NEVER);
@ -86,6 +86,7 @@ void main() {
test('console', MAY);
test('Array', NEVER);
test('Math', NEVER);
test('Object', NEVER);
test('typeof #', NEVER);
@ -93,8 +94,25 @@ void main() {
test('typeof foo.#', MAY);
test('typeof #.foo', NULL_NSM);
test('throw 123', MUST);
test('throw #', MUST);
test('throw #.x', MUST); // Could be better: is also an NSM guard.
test('throw #.x = 123', MUST);
test('throw 123', MAY);
test('throw #', MAY);
test('throw #.x', NULL_NSM_THEN_MAY);
test('throw #.x = 123', MAY);
test('#.f()', NULL_NSM_THEN_MAY);
test('#.f(#, #)', NULL_NSM_THEN_MAY);
test('#[#](#, #)', NULL_NSM_THEN_MAY);
test('#[f()](#, #)', MAY); // f() evaluated before
test('[]', NEVER);
test('[,,6,,,]', NEVER);
test('[#.f()]', NULL_NSM_THEN_MAY);
test('[,,#.f(),,f(),,]', NULL_NSM_THEN_MAY);
test('[,,f(),,#.f(),,]', MAY);
test('{}', NEVER);
test('{one: 1}', NEVER);
test('{one: #.f()}', NULL_NSM_THEN_MAY);
test('{one: #.f(), two: f()}', NULL_NSM_THEN_MAY);
test('{one: f(), two: #.f()}', MAY);
}