Skip to content

Commit

Permalink
Fix type inference of compound assignments when getter/setter have di…
Browse files Browse the repository at this point in the history
…fferent types.

For compound assignments, we need to look up the "combiner" operation
in the type of the getter (not the setter), since the receiver of the
combiner operation is the value that was read.  This is necessary for
soundness.

Change-Id: I02e0e93310b6b6b94d4a6253d4cf2fc1d3c69cd5
Reviewed-on: https://dart-review.googlesource.com/16607
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
  • Loading branch information
stereotype441 authored and commit-bot@chromium.org committed Oct 25, 2017
1 parent 39bfbfe commit cded101
Show file tree
Hide file tree
Showing 34 changed files with 946 additions and 261 deletions.
66 changes: 32 additions & 34 deletions pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart
Original file line number Diff line number Diff line change
Expand Up @@ -410,61 +410,63 @@ abstract class ShadowComplexAssignment extends ShadowSyntheticExpression {

DartType _inferRhs(
ShadowTypeInferrer inferrer, DartType readType, DartType writeContext) {
DartType inferredType = writeContext ?? const DynamicType();
DartType combinedType;
if (nullAwareCombiner != null) {
var rhsType = inferrer.inferExpression(rhs, writeContext, true);
_storeLetType(inferrer, rhs, rhsType);
MethodInvocation equalsInvocation = nullAwareCombiner.condition;
inferrer.findMethodInvocationMember(writeContext, equalsInvocation,
silent: true);
var combinedType = inferrer.typeSchemaEnvironment
.getLeastUpperBound(inferredType, rhsType);
// Note: the case of readType=null only happens for erroneous code.
combinedType = readType == null
? rhsType
: inferrer.typeSchemaEnvironment
.getLeastUpperBound(readType, rhsType);
if (inferrer.strongMode) {
nullAwareCombiner.staticType = combinedType;
}
return combinedType;
} else if (combiner != null) {
bool isOverloadedArithmeticOperator = false;
var combinerMember = inferrer
.findMethodInvocationMember(writeContext, combiner, silent: true);
var combinerMember =
inferrer.findMethodInvocationMember(readType, combiner, silent: true);
if (combinerMember is Procedure) {
isOverloadedArithmeticOperator = inferrer.typeSchemaEnvironment
.isOverloadedArithmeticOperatorAndType(
combinerMember, writeContext);
.isOverloadedArithmeticOperatorAndType(combinerMember, readType);
}
DartType combinedType;
DartType rhsType;
var combinerType =
inferrer.getCalleeFunctionType(combinerMember, writeContext, false);
if (isPostIncDec) {
combinedType = inferredType;
if (isPreIncDec || isPostIncDec) {
rhsType = inferrer.coreTypes.intClass.rawType;
} else {
DartType rhsType;
if (isPreIncDec) {
rhsType = inferrer.coreTypes.intClass.rawType;
} else {
// Analyzer uses a null context for the RHS here.
// TODO(paulberry): improve on this.
rhsType = inferrer.inferExpression(rhs, null, true);
_storeLetType(inferrer, rhs, rhsType);
}
if (isOverloadedArithmeticOperator) {
combinedType = inferrer.typeSchemaEnvironment
.getTypeOfOverloadedArithmetic(inferredType, rhsType);
} else {
combinedType = combinerType.returnType;
}
// Analyzer uses a null context for the RHS here.
// TODO(paulberry): improve on this.
rhsType = inferrer.inferExpression(rhs, null, true);
_storeLetType(inferrer, rhs, rhsType);
}
if (isOverloadedArithmeticOperator) {
combinedType = inferrer.typeSchemaEnvironment
.getTypeOfOverloadedArithmetic(readType, rhsType);
} else {
combinedType = combinerType.returnType;
}
var checkKind = inferrer.preCheckInvocationContravariance(read, readType,
combinerMember, combiner, combiner.arguments, combiner);
var replacedCombiner = inferrer.handleInvocationContravariance(checkKind,
combiner, combiner.arguments, combiner, combinedType, combinerType);
_storeLetType(inferrer, replacedCombiner, combinedType);
return combinedType;
} else {
var rhsType = inferrer.inferExpression(rhs, writeContext, true);
_storeLetType(inferrer, rhs, rhsType);
return rhsType;
combinedType = inferrer.inferExpression(rhs, writeContext, true);
_storeLetType(inferrer, rhs, combinedType);
}
if (write != null) {
if (this is ShadowIndexAssign) {
_storeLetType(inferrer, write, const VoidType());
} else {
_storeLetType(inferrer, write, combinedType);
}
}
return isPostIncDec ? readType : combinedType;
}
}

Expand Down Expand Up @@ -980,7 +982,6 @@ class ShadowIndexAssign extends ShadowComplexAssignmentWithReceiver {
// when doing compound assignment?
var calleeType =
inferrer.getCalleeFunctionType(writeMember, receiverType, false);
_storeLetType(inferrer, write, calleeType.returnType);
DartType indexContext;
DartType writeContext;
if (calleeType.positionalParameters.length >= 2) {
Expand Down Expand Up @@ -1485,7 +1486,6 @@ class ShadowPropertyAssign extends ShadowComplexAssignmentWithReceiver {
// member. TODO(paulberry): would it be better to use the read member when
// doing compound assignment?
var writeContext = inferrer.getSetterType(writeMember, receiverType);
_storeLetType(inferrer, write, writeContext);
var inferredType = _inferRhs(inferrer, readType, writeContext);
if (inferrer.strongMode) nullAwareGuard?.staticType = inferredType;
inferrer.listener.propertyAssignExit(desugared, inferredType);
Expand Down Expand Up @@ -1591,7 +1591,6 @@ class ShadowStaticAssignment extends ShadowComplexAssignment {
var write = this.write;
if (write is StaticSet) {
writeContext = write.target.setterType;
_storeLetType(inferrer, write, writeContext);
var target = write.target;
if (target is ShadowField && target._inferenceNode != null) {
target._inferenceNode.resolve();
Expand Down Expand Up @@ -2157,7 +2156,6 @@ class ShadowVariableAssignment extends ShadowComplexAssignment {
if (read != null) {
_storeLetType(inferrer, read, writeContext);
}
_storeLetType(inferrer, write, writeContext);
}
var inferredType = _inferRhs(inferrer, readType, writeContext);
inferrer.listener.variableAssignExit(desugared, inferredType);
Expand Down
11 changes: 7 additions & 4 deletions pkg/front_end/testcases/ast_builder.status
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,8 @@ inference/infer_accessor_from_later_inferred_field: Crash
inference/infer_assign_to_implicit_this: Crash
inference/infer_assign_to_implicit_this_upwards: Crash
inference/infer_assign_to_index_full: Crash
inference/infer_assign_to_index_set_vs_get: Crash
inference/infer_assign_to_index_super: Crash
inference/infer_assign_to_index_super_upwards: Crash
inference/infer_assign_to_index_this: Crash
inference/infer_assign_to_index_this_upwards: Crash
inference/infer_assign_to_index_upwards: Crash
inference/infer_assign_to_local: Crash
inference/infer_assign_to_local_upwards: Crash
inference/infer_assign_to_property_full: Crash
Expand Down Expand Up @@ -344,7 +340,12 @@ inference/void_return_type_subtypes_dynamic: Crash
inference_new/dependency_only_if_overloaded: Crash
inference_new/downwards_inference_inside_top_level: Crash
inference_new/field_inference_circularity: Crash
inference_new/indexed_assign_combiner: Crash
inference_new/infer_assign_to_index: Crash
inference_new/infer_assign_to_index_set_vs_get: Crash
inference_new/infer_assign_to_index_super_upwards: Crash
inference_new/infer_assign_to_index_this_upwards: Crash
inference_new/infer_assign_to_index_upwards: Crash
inference_new/infer_assign_to_property: Crash
inference_new/infer_assign_to_property_custom: Crash
inference_new/infer_assign_to_ref: Crash
Expand All @@ -356,7 +357,9 @@ inference_new/infer_instance_field_ref: Crash
inference_new/infer_instance_field_ref_circular: Crash
inference_new/infer_logical: Crash
inference_new/multiple_interface_inheritance: Fail # Issue #30547
inference_new/property_assign_combiner: Crash
inference_new/property_get_toplevel: Crash
inference_new/static_assign_combiner: Crash
inference_new/strongly_connected_component: Crash
inference_new/unsafe_block_closure_inference_function_call_explicit_dynamic_param_via_expr2: Crash
inference_new/unsafe_block_closure_inference_function_call_explicit_type_param_via_expr2: Crash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class C extends core::Object {
dynamic v2 = super.bar;
dynamic v3 = super.[](0);
dynamic v4 = super.bar = self::f<dynamic>();
dynamic v5 = let final core::int #t1 = 0 in let final dynamic #t2 = self::f<dynamic>() in let final dynamic #t3 = super.[]=(#t1, #t2) in #t2;
dynamic v5 = let final core::int #t1 = 0 in let final dynamic #t2 = self::f<dynamic>() in let final void #t3 = super.[]=(#t1, #t2) in #t2;
}
}
static method f<T extends core::Object>() → self::f::T
Expand Down
49 changes: 49 additions & 0 deletions pkg/front_end/testcases/inference_new/indexed_assign_combiner.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) 2017, 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.

/*@testedFeatures=inference*/
library test;

T f<T>() => null;

class A {
C operator +(int value) => null;
C operator *(D value) => null;
}

class B {
E operator +(int value) => null;
E operator *(F value) => null;
}

class C extends B {}

class D {}

class E {}

class F {}

class G {
A operator [](int i) => null;

void operator []=(int i, B value) {}
}

void test1(G g) {
g /*@target=G::[]=*/ [0] *= /*@typeArgs=dynamic*/ f();
var /*@type=C*/ x = g /*@target=G::[]=*/ [0] *= /*@typeArgs=dynamic*/ f();
}

void test2(G g) {
++g /*@target=G::[]=*/ [0];
var /*@type=C*/ x = ++g /*@target=G::[]=*/ [0];
}

void test3(G g) {
g /*@target=G::[]=*/ [0]++;
var /*@type=A*/ x = g /*@target=G::[]=*/ [0]++;
}

main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
library test;
import self as self;
import "dart:core" as core;

class A extends core::Object {
default constructor •() → void
: super core::Object::•()
;
operator +(core::int value) → self::C
return null;
operator *(self::D value) → self::C
return null;
}
class B extends core::Object {
default constructor •() → void
: super core::Object::•()
;
operator +(core::int value) → self::E
return null;
operator *(self::F value) → self::E
return null;
}
class C extends self::B {
default constructor •() → void
: super self::B::•()
;
}
class D extends core::Object {
default constructor •() → void
: super core::Object::•()
;
}
class E extends core::Object {
default constructor •() → void
: super core::Object::•()
;
}
class F extends core::Object {
default constructor •() → void
: super core::Object::•()
;
}
class G extends core::Object {
default constructor •() → void
: super core::Object::•()
;
operator [](core::int i) → self::A
return null;
operator []=(core::int i, self::B value) → void {}
}
static method f<T extends core::Object>() → self::f::T
return null;
static method test1(self::G g) → void {
let final dynamic #t1 = g in let final dynamic #t2 = 0 in #t1.[]=(#t2, #t1.[](#t2).*(self::f<dynamic>()));
dynamic x = let final dynamic #t3 = g in let final dynamic #t4 = 0 in let final dynamic #t5 = #t3.[](#t4).*(self::f<dynamic>()) in let final dynamic #t6 = #t3.[]=(#t4, #t5) in #t5;
}
static method test2(self::G g) → void {
let final dynamic #t7 = g in let final dynamic #t8 = 0 in let final dynamic #t9 = #t7.[](#t8).+(1) in let final dynamic #t10 = #t7.[]=(#t8, #t9) in #t9;
dynamic x = let final dynamic #t11 = g in let final dynamic #t12 = 0 in let final dynamic #t13 = #t11.[](#t12).+(1) in let final dynamic #t14 = #t11.[]=(#t12, #t13) in #t13;
}
static method test3(self::G g) → void {
let final dynamic #t15 = g in let final dynamic #t16 = 0 in #t15.[]=(#t16, #t15.[](#t16).+(1));
dynamic x = let final dynamic #t17 = g in let final dynamic #t18 = 0 in let final dynamic #t19 = #t17.[](#t18) in let final dynamic #t20 = #t17.[]=(#t18, #t19.+(1)) in #t19;
}
static method main() → dynamic {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
library test;
import self as self;
import "dart:core" as core;

class A extends core::Object {
default constructor •() → void
;
operator +(core::int value) → self::C
;
operator *(self::D value) → self::C
;
}
class B extends core::Object {
default constructor •() → void
;
operator +(core::int value) → self::E
;
operator *(self::F value) → self::E
;
}
class C extends self::B {
default constructor •() → void
;
}
class D extends core::Object {
default constructor •() → void
;
}
class E extends core::Object {
default constructor •() → void
;
}
class F extends core::Object {
default constructor •() → void
;
}
class G extends core::Object {
default constructor •() → void
;
operator [](core::int i) → self::A
;
operator []=(core::int i, self::B value) → void
;
}
static method f<T extends core::Object>() → self::f::T
;
static method test1(self::G g) → void
;
static method test2(self::G g) → void
;
static method test3(self::G g) → void
;
static method main() → dynamic
;
Loading

0 comments on commit cded101

Please sign in to comment.