Skip to content

Commit

Permalink
fix #26120, sideways casts no longer supported
Browse files Browse the repository at this point in the history
R=vsm@google.com

Review URL: https://codereview.chromium.org/2231273002 .
  • Loading branch information
John Messerly committed Aug 12, 2016
1 parent ee3a93a commit e36fa2b
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 101 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@
}
```

* Breaking change - sideways casts are no longer allowed
(SDK issue [26120](https://github.com/dart-lang/sdk/issues/26120)).

### Dart VM

* The dependency on BoringSSL has been rolled forward. Going forward, builds
Expand Down
5 changes: 2 additions & 3 deletions pkg/analyzer/lib/src/generated/type_system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,7 @@ class StrongTypeSystemImpl extends TypeSystem {

// Don't allow implicit downcasts between function types
// and call method objects, as these will almost always fail.
if ((fromType is FunctionType && getCallMethodType(toType) != null) ||
(toType is FunctionType && getCallMethodType(fromType) != null)) {
if (fromType is FunctionType && getCallMethodType(toType) != null) {
return false;
}

Expand All @@ -373,7 +372,7 @@ class StrongTypeSystemImpl extends TypeSystem {

// If the subtype relation goes the other way, allow the implicit
// downcast.
if (isSubtypeOf(toType, fromType) || toType.isAssignableTo(fromType)) {
if (isSubtypeOf(toType, fromType)) {
// TODO(leafp,jmesserly): we emit warnings/hints for these in
// src/task/strong/checker.dart, which is a bit inconsistent. That
// code should be handled into places that use isAssignableTo, such as
Expand Down
24 changes: 5 additions & 19 deletions pkg/analyzer/lib/src/task/strong/checker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -749,14 +749,10 @@ class CodeChecker extends RecursiveAstVisitor {
// fromT <: toT, no coercion needed.
if (rules.isSubtypeOf(from, to)) return;

// TODO(vsm): We can get rid of the second clause if we disallow
// all sideways casts - see TODO below.
// -------
// Note: a function type is never assignable to a class per the Dart
// spec - even if it has a compatible call method. We disallow as
// well for consistency.
if ((from is FunctionType && rules.getCallMethodType(to) != null) ||
(to is FunctionType && rules.getCallMethodType(from) != null)) {
if (from is FunctionType && rules.getCallMethodType(to) != null) {
return;
}

Expand All @@ -766,19 +762,9 @@ class CodeChecker extends RecursiveAstVisitor {
return;
}

// TODO(vsm): Once we have generic methods, we should delete this
// workaround. These sideways casts are always ones we warn about
// - i.e., we think they are likely to fail at runtime.
// -------
// Downcast if toT <===> fromT
// The intention here is to allow casts that are sideways in the restricted
// type system, but allowed in the regular dart type system, since these
// are likely to succeed. The canonical example is List<dynamic> and
// Iterable<T> for some concrete T (e.g. Object). These are unrelated
// in the restricted system, but List<dynamic> <: Iterable<T> in dart.
if (from.isAssignableTo(to)) {
_recordImplicitCast(expr, from, to);
}
// Anything else is an illegal sideways cast.
// However, these will have been reported already in error_verifier, so we
// don't need to report them again.
}

void _checkFieldAccess(AstNode node, AstNode target, SimpleIdentifier field) {
Expand Down Expand Up @@ -1057,7 +1043,7 @@ class CodeChecker extends RecursiveAstVisitor {
// toT <:_R fromT => to <: fromT
// NB: classes with call methods are subtypes of function
// types, but the function type is not assignable to the class
assert(toType.isSubtypeOf(fromType) || fromType.isAssignableTo(toType));
assert(toType.isSubtypeOf(fromType));

// Inference "casts":
if (expression is Literal || expression is FunctionExpression) {
Expand Down
22 changes: 16 additions & 6 deletions pkg/analyzer/test/generated/type_system_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ class LeastUpperBoundTest extends LeastUpperBoundTestBase {
FunctionType expected = _functionType([objectType, numType, numType]);
_checkLeastUpperBound(type1, type2, expected);
}

void test_nestedNestedFunctionsLubInnermostParamTypes() {
FunctionType type1 = _functionType([
_functionType([
Expand Down Expand Up @@ -620,7 +621,11 @@ class StrongAssignabilityTest {
numType,
bottomType
];
List<DartType> unrelated = <DartType>[intType, stringType, interfaceType,];
List<DartType> unrelated = <DartType>[
intType,
stringType,
interfaceType,
];

_checkGroups(doubleType,
interassignable: interassignable, unrelated: unrelated);
Expand Down Expand Up @@ -750,7 +755,10 @@ class StrongAssignabilityTest {
doubleType,
bottomType
];
List<DartType> unrelated = <DartType>[stringType, interfaceType,];
List<DartType> unrelated = <DartType>[
stringType,
interfaceType,
];

_checkGroups(numType,
interassignable: interassignable, unrelated: unrelated);
Expand Down Expand Up @@ -780,10 +788,12 @@ class StrongAssignabilityTest {

void _checkCrossLattice(
DartType top, DartType left, DartType right, DartType bottom) {
_checkGroups(top, interassignable: <DartType>[top, left, right, bottom]);
_checkGroups(left, interassignable: <DartType>[top, left, right, bottom]);
_checkGroups(right, interassignable: <DartType>[top, left, right, bottom]);
_checkGroups(bottom, interassignable: <DartType>[top, left, right, bottom]);
_checkGroups(top, interassignable: [top, left, right, bottom]);
_checkGroups(left,
interassignable: [top, left, bottom], unrelated: [right]);
_checkGroups(right,
interassignable: [top, right, bottom], unrelated: [left]);
_checkGroups(bottom, interassignable: [top, left, right, bottom]);
}

void _checkEquivalent(DartType type1, DartType type2) {
Expand Down
Loading

0 comments on commit e36fa2b

Please sign in to comment.