Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(567)

Unified Diff: lib/compiler/implementation/resolver.dart

Issue 10915083: Change assert implementation to not depend on a top-level function called 'assert'. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Moved assert detection further down. Now demetered. Created 8 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/compiler/implementation/lib/mock.dart ('k') | lib/compiler/implementation/scanner/keyword.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/compiler/implementation/resolver.dart
diff --git a/lib/compiler/implementation/resolver.dart b/lib/compiler/implementation/resolver.dart
index 4b6a8f49fdbe5ed61d7e91227689569694408830..b525c63b12202ef48aace1df5bcba00a94acb373 100644
--- a/lib/compiler/implementation/resolver.dart
+++ b/lib/compiler/implementation/resolver.dart
@@ -954,6 +954,7 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
bool inInstanceContext;
Scope scope;
ClassElement currentClass;
+ ExpressionStatement currentExpressionStatement;
bool typeRequired = false;
StatementScope statementScope;
int allowedCategory = ElementCategory.VARIABLE | ElementCategory.FUNCTION;
@@ -1138,7 +1139,10 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
visitEmptyStatement(EmptyStatement node) { }
visitExpressionStatement(ExpressionStatement node) {
+ ExpressionStatement oldExpressionStatement = currentExpressionStatement;
+ currentExpressionStatement = node;
visit(node.expression);
+ currentExpressionStatement = oldExpressionStatement;
}
visitFor(For node) {
@@ -1192,10 +1196,47 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
return (str === '&&' || str == '||' || str == '!');
}
+ /**
+ * Check the lexical scope chain for a declaration with the name "assert".
+ *
+ * This is used to detect whether "assert(x)" is actually an assertion or
+ * just a call expression.
+ * It does not check fields inherited from a superclass.
+ * This does not find setters, since their name would be "assert=".
ngeoffray 2012/09/06 09:37:31 what is this assert=? We don't create new names to
Lasse Reichstein Nielsen 2012/09/06 11:16:14 That's from the specification. The name of the dec
+ *
+ */
+ bool isAssertInLexicalScope() {
+ for (Scope scope = this.scope; scope !== null; scope = scope.parent) {
ahe 2012/09/06 08:50:42 Could we make this an operation on the scope inste
Lasse Reichstein Nielsen 2012/09/06 09:12:36 How about for (Scope.scope in this.scope.chain) {
+ Element local = scope.localLookup(const SourceString("assert"));
+ if (local !== null) {
+ if (local.isAbstractField()) {
ahe 2012/09/06 08:50:42 I don't believe this implementation is correct: a
Lasse Reichstein Nielsen 2012/09/06 09:12:36 I queried Gilad, and this interpretation of the sp
+ AbstractFieldElement field = local;
+ if (field.getter === null) continue;
+ }
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /** Check whether [node] is the body of the current expression statement. */
+ bool isExpressionStatementBody(Node node) {
+ return currentExpressionStatement !== null &&
+ currentExpressionStatement.expression === node;
+ }
+
Element resolveSend(Send node) {
Selector selector = resolveSelector(node);
if (node.receiver === null) {
+ // If this send is the body of an expression statement, and is on the
+ // form "assert(expr);", and there is no declaration with name "assert"
+ // in the lexical scope, then this is actually an assertion.
+ if (isExpressionStatementBody(node) &&
+ selector.isAssertSyntax() &&
+ !isAssertInLexicalScope()) {
+ return compiler.assertMethod;
+ }
return node.selector.accept(this);
}
@@ -1379,16 +1420,6 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
world.registerDynamicInvocation(call.name, call);
}
- // TODO(ngeoffray): We should do the check in
- // visitExpressionStatement instead.
- if (target === compiler.assertMethod && !node.isCall) {
- // We can only use assert by calling it.
- if (!inInstanceContext) {
- error(node, MessageKind.MISSING_ARGUMENTS_TO_ASSERT, [node]);
- }
- target = null;
- }
-
// TODO(ngeoffray): Warn if target is null and the send is
// unqualified.
useElement(node, target);
@@ -2557,7 +2588,14 @@ class Scope {
Scope(this.parent, this.element);
abstract Element add(Element element);
- abstract Element lookup(SourceString name);
+
+ Element lookup(SourceString name) {
+ Element result = localLookup(name);
+ if (result != null) return result;
+ return parent.lookup(name);
+ }
+
+ abstract Element localLookup(SourceString name);
}
/**
@@ -2586,7 +2624,7 @@ class TypeDeclarationScope extends Scope {
return null;
}
- Element lookup(SourceString name) {
+ Element localLookup(SourceString name) {
Link<DartType> typeVariableLink = element.typeVariables;
while (!typeVariableLink.isEmpty()) {
TypeVariableType typeVariable = typeVariableLink.head;
@@ -2595,8 +2633,7 @@ class TypeDeclarationScope extends Scope {
}
typeVariableLink = typeVariableLink.tail;
}
-
- return parent.lookup(name);
+ return null;
}
String toString() =>
@@ -2612,11 +2649,7 @@ class MethodScope extends Scope {
assert(parent !== null);
}
- Element lookup(SourceString name) {
- Element found = elements[name];
- if (found !== null) return found;
- return parent.lookup(name);
- }
+ Element localLookup(SourceString name) => elements[name];
Element add(Element newElement) {
if (elements.containsKey(newElement.name)) {
@@ -2644,12 +2677,17 @@ class ClassScope extends TypeDeclarationScope {
ClassScope(Scope parentScope, ClassElement element)
: super(parentScope, element);
- Element lookup(SourceString name) {
+ Element localLookup(SourceString name) {
ClassElement cls = element;
Element result = cls.lookupLocalMember(name);
if (result !== null) return result;
- result = super.lookup(name);
- if (result != null) return result;
+ return super.localLookup(name);
+ }
+
+ Element lookup(SourceString name) {
+ Element result = super.lookup(name);
+ if (result !== null) return result;
+ ClassElement cls = element;
return cls.lookupSuperMember(name);
}
@@ -2664,9 +2702,9 @@ class TopScope extends Scope {
LibraryElement get library => element;
TopScope(LibraryElement library) : super(null, library);
- Element lookup(SourceString name) {
- return library.find(name);
- }
+
+ Element localLookup(SourceString name) => library.find(name);
+ Element lookup(SourceString name) => localLookup(name);
Element add(Element newElement) {
throw "Cannot add an element in the top scope";
« no previous file with comments | « lib/compiler/implementation/lib/mock.dart ('k') | lib/compiler/implementation/scanner/keyword.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698