Peek past balanced parens to see if they are an expression or lambda.

parser.dart

Index: parser.dart
diff --git a/parser.dart b/parser.dart
index 1fe93cb0fa817ae5b66c8bfd58d36624d164a84d..62777145c9194271791acca757a70c910e0f0b8e 100644
--- a/parser.dart
+++ b/parser.dart
@@ -13,7 +13,7 @@
  * very clearly detected and is reported in a later compiler phase.
  */
 class Parser {
-  Tokenizer tokenizer;
+  TokenSource tokenizer;

[jimhug, 2011/10/31 14:16:46]

Note: This change seems good independent of the rest.

 
   final SourceFile source;
   /** Enables diet parse, which skips function bodies. */
@@ -25,11 +25,20 @@ class Parser {
   Token _previousToken;
   Token _peekToken;
 
+  // Map from start position of a '(' to token following the matching ')'. Used
+  // to distinguish closure formal parameter lists from parenthesised
+  // expressions and argument lists.  Closure formals are followed by '=>' or
+  // '{'.
+  Map<int, Token> _afterCloseParenCache;
+  int _highestCachePosition = -1;
+
   Parser(this.source, [this.diet = false, int startOffset = 0]) {
     tokenizer = new Tokenizer(source, true, startOffset);
     _peekToken = tokenizer.next();
     _previousToken = null;
     _inInitializers = false;
+
+    _afterCloseParenCache = new Map<int, Token>();
   }
 
   /** Generate an error if [source] has not been completely consumed. */
@@ -870,7 +879,6 @@ class Parser {
 
   arguments() {
     var args = [];
-    // TODO(jimhug): switch to forced formals when get a DeclaredId
     _eat(TokenKind.LPAREN);
     if (!_maybeEat(TokenKind.RPAREN)) {
       do {
@@ -884,8 +892,7 @@ class Parser {
   finishPostfixExpression(expr) {
     switch(_peek()) {
       case TokenKind.LPAREN:
-        return finishPostfixExpression(new CallExpression(expr, arguments(),
-          _makeSpan(expr.span.start)));
+        return finishCallOrLambdaExpression(expr);
       case TokenKind.LBRACK:
         _eat(TokenKind.LBRACK);
         var index = expression();
@@ -905,11 +912,10 @@ class Parser {
 
       // These are pseudo-expressions supported for cover grammar
       // must be forbidden when parsing initializers.
+
       case TokenKind.ARROW:
       case TokenKind.LBRACE:
-        if (_inInitializers) return expr;
-        var body = functionBody(true);
-        return _makeFunction(expr, body);
+         return expr;
 
       default:
         if (_peekIdentifier()) {
@@ -922,6 +928,18 @@ class Parser {
     }
   }
 
+  finishCallOrLambdaExpression(expr) {
+    if (!_inInitializers && _atClosureParameters()) {
+      var formals = formalParameterList();
+      var body = functionBody(true);
+      return _makeFunction(expr, formals, body);
+    } else {
+      var args = arguments();
+      return finishPostfixExpression(
+          new CallExpression(expr, args, _makeSpan(expr.span.start)));
+    }
+  }
+
   /** Checks if the given expression is a binary op of the given kind. */
   _isBin(expr, kind) {
     return expr is BinaryExpression && expr.op.kind == kind;
@@ -1090,15 +1108,14 @@ class Parser {
 
   _parenOrLambda() {
     int start = _peekToken.start;
-    var args = arguments();
-    if (!_inInitializers &&
-        (_peekKind(TokenKind.ARROW) || _peekKind(TokenKind.LBRACE))) {
+    if (!_inInitializers && _atClosureParameters()) {
+      var formals = formalParameterList();
       var body = functionBody(true);
-      var formals = _makeFormals(args);
       var func = new FunctionDefinition(null, null, null, formals, null,
         body, _makeSpan(start));
       return new LambdaExpression(func, func.span);
     } else {
+      var args = arguments();
       if (args.length == 1) {
         return new ParenExpression(args[0].value, _makeSpan(start));
       } else {
@@ -1108,6 +1125,63 @@ class Parser {
     }
   }
 
+  bool _atClosureParameters() {
+    Token afterCloseParen = _peekPastCloseParen();
+    return afterCloseParen.kind == TokenKind.ARROW
+        || afterCloseParen.kind == TokenKind.LBRACE;
+  }
+
+  Token _peekPastCloseParen() {
+    int pos = _peekToken.start;
+    if (pos > _highestCachePosition)

[jimhug, 2011/10/31 14:16:46]

Style: The return either needs to be on the same line as the if or it needs {}'s
surrounding it.

+      return _fillAfterCloseParenCache();
+    return _afterCloseParenCache[pos];
+  }
+
+  _fillAfterCloseParenCache() {
+    // Scan for the matching RPAREN to the current LPAREN and return the
+    // following token.  Add intermediate values to cache to prevent this
+    // look-ahead scan from being called again for nested parentheses.  Note
+    // that the outermost parens are not added to the cache as the following
+    // token is directly available; not touching the cache for non-nested parens
+    // has a small performance benefit.
+    List tokens = [];
+    List positions = [];
+    Token firstOpenParen = _peekToken;
+    while (true) {
+      Token token = _next();
+      tokens.add(token);
+      int kind = token.kind;
+      if (kind == TokenKind.LPAREN) {
+        positions.add(token.start);
+      } else if (kind == TokenKind.RPAREN) {
+        int openPos = positions.removeLast();
+        if (positions.length == 0)
+          break;
+        _afterCloseParenCache[openPos] = _peekToken;
+        if (openPos > _highestCachePosition)
+          _highestCachePosition = openPos;
+      } else if (kind == TokenKind.END_OF_FILE) {
+        _error('parenthesis never closed', firstOpenParen.span);
+        // The invariant that all positions less than _highestCachePosition are
+        // in the cache is violated if we bail out here due to the error.  We
+        // could add the pending elements of the positions list, but instead we
+        // clear the cache.  Clearing the cache also causes all the unmatched
+        // parens to be enumerated, which might be a useful diagnostic behavior.
+        _afterCloseParenCache = new Map<int,Token>();
+        _highestCachePosition = -1;
+        break;
+      }
+    }
+
+    var after = _peekToken;
+    // Put all the lookahead tokens back into the parser's token stream.
+    tokens.add(_peekToken);
+    tokenizer = new DivertedTokenSource(tokens, this, tokenizer);
+    _next();  // Re-synchronize parser lookahead state.
+    return after;
+  }
+
 
   _typeAsIdentifier(type) {
     // TODO(jimhug): lots of errors to check for
@@ -1488,123 +1562,24 @@ class Parser {
   ///////////////////////////////////////////////////////////////////
 
   /**
-   * Converts an [Expression] and a [Statment] body into a
+   * Converts an [Expression], [Formals] and a [Statment] body into a
    * [FunctionDefinition].
    */
-  _makeFunction(expr, body) {
+  _makeFunction(expr, formals, body) {
     var name, type;
-    if (expr is CallExpression) {
-      if (expr.target is VarExpression) {
-        name = expr.target.name;
-        type = null;
-      } else if (expr.target is DeclaredIdentifier) {
-        name = expr.target.name;
-        type = expr.target.type;
-      } else {
-        _error('bad function');
-      }
-      var formals = _makeFormals(expr.arguments);
-      var span =
-        new SourceSpan(expr.span.file, expr.span.start, body.span.end);
-      var func =
-        new FunctionDefinition(null, type, name, formals, null, body, span);
-      return new LambdaExpression(func, func.span);
-    } else {
-      _error('expected function');
-    }
-  }
-
-  /** Converts a single expression into a formal or list of formals. */
-  _makeFormal(expr) {
     if (expr is VarExpression) {
-      return new FormalNode(false, false, null, expr.name, null, expr.span);
+      name = expr.name;
+      type = null;
     } else if (expr is DeclaredIdentifier) {
-      return new FormalNode(false, false, expr.type, expr.name, null,
-        expr.span);
-    } else if (_isBin(expr, TokenKind.ASSIGN) &&
-               (expr.x is DeclaredIdentifier)) {
-      DeclaredIdentifier di = expr.x; // TODO(jimhug): inference should handle!
-      return new FormalNode(false, false, di.type, di.name, expr.y,
-        expr.span);
-    } else if (_isBin(expr, TokenKind.LT)) {
-      // special signaling value to merge with next arg.
-      return null;
-    } else if (expr is ListExpression) {
-      return _makeFormalsFromList(expr);
+      name = expr.name;
+      type = expr.type;
     } else {
-      _error('expected formal', expr.span);
-    }
-  }
-
-  _makeFormalsFromList(expr) {
-    if (expr.isConst) {
-      _error('expected formal, but found "const"', expr.span);
-    } else if (expr.type != null) {
-      _error('expected formal, but found generic type arguments',
-         expr.type.span);
-    }
-
-    return _makeFormalsFromExpressions(expr.values, allowOptional:false);
-  }
-
-  /** Converts a list of arguments into a list of formals. */
-  _makeFormals(arguments) {
-    var expressions = [];
-    for (int i = 0; i < arguments.length; i++) {
-      final arg = arguments[i];
-      if (arg.label != null) {
-        _error('expected formal, but found ":"');
-      }
-      expressions.add(arg.value);
-    }
-    return _makeFormalsFromExpressions(expressions, allowOptional:true);
-  }
-
-  /** Converts a list of expressions into a list of formals. */
-  _makeFormalsFromExpressions(expressions, [bool allowOptional]) {
-    var formals = [];
-    for (int i = 0; i < expressions.length; i++) {
-      var formal = _makeFormal(expressions[i]);
-      if (formal == null) {
-        // special signal that we have the A<C case
-        var baseType = _makeType(expressions[i].x);
-        var typeParams = [_makeType(expressions[i].y)];
-        i++;
-        while (i < expressions.length) {
-          var expr = expressions[i++];
-          // Looking for D > m closer
-          if (_isBin(expr, TokenKind.GT)) {
-            typeParams.add(_makeType(expr.x));
-            var type = new GenericTypeReference(baseType, typeParams, 0,
-              _makeSpan(baseType.span.start));
-            var name = null;
-            if (expr.y is VarExpression) {
-              // TODO(jimhug): Should be handled by inference!
-              VarExpression ve = expr.y;
-              name = ve.name;
-            } else {
-              _error('expected formal', expr.span);
-            }
-            formal = new FormalNode(false, false, type, name, null,
-              _makeSpan(expressions[0].span.start));
-            break;
-          } else {
-            typeParams.add(_makeType(expr));
-          }
-        }
-        formals.add(formal);
-
-      } else if (formal is List) {
-        formals.addAll(formal);
-        if (!allowOptional) {
-          _error('unexpected nested optional formal', expressions[i].span);
-        }
-
-      } else {
-        formals.add(formal);
-      }
+      _error('bad function');
     }
-    return formals;
+    var span = new SourceSpan(expr.span.file, expr.span.start, body.span.end);
+    var func =
+        new FunctionDefinition(null, type, name, formals, null, body, span);
+    return new LambdaExpression(func, func.span);
   }
 
   /** Converts an expression to a [DeclaredIdentifier]. */
@@ -1629,3 +1604,21 @@ class Parser {
     }
   }
 }
+
+
+class DivertedTokenSource implements TokenSource {

[jimhug, 2011/10/31 14:16:46]

Now that you've provided this, I'd love to do an experiment where we first
Tokenize and then parse the file.  I can' work out in my head how that will turn
out  - since I think it will have to do with cache contention issues more than
anything else.

+  final List tokens;
+  final Parser parser;
+  final TokenSource previousTokenizer;
+  DivertedTokenSource(this.tokens, this.parser, this.previousTokenizer);
+
+  int _pos = 0;
+  next() {
+    var token = tokens[_pos];
+    ++_pos;

[jimhug, 2011/10/31 14:16:46]

Style: prefer _pos++ for this bare increment.

+    if (_pos == tokens.length) {
+      parser.tokenizer = previousTokenizer;
+    }
+    return token;
+  }
+}