Skip to content

Commit

Permalink
Fix warnings on implicit casts
Browse files Browse the repository at this point in the history
R=leafp@google.com

Review URL: https://codereview.chromium.org/1418653003 .
  • Loading branch information
vsmenon committed Oct 20, 2015
1 parent bf0ba96 commit ba60781
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 13 deletions.
19 changes: 18 additions & 1 deletion pkg/analyzer/lib/src/task/strong/info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,28 @@ abstract class DownCast extends CoercionInfo {
return new StaticTypeError(rules, expression, toT, reason: reason);
}

// TODO(vsm): Change this to an assert when we have generic methods and
// fix TypeRules._coerceTo to disallow implicit sideways casts.
if (!rules.isSubTypeOf(toT, fromT)) {
assert(toT.isSubtypeOf(fromT) || fromT.isAssignableTo(toT));
return new DownCastComposite(rules, expression, cast);
}

// Composite cast: these are more likely to fail.
if (!rules.isGroundType(toT)) {
// This cast is (probably) due to our different treatment of dynamic.
// It may be more likely to fail at runtime.
return new DownCastComposite(rules, expression, cast);
if (fromT is InterfaceType) {
// For class types, we'd like to allow non-generic down casts, e.g.,
// Iterable<T> to List<T>. The intuition here is that raw (generic)
// casts are problematic, and we should complain about those.
var typeArgs = fromT.typeArguments;
if (typeArgs.isEmpty || typeArgs.any((t) => t.isDynamic)) {
return new DownCastComposite(rules, expression, cast);
}
} else {
return new DownCastComposite(rules, expression, cast);
}
}

// Dynamic cast
Expand Down
18 changes: 11 additions & 7 deletions pkg/analyzer/lib/src/task/strong/rules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,6 @@ class TypeRules {

// Produce a coercion which coerces something of type fromT
// to something of type toT.
// If wrap is true and both are function types, a closure
// wrapper coercion is produced using _wrapTo (see above)
// Returns the error coercion if the types cannot be coerced
// according to our current criteria.
Coercion _coerceTo(DartType fromT, DartType toT) {
Expand All @@ -387,11 +385,12 @@ class TypeRules {
// fromT <: toT, no coercion needed
if (isSubTypeOf(fromT, toT)) return Coercion.identity(toT);

// For now, reject conversions between function types and
// call method objects. We could choose to allow casts here.
// Wrapping a function type to assign it to a call method
// object will never succeed. Wrapping the other way could
// be allowed.
// 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 ((fromT is FunctionType && getCallMethodType(toT) != null) ||
(toT is FunctionType && getCallMethodType(fromT) != null)) {
return Coercion.error();
Expand All @@ -400,6 +399,10 @@ class TypeRules {
// Downcast if toT <: fromT
if (isSubTypeOf(toT, fromT)) return Coercion.cast(fromT, toT);

// 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
Expand All @@ -409,6 +412,7 @@ class TypeRules {
if (fromT.isAssignableTo(toT)) {
return Coercion.cast(fromT, toT);
}

return Coercion.error();
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/analyzer/test/src/task/strong/checker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ void main() {
lOfAs = /*severe:StaticTypeError*/mOfOs;
lOfAs = mOfAs;
lOfAs = /*warning:DownCastComposite*/lOfDs;
lOfAs = /*warning:DownCastComposite*/lOfOs;
lOfAs = /*info:DownCastImplicit*/lOfOs;
lOfAs = lOfAs;
}
{
Expand All @@ -993,7 +993,7 @@ void main() {
mOfDs = mOfAs;
mOfDs = /*info:DownCastImplicit*/lOfDs;
mOfDs = /*info:DownCastImplicit*/lOfOs;
mOfDs = /*info:DownCastImplicit*/lOfAs;
mOfDs = /*warning:DownCastComposite*/lOfAs;
}
{
mOfOs = mOfDs;
Expand All @@ -1005,11 +1005,11 @@ void main() {
}
{
mOfAs = /*warning:DownCastComposite*/mOfDs;
mOfAs = /*warning:DownCastComposite*/mOfOs;
mOfAs = /*info:DownCastImplicit*/mOfOs;
mOfAs = mOfAs;
mOfAs = /*warning:DownCastComposite*/lOfDs;
mOfAs = /*warning:DownCastComposite*/lOfOs;
mOfAs = /*warning:DownCastComposite*/lOfAs;
mOfAs = /*info:DownCastImplicit*/lOfOs;
mOfAs = /*info:DownCastImplicit*/lOfAs;
}
}
Expand Down

0 comments on commit ba60781

Please sign in to comment.