Skip to content

Commit

Permalink
Fixes #287 and other failures statically checking SDK tests
Browse files Browse the repository at this point in the history
This adds some defensiveness to override checking and other checks.

It fixes all known crashers running dartanalyzer in strong mode on language, co19, etc.

R=leafp@google.com

Review URL: https://codereview.chromium.org/1303913003 .
  • Loading branch information
vsmenon committed Aug 21, 2015
1 parent e420033 commit 242511d
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 10 deletions.
30 changes: 23 additions & 7 deletions pkg/dev_compiler/lib/src/checker/checker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,14 @@ class _OverrideChecker {
void _checkSuperOverrides(ClassDeclaration node) {
var seen = new Set<String>();
var current = node.element.type;
var visited = new Set<InterfaceType>();
do {
visited.add(current);
current.mixins.reversed
.forEach((m) => _checkIndividualOverridesFromClass(node, m, seen));
_checkIndividualOverridesFromClass(node, current.superclass, seen);
current = current.superclass;
} while (!current.isObject);
} while (!current.isObject && !visited.contains(current));
}

/// Checks that implementations correctly override all reachable interfaces.
Expand Down Expand Up @@ -152,10 +154,21 @@ class _OverrideChecker {
/// invalid override, for [InterfaceType]s we use [errorLocation] instead.
void _checkInterfacesOverrides(
cls, Iterable<InterfaceType> interfaces, Set<String> seen,
{bool includeParents: true, AstNode errorLocation}) {
{Set<InterfaceType> visited,
bool includeParents: true,
AstNode errorLocation}) {
var node = cls is ClassDeclaration ? cls : null;
var type = cls is InterfaceType ? cls : node.element.type;

if (visited == null) {
visited = new Set<InterfaceType>();
} else if (visited.contains(type)) {
// Malformed type.
return;
} else {
visited.add(type);
}

// Check direct overrides on [type]
for (var interfaceType in interfaces) {
if (node != null) {
Expand Down Expand Up @@ -186,7 +199,7 @@ class _OverrideChecker {
// No need to copy [seen] here because we made copies above when reporting
// errors on mixins.
_checkInterfacesOverrides(parent, interfaces, seen,
includeParents: true, errorLocation: loc);
visited: visited, includeParents: true, errorLocation: loc);
}
}

Expand Down Expand Up @@ -232,6 +245,7 @@ class _OverrideChecker {
var setter = element.setter;
bool found = _checkSingleOverride(getter, baseType, variable, member);
if (!variable.isFinal &&
!variable.isConst &&
_checkSingleOverride(setter, baseType, variable, member)) {
found = true;
}
Expand Down Expand Up @@ -389,10 +403,12 @@ class CodeChecker extends RecursiveAstVisitor {
: rules.provider.iterableType;
var loopVariable = node.identifier != null
? node.identifier
: node.loopVariable.identifier;
var iteratorType = loopVariable.staticType;
var checkedType = iterableType.substitute4([iteratorType]);
checkAssignment(expr, checkedType);
: node.loopVariable?.identifier;
if (loopVariable != null) {
var iteratorType = loopVariable.staticType;
var checkedType = iterableType.substitute4([iteratorType]);
checkAssignment(expr, checkedType);
}
node.visitChildren(this);
}

Expand Down
7 changes: 5 additions & 2 deletions pkg/dev_compiler/lib/src/checker/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ class LibraryResolverWithInference extends LibraryResolver {
var element = cls.element;
var type = element.type;
if (seen.contains(type)) return;
seen.add(type);
for (var supertype in element.allSupertypes) {
var supertypeClass = typeToDeclaration[supertype];
if (supertypeClass != null) visit(supertypeClass);
}
seen.add(type);

if (_options.inferFromOverrides) {
// Infer field types from overrides first, otherwise from initializers.
Expand Down Expand Up @@ -466,7 +466,10 @@ class RestrictedResolverVisitor extends ResolverVisitor {
var element = node.element;
if (element is FieldFormalParameterElement) {
if (element.type.isDynamic) {
element.type = element.field.type;
// In malformed code, there may be no actual field.
if (element.field != null) {
element.type = element.field.type;
}
}
}
super.visitFieldFormalParameter(node);
Expand Down
12 changes: 11 additions & 1 deletion pkg/dev_compiler/lib/src/checker/rules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ class RestrictedRules extends TypeRules {
return true;
}

throw new StateError("Unexpected type");
// We should not see any other type aside from malformed code.
return false;
}

/// Check that f1 is a subtype of f2. [ignoreReturn] is used in the DDC
Expand Down Expand Up @@ -407,6 +408,10 @@ class RestrictedRules extends TypeRules {
return isSubTypeOf(bound, t2);
}

if (t2 is TypeParameterType) {
return false;
}

if (t2.isDartCoreFunction) {
if (t1 is FunctionType) return true;
if (t1.element is ClassElement) {
Expand Down Expand Up @@ -513,6 +518,10 @@ class RestrictedRules extends TypeRules {
}

DartType elementType(Element e) {
if (e == null) {
// Malformed code - just return dynamic.
return provider.dynamicType;
}
return (e as dynamic).type;
}

Expand Down Expand Up @@ -704,6 +713,7 @@ class DownwardsInference {
// Check that we were not passed any type arguments
if (classTypeName.typeArguments != null) return false;
// Infer type arguments
if (t is! InterfaceType) return false;
var targs = _matchTypes(type, t);
if (targs == null) return false;
if (e.staticElement == null) return false;
Expand Down

0 comments on commit 242511d

Please sign in to comment.