Implement parsing support for generic functions

runtime/vm/parser.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/parser.h"
 #include "vm/flags.h"
 
 #ifndef DART_PRECOMPILED_RUNTIME
 
 #include "lib/invocation_mirror.h"
@@ skipping 30 matching lines @@
 
 namespace dart {
 
 DEFINE_FLAG(bool, enable_debug_break, false, "Allow use of break \"message\".");
 DEFINE_FLAG(bool, trace_parser, false, "Trace parser operations.");
 DEFINE_FLAG(bool, warn_mixin_typedef, true, "Warning on legacy mixin typedef.");
 // TODO(floitsch): remove the conditional-directive flag, once we publicly
 // committed to the current version.
 DEFINE_FLAG(bool, conditional_directives, true,
     "Enable conditional directives");
+DEFINE_FLAG(bool, generic_method_syntax, false, "Enbable generic functions.");
 DEFINE_FLAG(bool, initializing_formal_access, false,
     "Make initializing formal parameters visible in initializer list.");
 DEFINE_FLAG(bool, warn_super, false,
     "Warning if super initializer not last in initializer list.");
 DEFINE_FLAG(bool, warn_patch, false, "Warn on old-style patch syntax.");
 DEFINE_FLAG(bool, await_is_keyword, false,
     "await and yield are treated as proper keywords in synchronous code.");
 
 DECLARE_FLAG(bool, profile_vm);
 DECLARE_FLAG(bool, trace_service);
@@ skipping 57 matching lines @@
   ~BoolScope() {
     *_addr = _saved_value;
   }
 
  private:
   bool* _addr;
   bool _saved_value;
 };
 
 
+// Helper class to save and restore token position.
+class Parser::TokenPosScope : public ValueObject {
+ public:
+  explicit TokenPosScope(Parser *p) : _p(p) {
+    _saved_pos = p->TokenPos();
+  }
+  TokenPosScope(Parser *p, TokenPosition pos) : _p(p), _saved_pos(pos) {
+  }
+  ~TokenPosScope() {
+    _p->SetPosition(_saved_pos);
+  }
+
+ private:
+  Parser* _p;
+  TokenPosition _saved_pos;

[siva, 2016/08/19 20:30:58]

Normally we have been using _ as a suffix not prefix for private instance
variables, any reason for changing that.

DISALLOW_COPY_AND_ASSIGN(Parser::TokenPosScope);

[hausner, 2016/08/19 21:40:04]

Done. Accedentally mixed dart and c++ convention.

+};
+
+
 class RecursionChecker : public ValueObject {
  public:
   explicit RecursionChecker(Parser* p) : parser_(p) {
     parser_->recursion_counter_++;
     // No need to check the stack unless the parser is in an unusually deep
     // recurive state. Thus, we omit the more expensive stack checks in
     // the common case.
     const int kMaxUncheckedDepth = 100;  // Somewhat arbitrary.
     if (parser_->recursion_counter_ > kMaxUncheckedDepth) {
       parser_->CheckStack();
@@ skipping 1920 matching lines @@
   const intptr_t num_existing_parameters =
       params->num_fixed_parameters + params->num_optional_parameters;
   for (intptr_t i = 0; i < num_existing_parameters; i++) {
     ParamDesc& existing_parameter = (*params->parameters)[i];
     if (existing_parameter.name->Equals(*parameter.name)) {
       ReportError(parameter.name_pos, "duplicate formal parameter '%s'",
                   parameter.name->ToCString());
     }
   }
 
-  if (CurrentToken() == Token::kLPAREN) {
+  if (IsParameterPart()) {
     // This parameter is probably a closure. If we saw the keyword 'var'
     // or 'final', a closure is not legal here and we ignore the
     // opening parens.
     // TODO(hausner): The language spec appears to allow var and final
     // in signature types when used with initializing formals:
     // fieldFormalParameter:
     // metadata finalConstVarOrType? this ‘.’ identifier formalParameterList? ;
     if (!var_seen && !final_seen) {
       // The parsed parameter type is actually the function result type.
       const AbstractType& result_type =
           AbstractType::Handle(Z, parameter.type->raw());
 
       // Finish parsing the function type parameter.
+      if (CurrentToken() == Token::kLT) {
+        // TODO(hausner): handle generic function types.
+        TokenPosition type_param_pos = TokenPos();
+        if (!TryParseTypeParameters()) {
+          ReportError(type_param_pos, "error in type parameters");

[siva, 2016/08/19 20:30:58]

This error seems to be printed unconditionally even when
'FLAG_generic_method_syntax is not on', the old code did not have this error?

[hausner, 2016/08/19 21:40:04]

The old code would have printed some other syntax error message. In practice,
this should actually not fail since the IsParameterPart() call above already
checks whether the type parameters look syntactically correct.

I'll reverse the two error messages so that the "not supported" error is always
printed when the flag is not enabled.

+        }
+        if (!FLAG_generic_method_syntax) {
+          ReportError(type_param_pos, "generic function types not supported");
+        }
+      }
+
+      ASSERT(CurrentToken() == Token::kLPAREN);
       ParamList func_params;
 
       // Add implicit closure object parameter.
       func_params.AddFinalParameter(
           TokenPos(),
           &Symbols::ClosureParameter(),
           &Object::dynamic_type());
 
       const bool no_explicit_default_values = false;
       ParseFormalParameterList(no_explicit_default_values, false, &func_params);
@@ skipping 1617 matching lines @@
 
   // We have a name that is not shadowed, followed by a period or #.
   // Consume the identifier, let the caller consume the . or #.
   ConsumeToken();
   return prefix.raw();
 }
 
 
 void Parser::ParseMethodOrConstructor(ClassDesc* members, MemberDesc* method) {
   TRACE_PARSER("ParseMethodOrConstructor");
-  ASSERT(CurrentToken() == Token::kLPAREN || method->IsGetter());
+  // We are at the beginning of the formal parameters list.
+  ASSERT(CurrentToken() == Token::kLPAREN ||
+         CurrentToken() == Token::kLT ||
+         method->IsGetter());
   ASSERT(method->type != NULL);
   ASSERT(current_member_ == method);
 
   if (method->has_var) {
     ReportError(method->name_pos, "keyword var not allowed for methods");
   }
   if (method->has_final) {
     ReportError(method->name_pos, "'final' not allowed for methods");
   }
   if (method->has_abstract && method->has_static) {
     ReportError(method->name_pos,
                 "static method '%s' cannot be abstract",
                 method->name->ToCString());
      }
   if (method->has_const && !method->IsFactoryOrConstructor()) {
     ReportError(method->name_pos, "'const' not allowed for methods");
   }
   if (method->has_abstract && method->IsFactoryOrConstructor()) {
     ReportError(method->name_pos, "constructor cannot be abstract");
   }
   if (method->has_const && method->IsConstructor()) {
     current_class().set_is_const();
   }
 
+  if (CurrentToken() == Token::kLT) {
+    // Parse type parameters, but ignore them.
+    // TODO(hausner): handle type parameters.
+    TokenPosition type_param_pos = TokenPos();
+    if (method->IsFactoryOrConstructor()) {
+      ReportError(method->name_pos, "constructor cannot be generic");
+    }
+    if (method->IsGetter() || method->IsSetter()) {
+      ReportError(type_param_pos, "%s cannot be generic",
+          method->IsGetter() ? "getter" : "setter");
+    }
+    if (!TryParseTypeParameters()) {
+      ReportError(type_param_pos, "error in type parameters");
+    }

[siva, 2016/08/19 20:30:58]

Ditto comment about error being unconditional and we don't seem to complain
about this for the !FLAG_generic_method_syntax case.

[hausner, 2016/08/19 21:40:04]

The error will be printed when the function is actually compiled. If the
function is not used, we silently accept the syntax if it's correct. Is that too
permissive? Maybe. I'll add the error eagerly.

+  }
+
   // Parse the formal parameters.
   const bool are_implicitly_final = method->has_const;
   const bool allow_explicit_default_values = true;
   const TokenPosition formal_param_pos = TokenPos();
   method->params.Clear();
   // Static functions do not have a receiver.
   // The first parameter of a factory is the TypeArguments vector of
   // the type of the instance to be allocated.
   if (!method->has_static || method->IsConstructor()) {
     method->params.AddReceiver(ReceiverType(current_class()), formal_param_pos);
@@ skipping 580 matching lines @@
   } else if ((CurrentToken() == Token::kFACTORY) &&
       (LookaheadToken(1) != Token::kLPAREN)) {
     ConsumeToken();
     if (member.has_static) {
       ReportError("factory method cannot be explicitly marked static");
     }
     member.has_factory = true;
     member.has_static = true;
     // The result type depends on the name of the factory method.
   }
+
   // Optionally parse a type.
   if (CurrentToken() == Token::kVOID) {
     if (member.has_var || member.has_factory) {
       ReportError("void not expected");
     }
     ConsumeToken();
     ASSERT(member.type == NULL);
     member.type = &Object::void_type();
-  } else if (CurrentToken() == Token::kIDENT) {
-    // This is either a type name or the name of a method/constructor/field.
-    if ((member.type == NULL) && !member.has_factory) {
-      // We have not seen a member type yet, so we check if the next
-      // identifier could represent a type before parsing it.
-      Token::Kind follower = LookaheadToken(1);
-      // We have an identifier followed by a 'follower' token.
-      // We either parse a type or assume that no type is specified.
-      if ((follower == Token::kLT) ||  // Parameterized type.
-          (follower == Token::kGET) ||  // Getter following a type.
-          (follower == Token::kSET) ||  // Setter following a type.
-          (follower == Token::kOPERATOR) ||  // Operator following a type.
-          (Token::IsIdentifier(follower)) ||  // Member name following a type.
-          ((follower == Token::kPERIOD) &&    // Qualified class name of type,
-           (LookaheadToken(3) != Token::kLPAREN))) {  // but not a named constr.
-        ASSERT(is_top_level_);
-        // The declared type of fields is never ignored, even in unchecked mode,
-        // because getters and setters could be closurized at some time (not
-        // supported yet).
-        member.type = &AbstractType::ZoneHandle(Z,
-            ParseType(ClassFinalizer::kResolveTypeParameters));
+  } else {
+    bool found_type = false;
+    {
+      // Lookahead to determine whether the next tokens are a return type.
+      TokenPosScope saved_pos(this);
+      if (TryParseReturnType()) {
+        if (IsIdentifier() ||
+           (CurrentToken() == Token::kGET) ||
+           (CurrentToken() == Token::kSET) ||
+           (CurrentToken() == Token::kOPERATOR)) {
+          found_type = true;
+        }
       }
     }
+    if (found_type) {
+      member.type = &AbstractType::ZoneHandle(Z,
+          ParseType(ClassFinalizer::kResolveTypeParameters));
+    }
   }
 
   // Optionally parse a (possibly named) constructor name or factory.
   if (IsIdentifier() &&
       (CurrentLiteral()->Equals(members->class_name()) || member.has_factory)) {
     member.name_pos = TokenPos();
     member.name = CurrentLiteral();  // Unqualified identifier.
     ConsumeToken();
     if (member.has_factory) {
       // The factory name may be qualified, but the first identifier must match
@@ skipping 30 matching lines @@
     if (CurrentToken() == Token::kPERIOD) {
       // Named constructor.
       ConsumeToken();
       member.dict_name = ExpectIdentifier("identifier expected");
       to_concat.Add(*member.dict_name);
     }
     *member.name = Symbols::FromConcatAll(T, to_concat);
     CheckToken(Token::kLPAREN);
   } else if ((CurrentToken() == Token::kGET) && !member.has_var &&
              (LookaheadToken(1) != Token::kLPAREN) &&
+             (LookaheadToken(1) != Token::kLT) &&
              (LookaheadToken(1) != Token::kASSIGN) &&
              (LookaheadToken(1) != Token::kCOMMA)  &&
              (LookaheadToken(1) != Token::kSEMICOLON)) {
     ConsumeToken();
     member.kind = RawFunction::kGetterFunction;
     member.name_pos = this->TokenPos();
     member.name = ExpectIdentifier("identifier expected");
     // If the result type was not specified, it will be set to DynamicType.
   } else if ((CurrentToken() == Token::kSET) && !member.has_var &&
              (LookaheadToken(1) != Token::kLPAREN) &&
+             (LookaheadToken(1) != Token::kLT) &&
              (LookaheadToken(1) != Token::kASSIGN) &&
              (LookaheadToken(1) != Token::kCOMMA)  &&
              (LookaheadToken(1) != Token::kSEMICOLON))  {
     ConsumeToken();
     member.kind = RawFunction::kSetterFunction;
     member.name_pos = this->TokenPos();
     member.name = ExpectIdentifier("identifier expected");
     CheckToken(Token::kLPAREN);
     // The grammar allows a return type, so member.type is not always NULL here.
     // If no return type is specified, the return type of the setter is dynamic.
     if (member.type == NULL) {
       member.type = &Object::dynamic_type();
     }
   } else if ((CurrentToken() == Token::kOPERATOR) && !member.has_var &&
              (LookaheadToken(1) != Token::kLPAREN) &&
              (LookaheadToken(1) != Token::kASSIGN) &&
              (LookaheadToken(1) != Token::kCOMMA)  &&
              (LookaheadToken(1) != Token::kSEMICOLON)) {
+    // TODO(hausner): handle the case of a generic function named 'operator':
+    // eg: T operator<T>(a, b) => ...
     ConsumeToken();
     if (!Token::CanBeOverloaded(CurrentToken())) {
       ReportError("invalid operator overloading");
     }
     if (member.has_static) {
       ReportError("operator overloading functions cannot be static");
     }
     member.operator_token = CurrentToken();
     member.has_operator = true;
     member.kind = RawFunction::kRegularFunction;
     member.name_pos = this->TokenPos();
     member.name = &String::ZoneHandle(Z,
         Symbols::Token(member.operator_token).raw());
     ConsumeToken();
   } else if (IsIdentifier()) {
     member.name = CurrentLiteral();
     member.name_pos = TokenPos();
     ConsumeToken();
   } else {
     ReportError("identifier expected");
   }
 
   ASSERT(member.name != NULL);
-  if (CurrentToken() == Token::kLPAREN || member.IsGetter()) {
+  if (IsParameterPart() || member.IsGetter()) {
     // Constructor or method.
     if (member.type == NULL) {
       member.type = &Object::dynamic_type();
     }
     ASSERT(member.IsFactory() == member.has_factory);
     ParseMethodOrConstructor(members, &member);
   } else if (CurrentToken() ==  Token::kSEMICOLON ||
              CurrentToken() == Token::kCOMMA ||
              CurrentToken() == Token::kASSIGN) {
     // Field definition.
@@ skipping 634 matching lines @@
 
 
 // Look ahead to detect if we are seeing ident [ TypeParameters ] "(".
 // We need this lookahead to distinguish between the optional return type
 // and the alias name of a function type alias.
 // Token position remains unchanged.
 bool Parser::IsFunctionTypeAliasName() {
   if (IsIdentifier() && (LookaheadToken(1) == Token::kLPAREN)) {
     return true;
   }
-  const TokenPosition saved_pos = TokenPos();
-  bool is_alias_name = false;
+  const TokenPosScope saved_pos(this);
   if (IsIdentifier() && (LookaheadToken(1) == Token::kLT)) {
     ConsumeToken();
     if (TryParseTypeParameters() && (CurrentToken() == Token::kLPAREN)) {
-      is_alias_name = true;
+      return true;
     }
   }
-  SetPosition(saved_pos);
-  return is_alias_name;
+  return false;
 }
 
 
 // Look ahead to detect if we are seeing ident [ TypeParameters ] "=".
 // Token position remains unchanged.
 bool Parser::IsMixinAppAlias() {
   if (IsIdentifier() && (LookaheadToken(1) == Token::kASSIGN)) {
     return true;
   }
-  const TokenPosition saved_pos = TokenPos();
-  bool is_mixin_def = false;
+  const TokenPosScope saved_pos(this);
   if (IsIdentifier() && (LookaheadToken(1) == Token::kLT)) {
     ConsumeToken();
     if (TryParseTypeParameters() && (CurrentToken() == Token::kASSIGN)) {
-      is_mixin_def = true;
+      return true;
     }
   }
-  SetPosition(saved_pos);
-  return is_mixin_def;
+  return false;
 }
 
 
 void Parser::ParseTypedef(const GrowableObjectArray& pending_classes,
                           const Object& tl_owner,
                           TokenPosition metadata_pos) {
   TRACE_PARSER("ParseTypedef");
   TokenPosition declaration_pos =
       metadata_pos.IsReal() ? metadata_pos : TokenPos();
   ExpectToken(Token::kTYPEDEF);
@@ skipping 86 matching lines @@
   ASSERT(!function_type_alias.is_finalized());
   pending_classes.Add(function_type_alias, Heap::kOld);
   if (FLAG_enable_mirrors && metadata_pos.IsReal()) {
     library_.AddClassMetadata(function_type_alias,
                               tl_owner,
                               metadata_pos);
   }
 }
 
 
-// Consumes exactly one right angle bracket. If the current token is a single
-// bracket token, it is consumed normally. However, if it is a double or triple
-// bracket, it is replaced by a single or double bracket token without
+// Consumes exactly one right angle bracket. If the current token is
+// a single bracket token, it is consumed normally. However, if it is
+// a double bracket, it is replaced by a single bracket token without
 // incrementing the token index.
 void Parser::ConsumeRightAngleBracket() {
   if (token_kind_ == Token::kGT) {
     ConsumeToken();
   } else if (token_kind_ == Token::kSHR) {
     token_kind_ = Token::kGT;
   } else {
     UNREACHABLE();
   }
 }
 
 
 bool Parser::IsPatchAnnotation(TokenPosition pos) {
   if (pos == TokenPosition::kNoSource) {
     return false;
   }
-  TokenPosition saved_pos = TokenPos();
+  TokenPosScope saved_pos(this);
   SetPosition(pos);
   ExpectToken(Token::kAT);
-  bool is_patch = IsSymbol(Symbols::Patch());
-  SetPosition(saved_pos);
-  return is_patch;
+  return IsSymbol(Symbols::Patch());
 }
 
 
 TokenPosition Parser::SkipMetadata() {
   if (CurrentToken() != Token::kAT) {
     return TokenPosition::kNoSource;
   }
   TokenPosition metadata_pos = TokenPos();
   while (CurrentToken() == Token::kAT) {
     ConsumeToken();
@@ skipping 337 matching lines @@
     metadata_pos = TokenPosition::kNoSource;
   } else if (CurrentToken() == Token::kEXTERNAL) {
     ConsumeToken();
     is_external = true;
   }
   if (CurrentToken() == Token::kVOID) {
     ConsumeToken();
     result_type = Type::VoidType();
   } else {
     // Parse optional type.
-    if ((CurrentToken() == Token::kIDENT) &&
-        (LookaheadToken(1) != Token::kLPAREN)) {
+    if (IsFunctionReturnType()) {
       result_type = ParseType(ClassFinalizer::kResolveTypeParameters);
     }
   }
   const TokenPosition name_pos = TokenPos();
   const String& func_name = *ExpectIdentifier("function name expected");
 
   bool found = library_.LookupLocalObject(func_name) != Object::null();
   if (found && !is_patch) {
     ReportError(name_pos, "'%s' is already defined", func_name.ToCString());
   } else if (!found && is_patch) {
     ReportError(name_pos, "missing '%s' cannot be patched",
                 func_name.ToCString());
   }
   String& accessor_name = String::Handle(Z, Field::GetterName(func_name));
   if (library_.LookupLocalObject(accessor_name) != Object::null()) {
     ReportError(name_pos, "'%s' is already defined as getter",
                 func_name.ToCString());
   }
   // A setter named x= may co-exist with a function named x, thus we do
   // not need to check setters.
 
+  if (CurrentToken() == Token::kLT) {
+    // Type parameters of generic function.
+    // TODO(hausner): handle type parameters.
+    TokenPosition type_arg_pos = TokenPos();
+    if (!TryParseTypeParameters()) {
+      ReportError(type_arg_pos, "error in type parameters");
+    }

[siva, 2016/08/19 20:30:58]

Ditto comment about the error being unconditional (FLAG_generic_method_syntax)

[hausner, 2016/08/19 21:40:04]

Done.

+    if (!FLAG_generic_method_syntax) {
+      ReportError(type_arg_pos, "generic functions not supported");
+    }
+  }
+
   CheckToken(Token::kLPAREN);
   const TokenPosition function_pos = TokenPos();
   ParamList params;
   const bool allow_explicit_default_values = true;
   ParseFormalParameterList(allow_explicit_default_values, false, &params);
 
   const TokenPosition modifier_pos = TokenPos();
   RawFunction::AsyncModifier func_modifier = ParseFunctionModifier();
 
   TokenPosition function_end_pos = function_pos;
@@ skipping 2072 matching lines @@
   TRACE_PARSER("ParseFunctionStatement");
   AbstractType& result_type = AbstractType::Handle(Z);
   const String* variable_name = NULL;
   const String* function_name = NULL;
 
   result_type = Type::DynamicType();
 
   const TokenPosition function_pos = TokenPos();
   TokenPosition metadata_pos = TokenPosition::kNoSource;
   if (is_literal) {
-    ASSERT(CurrentToken() == Token::kLPAREN);
+    ASSERT(CurrentToken() == Token::kLPAREN || CurrentToken() == Token::kLT);
     function_name = &Symbols::AnonymousClosure();
   } else {
     metadata_pos = SkipMetadata();
     if (CurrentToken() == Token::kVOID) {
       ConsumeToken();
       result_type = Type::VoidType();
-    } else if ((CurrentToken() == Token::kIDENT) &&
-               (LookaheadToken(1) != Token::kLPAREN)) {
+    } else if (IsFunctionReturnType()) {
       result_type = ParseType(ClassFinalizer::kCanonicalize);
     }
     const TokenPosition name_pos = TokenPos();
     variable_name = ExpectIdentifier("function name expected");
     function_name = variable_name;
 
     // Check that the function name has not been referenced
     // before this declaration.
     ASSERT(current_block_ != NULL);
     const TokenPosition previous_pos =
         current_block_->scope->PreviousReferencePos(*function_name);
     if (previous_pos.IsReal()) {
       ASSERT(!script_.IsNull());
       intptr_t line_number;
       script_.GetTokenLocation(previous_pos, &line_number, NULL);
       ReportError(name_pos,
                   "identifier '%s' previously used in line %" Pd "",
                   function_name->ToCString(),
                   line_number);
     }
   }
+
+  if (CurrentToken() == Token::kLT) {
+    TokenPosition type_arg_pos = TokenPos();
+    // TODO(hausner): handle type parameters of generic function.
+    if (!TryParseTypeParameters()) {
+      ReportError(type_arg_pos, "error in type parameters");
+    }

[siva, 2016/08/19 20:30:58]

Ditto comment about the error being unconditional (FLAG_generic_method_syntax)

[hausner, 2016/08/19 21:40:04]

Done.

+    if (!FLAG_generic_method_syntax) {
+      ReportError(type_arg_pos, "generic functions not supported");
+    }
+  }
+
   CheckToken(Token::kLPAREN);
 
   // Check whether we have parsed this closure function before, in a previous
   // compilation. If so, reuse the function object, else create a new one
   // and register it in the current class.
   // Note that we cannot share the same closure function between the closurized
   // and non-closurized versions of the same parent function.
   Function& function = Function::ZoneHandle(Z);
   bool found_func = true;
   // TODO(hausner): There could be two different closures at the given
@@ skipping 152 matching lines @@
   } else {
     AstNode* initialization = new(Z) StoreLocalNode(
         function_pos, function_variable, closure);
     return initialization;
   }
 }
 
 
 // Returns true if the current and next tokens can be parsed as type
 // parameters. Current token position is not saved and restored.
 bool Parser::TryParseTypeParameters() {

[siva, 2016/08/19 20:30:58]

seems to be always called after a CurrentToken() == Token::kLT check,
can we just turn this 'if' check into an assertion
ASSERT(CurrentToken() == Token::kLT);

[hausner, 2016/08/19 21:40:04]

Yes, that makes sense.

   if (CurrentToken() == Token::kLT) {
     // We are possibly looking at type parameters. Find closing ">".
     int nesting_level = 0;
     do {
-      if (CurrentToken() == Token::kLT) {
+      Token::Kind ct = CurrentToken();
+      if (ct == Token::kLT) {
         nesting_level++;
-      } else if (CurrentToken() == Token::kGT) {
+      } else if (ct == Token::kGT) {
         nesting_level--;
-      } else if (CurrentToken() == Token::kSHR) {
+      } else if (ct == Token::kSHR) {
         nesting_level -= 2;
-      } else if (CurrentToken() == Token::kIDENT) {
+      } else if (ct == Token::kIDENT) {
         // Check to see if it is a qualified identifier.
         if (LookaheadToken(1) == Token::kPERIOD) {
           // Consume the identifier, the period will be consumed below.
           ConsumeToken();
         }
-      } else if (CurrentToken() != Token::kCOMMA &&
-                 CurrentToken() != Token::kEXTENDS) {
+      } else if ((ct != Token::kCOMMA) &&
+                 (ct != Token::kEXTENDS) &&
+                 (!FLAG_generic_method_syntax || (ct != Token::kSUPER))) {
         // We are looking at something other than type parameters.
         return false;
       }
       ConsumeToken();
     } while (nesting_level > 0);
     if (nesting_level < 0) {
       return false;
     }
   }

[siva, 2016/08/19 20:30:58]

This will return true for something like
<<>,<>> or <> or <<>> or <<>A,>
is that expected?

[hausner, 2016/08/19 21:40:04]

Yes, that is expected. This function is an approximation to parsing type
parameters. It checks that the next few tokens could be type parameters. If
there is a syntax error, it will be caught by the code that actually parses and
processes the type parameters. (This code does not yet exist for functions, only
for classes.)

[hausner, 2016/08/19 23:12:00]

One small correction: <>> (or anything else with too many closing brackets) will
return false. The nesting level would end up being -1, which is caught in line
8057.

   return true;
 }
 
 
+// Returns true if the next tokens can be parsed as type parameters.
+bool Parser::IsTypeParameters() {
+  if (CurrentToken() == Token::kLT) {
+    TokenPosScope param_pos(this);
+    if (!TryParseTypeParameters()) {
+      return false;
+    }
+    return true;
+  }
+  return false;
+}
+
+
+// Returns true if the next tokens are [ typeParameters ] '('.
+bool Parser::IsParameterPart() {
+  if (CurrentToken() == Token::kLPAREN) {
+    return true;
+  }
+  if (CurrentToken() == Token::kLT) {
+    TokenPosScope type_arg_pos(this);
+    if (!TryParseTypeParameters()) {
+      return false;
+    }
+    return CurrentToken() == Token::kLPAREN;
+  }
+  return false;
+}
+
+
+// Returns true if the current and next tokens can be parsed as type
+// arguments. Current token position is not saved and restored.
+bool Parser::TryParseTypeArguments() {
+  if (CurrentToken() == Token::kLT) {

[siva, 2016/08/19 20:30:58]

Ditto comment about turning this 'if' into an ASSERT

[hausner, 2016/08/19 21:40:04]

Done.

+    // We are possibly looking at type arguments. Find closing ">".
+    int nesting_level = 0;
+    do {
+      Token::Kind ct = CurrentToken();
+      if (ct == Token::kLT) {
+        nesting_level++;
+      } else if (ct == Token::kGT) {
+        nesting_level--;
+      } else if (ct == Token::kSHR) {
+        nesting_level -= 2;
+      } else if (ct == Token::kIDENT) {
+        // Check to see if it is a qualified identifier.
+        if (ct == Token::kPERIOD) {

[siva, 2016/08/19 20:30:58]

ct was kIDENT based on the check above, how will (ct == kPERIOD) ever be true.

Probably needs:
 if (LookahreadToken(1) == Token::kPERIOD) {
    ConsumeToken();
 }

[hausner, 2016/08/19 21:40:04]

Nice catch, thank you.

+          // Consume the identifier, the period will be consumed below.
+          ConsumeToken();
+        }
+      } else if (ct != Token::kCOMMA) {
+        return false;
+      }
+      ConsumeToken();
+    } while (nesting_level > 0);
+    if (nesting_level < 0) {
+      return false;
+    }
+  }
+  return true;

[siva, 2016/08/19 20:30:58]

This will return true for something like
<<>,<>> or <> or <<>> or <<>A,>
is that expected?

[hausner, 2016/08/19 21:40:04]

Yes, for the same reason as explained above. ParseTypeArguments() will catch
those errors.

This function is mainly a way to peek ahead so the parser can determine what the
next tokens most likely are.

+}
+
+
+// Returns true if the next tokens are [ typeArguments ] '('.
+bool Parser::IsArgumentPart() {
+  if (CurrentToken() == Token::kLPAREN) {
+    return true;
+  }
+  if (CurrentToken() == Token::kLT) {
+    TokenPosScope type_arg_pos(this);
+    if (!TryParseTypeArguments()) {
+      return false;
+    }
+    return CurrentToken() == Token::kLPAREN;
+  }
+  return false;
+}
+
+
 bool Parser::IsSimpleLiteral(const AbstractType& type, Instance* value) {
   // Assigning null never causes a type error.
   if (CurrentToken() == Token::kNULL) {
     *value = Instance::null();
     return true;
   }
   // If the type of the const field is guaranteed to be instantiated once
   // resolved at class finalization time, and if the type of the literal is one
   // of int, double, String, or bool, then preset the field with the value and
   // perform the type check (in checked mode only) at finalization time.
@@ skipping 143 matching lines @@
           (CurrentToken() == Token::kASSIGN)) {
         is_var_decl = true;
       }
     }
   }
   SetPosition(saved_pos);
   return is_var_decl;
 }
 
 
+// Look ahead to see if the following tokens are a return type followed
+// by an identifier.
+bool Parser::IsFunctionReturnType() {
+  TokenPosScope decl_pos(this);
+  if (TryParseReturnType()) {
+    if (IsIdentifier()) {
+      // Return type followed by function name.
+      return true;
+    }
+  }
+  return false;
+}
+
+
 // Look ahead to detect whether the next tokens should be parsed as
 // a function declaration. Token position remains unchanged.
 bool Parser::IsFunctionDeclaration() {
-  const TokenPosition saved_pos = TokenPos();
   bool is_external = false;
+  TokenPosScope decl_pos(this);
   SkipMetadata();
-  if (is_top_level_ && (CurrentToken() == Token::kEXTERNAL)) {
-    // Skip over 'external' for top-level function declarations.
-    is_external = true;
-    ConsumeToken();
+  if (is_top_level_) {
+    if (is_patch_source() &&
+        (CurrentToken() == Token::kIDENT) &&
+        CurrentLiteral()->Equals("patch") &&
+        (LookaheadToken(1) != Token::kLPAREN)) {
+      // Skip over 'patch' for top-level function declarations in patch sources.

[siva, 2016/08/19 20:30:58]

I thought we removed support for 'patch' going forward.

[hausner, 2016/08/19 21:40:04]

Yes. This probably reappeared because I copied this function before the patch
removal CL, and the merge didn't catch it later.

Removed.

+      ConsumeToken();
+    } else if (CurrentToken() == Token::kEXTERNAL) {
+      // Skip over 'external' for top-level function declarations.
+      is_external = true;
+      ConsumeToken();
+    }
   }
-  if (IsIdentifier() && (LookaheadToken(1) == Token::kLPAREN)) {
-    // Possibly a function without explicit return type.
-    ConsumeToken();  // Consume function identifier.
-  } else if (TryParseReturnType()) {
+  const TokenPosition type_or_name_pos = TokenPos();
+  if (TryParseReturnType()) {
     if (!IsIdentifier()) {
-      SetPosition(saved_pos);
-      return false;
+      SetPosition(type_or_name_pos);
     }
-    ConsumeToken();  // Consume function identifier.
   } else {
-    SetPosition(saved_pos);
+    SetPosition(type_or_name_pos);
+  }
+  // Check for function name followed by optional type parameters.
+  if (!IsIdentifier()) {
     return false;
   }
+  ConsumeToken();
+  if ((CurrentToken() == Token::kLT) && !TryParseTypeParameters()) {
+    return false;
+  }
+
+  // Optional type, function name and optinal type parameters are parsed.
+  if (CurrentToken() != Token::kLPAREN) {
+    return false;
+  }
+
   // Check parameter list and the following token.
-  if (CurrentToken() == Token::kLPAREN) {
-    SkipToMatchingParenthesis();
-    if ((CurrentToken() == Token::kLBRACE) ||
-        (CurrentToken() == Token::kARROW) ||
-        (is_top_level_ && IsSymbol(Symbols::Native())) ||
-        is_external ||
-        IsSymbol(Symbols::Async()) ||
-        IsSymbol(Symbols::Sync())) {
-      SetPosition(saved_pos);
-      return true;
-    }
+  SkipToMatchingParenthesis();
+  if ((CurrentToken() == Token::kLBRACE) ||
+      (CurrentToken() == Token::kARROW) ||
+      (is_top_level_ && IsSymbol(Symbols::Native())) ||
+      is_external ||
+      IsSymbol(Symbols::Async()) ||
+      IsSymbol(Symbols::Sync())) {
+    return true;
   }
-  SetPosition(saved_pos);
   return false;
 }
 
 
 bool Parser::IsTopLevelAccessor() {
-  const TokenPosition saved_pos = TokenPos();
+  const TokenPosScope saved_pos(this);
   if (CurrentToken() == Token::kEXTERNAL) {
     ConsumeToken();
   }
   if ((CurrentToken() == Token::kGET) || (CurrentToken() == Token::kSET)) {
-    SetPosition(saved_pos);
     return true;
   }
   if (TryParseReturnType()) {
     if ((CurrentToken() == Token::kGET) || (CurrentToken() == Token::kSET)) {
       if (Token::IsIdentifier(LookaheadToken(1))) {  // Accessor name.
-        SetPosition(saved_pos);
         return true;
       }
     }
   }
-  SetPosition(saved_pos);
   return false;
 }
 
 
 bool Parser::IsFunctionLiteral() {
-  if (CurrentToken() != Token::kLPAREN || !allow_function_literals_) {
+  if (!allow_function_literals_) {
     return false;
   }
-  const TokenPosition saved_pos = TokenPos();
-  bool is_function_literal = false;
-  SkipToMatchingParenthesis();
-  ParseFunctionModifier();
-  if ((CurrentToken() == Token::kLBRACE) ||
-      (CurrentToken() == Token::kARROW)) {
-    is_function_literal = true;
+  if ((CurrentToken() == Token::kLPAREN) || (CurrentToken() == Token::kLT)) {
+    TokenPosScope saved_pos(this);
+    if ((CurrentToken() == Token::kLT) && !TryParseTypeParameters()) {
+      return false;
+    }
+    if (CurrentToken() != Token::kLPAREN) {
+      return false;
+    }
+    SkipToMatchingParenthesis();
+    ParseFunctionModifier();
+    if ((CurrentToken() == Token::kLBRACE) ||
+        (CurrentToken() == Token::kARROW)) {
+       return true;
+    }
   }
-  SetPosition(saved_pos);
-  return is_function_literal;
+  return false;
 }
 
 
 // Current token position is the token after the opening ( of the for
 // statement. Returns true if we recognize a for ( .. in expr)
 // statement.
 bool Parser::IsForInStatement() {
-  const TokenPosition saved_pos = TokenPos();
-  bool result = false;
+  const TokenPosScope saved_pos(this);
   // Allow const modifier as well when recognizing a for-in statement
   // pattern. We will get an error later if the loop variable is
   // declared with const.
   if (CurrentToken() == Token::kVAR ||
       CurrentToken() == Token::kFINAL ||
       CurrentToken() == Token::kCONST) {
     ConsumeToken();
   }
   if (IsIdentifier()) {
     if (LookaheadToken(1) == Token::kIN) {
-      result = true;
+      return true;
     } else if (TryParseOptionalType()) {
       if (IsIdentifier()) {
         ConsumeToken();
       }
-      result = (CurrentToken() == Token::kIN);
+      return CurrentToken() == Token::kIN;
     }
   }
-  SetPosition(saved_pos);
-  return result;
+  return false;
 }
 
 
 static bool ContainsAbruptCompletingStatement(SequenceNode* seq);
 
 static bool IsAbruptCompleting(AstNode* statement) {
   return statement->IsReturnNode() ||
          statement->IsJumpNode()   ||
          statement->IsThrowNode()  ||
          (statement->IsSequenceNode() &&
@@ skipping 3379 matching lines @@
           left = new(Z) TypeNode(primary->token_pos(), type_parameter);
         } else {
           // Super field access handled in ParseSuperFieldAccess(),
           // super calls handled in ParseSuperCall().
           ASSERT(!primary_node->IsSuper());
           left = LoadFieldIfUnresolved(left);
         }
       }
       const TokenPosition ident_pos = TokenPos();
       String* ident = ExpectIdentifier("identifier expected");
-      if (CurrentToken() == Token::kLPAREN) {
-        // Identifier followed by a opening paren: method call.
+      if (IsArgumentPart()) {
+        // Identifier followed by optional type arguments and opening paren:
+        // method call.
+        if (CurrentToken() == Token::kLT) {
+          // Type arguments.
+          if (!FLAG_generic_method_syntax) {
+            ReportError("generic type arguments not supported.");
+          }
+          // TODO(hausner): handle type arguments.
+          ParseTypeArguments(ClassFinalizer::kIgnore);
+        }
         if (left->IsPrimaryNode() &&
             left->AsPrimaryNode()->primary().IsClass()) {
           // Static method call prefixed with class name.
           const Class& cls = Class::Cast(left->AsPrimaryNode()->primary());
           selector = ParseStaticCall(cls, *ident, ident_pos);
         } else {
           selector = ParseInstanceCall(left, *ident, ident_pos, is_conditional);
         }
       } else {
         // Field access.
@@ skipping 71 matching lines @@
               TypeParameter::Cast(primary_node->primary()),
               ClassFinalizer::kCanonicalize);
           ASSERT(!type_parameter.IsMalformed());
           array = new(Z) TypeNode(primary_pos, type_parameter);
         } else {
           UNREACHABLE();  // Internal parser error.
         }
       }
       selector =  new(Z) LoadIndexedNode(
           bracket_pos, array, index, Class::ZoneHandle(Z));
-    } else if (CurrentToken() == Token::kLPAREN) {
+    } else if (IsArgumentPart()) {
+      if (CurrentToken() == Token::kLT) {
+        // Type arguments.
+        if (!FLAG_generic_method_syntax) {
+          ReportError("generic type arguments not supported.");
+        }
+        // TODO(hausner): handle type arguments.
+        ParseTypeArguments(ClassFinalizer::kIgnore);
+      }
       if (left->IsPrimaryNode()) {
         PrimaryNode* primary_node = left->AsPrimaryNode();
         const TokenPosition primary_pos = primary_node->token_pos();
         if (primary_node->primary().IsFunction()) {
           const Function& func = Function::Cast(primary_node->primary());
           const String& func_name = String::ZoneHandle(Z, func.name());
           if (func.is_static()) {
             // Parse static function call.
             Class& cls = Class::Handle(Z, func.Owner());
             selector = ParseStaticCall(cls, func_name, primary_pos);
@@ skipping 2600 matching lines @@
       ConsumeToken();
     } else {
       break;
     }
   }
   ExpectToken(Token::kRBRACE);
 }
 
 
 void Parser::SkipActualParameters() {
+  if (CurrentToken() == Token::kLT) {
+    SkipTypeArguments();
+  }
   ExpectToken(Token::kLPAREN);
   while (CurrentToken() != Token::kRPAREN) {
     if (IsIdentifier() && (LookaheadToken(1) == Token::kCOLON)) {
       // Named actual parameter.
       ConsumeToken();
       ConsumeToken();
     }
     SkipNestedExpr();
     if (CurrentToken() == Token::kCOMMA) {
       ConsumeToken();
@@ skipping 147 matching lines @@
         ExpectIdentifier("identifier or [ expected after ..");
       }
     } else if ((current_token == Token::kPERIOD) ||
         (current_token == Token::kQM_PERIOD)) {
       ConsumeToken();
       ExpectIdentifier("identifier expected");
     } else if (current_token == Token::kLBRACK) {
       ConsumeToken();
       SkipNestedExpr();
       ExpectToken(Token::kRBRACK);
-    } else if (current_token == Token::kLPAREN) {
+    } else if (IsArgumentPart()) {
       SkipActualParameters();
     } else {
       break;
     }
   }
 }
 
 void Parser::SkipPostfixExpr() {
   SkipPrimary();
   if (CurrentToken() == Token::kHASH) {
@@ skipping 183 matching lines @@
     const ArgumentListNode& function_args,
     const LocalVariable* temp_for_last_arg,
     bool is_super_invocation) {
   UNREACHABLE();
   return NULL;
 }
 
 }  // namespace dart
 
 #endif  // DART_PRECOMPILED_RUNTIME