Move the begin token of explicit functions to the start of the function declaration.

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 "lib/invocation_mirror.h"
 #include "vm/bigint_operations.h"
 #include "vm/bootstrap.h"
 #include "vm/class_finalizer.h"
@@ skipping 508 matching lines @@
   bool has_final;
   bool has_const;
   bool has_static;
   bool has_var;
   bool has_factory;
   bool has_operator;
   intptr_t metadata_pos;
   Token::Kind operator_token;
   const AbstractType* type;
   intptr_t name_pos;
+  intptr_t decl_begin_pos;
   String* name;
   // For constructors: NULL or name of redirected to constructor.
   String* redirect_name;
   // For constructors: NULL for unnamed constructor,
   // identifier after classname for named constructors.
   String* constructor_name;
   ParamList params;
   RawFunction::Kind kind;
 };
 
@@ skipping 218 matching lines @@
   SequenceNode* node_sequence = NULL;
   Array& default_parameter_values = Array::ZoneHandle(isolate, Array::null());
   switch (func.kind()) {
     case RawFunction::kRegularFunction:
     case RawFunction::kClosureFunction:
     case RawFunction::kGetterFunction:
     case RawFunction::kSetterFunction:
     case RawFunction::kConstructor:
       // The call to a redirecting factory is redirected.
       ASSERT(!func.IsRedirectingFactory());
+      parser.SkipFunctionPreamble(func);
       node_sequence = parser.ParseFunc(func, default_parameter_values);
       break;
     case RawFunction::kImplicitGetter:
       ASSERT(!func.is_static());
       node_sequence = parser.ParseInstanceGetter(func);
       break;
     case RawFunction::kImplicitSetter:
       ASSERT(!func.is_static());
       node_sequence = parser.ParseInstanceSetter(func);
       break;
@@ skipping 633 matching lines @@
 
       // The field 'is_static' has no meaning for signature functions.
       const Function& signature_function = Function::Handle(
           Function::New(*parameter.name,
                         RawFunction::kSignatureFunction,
                         /* is_static = */ false,
                         /* is_const = */ false,
                         /* is_abstract = */ false,
                         /* is_external = */ false,
                         current_class(),
                         parameter.name_pos));

[hausner, 2013/08/13 15:40:11]

In the case of typedef, you change the token position of the signature function
to the beginning of the declaration. Here you leave the position unchanged to
point to the name, not the beginning of the type declaration. Should this not be
consistent?

[Michael Lippautz (Google), 2013/08/13 18:07:34]

This is about closure parameters, so the beginning of the declaration is
actually the opening paren. I don't think we need to change this (?)

Reverted the change on typedef begin token though, since there's no need to have
it pointing at the begin of the declaration (for now).

       signature_function.set_result_type(result_type);
       AddFormalParamsToFunction(&func_params, signature_function);
       const String& signature = String::Handle(signature_function.Signature());
       // Lookup the signature class, i.e. the class whose name is the signature.
       // We only lookup in the current library, but not in its imports, and only
       // create a new canonical signature class if it does not exist yet.
       Class& signature_class = Class::ZoneHandle(
           library_.LookupLocalClass(signature));
       if (signature_class.IsNull()) {
         signature_class = Class::NewSignatureClass(signature,
@@ skipping 1397 matching lines @@
         }
       }
     }
   }
 }
 
 
 void Parser::ParseMethodOrConstructor(ClassDesc* members, MemberDesc* method) {
   TRACE_PARSER("ParseMethodOrConstructor");
   ASSERT(CurrentToken() == Token::kLPAREN || method->IsGetter());
-  intptr_t method_pos = this->TokenPos();
   ASSERT(method->type != NULL);
   ASSERT(method->name_pos > 0);
   ASSERT(current_member_ == method);
 
   if (method->has_var) {
     ErrorMsg(method->name_pos, "keyword var not allowed for methods");
   }
   if (method->has_final) {
     ErrorMsg(method->name_pos, "'final' not allowed for methods");
   }
@@ skipping 233 matching lines @@
     function_kind = RawFunction::kRegularFunction;
   }
   Function& func = Function::Handle(
       Function::New(*method->name,
                     function_kind,
                     method->has_static,
                     method->has_const,
                     method->has_abstract,
                     method->has_external,
                     current_class(),
-                    method_pos));
+                    method->decl_begin_pos));
   func.set_result_type(*method->type);
   func.set_end_token_pos(method_end_pos);
   if (method->metadata_pos > 0) {
     library_.AddFunctionMetadata(func, method->metadata_pos);
   }
 
   // If this method is a redirecting factory, set the redirection information.
   if (!redirection_type.IsNull()) {
     ASSERT(func.IsFactory());
     func.SetRedirectionType(redirection_type);
@@ skipping 175 matching lines @@
   }
 }
 
 
 void Parser::ParseClassMemberDefinition(ClassDesc* members,
                                         intptr_t metadata_pos) {
   TRACE_PARSER("ParseClassMemberDefinition");
   MemberDesc member;
   current_member_ = &member;
   member.metadata_pos = metadata_pos;
+  member.decl_begin_pos = TokenPos();
   if ((CurrentToken() == Token::kEXTERNAL) &&
       (LookaheadToken(1) != Token::kLPAREN)) {
     ConsumeToken();
     member.has_external = true;
   }
   if ((CurrentToken() == Token::kSTATIC) &&
       (LookaheadToken(1) != Token::kLPAREN)) {
     ConsumeToken();
     member.has_static = true;
   }
@@ skipping 569 matching lines @@
     }
   }
   SetPosition(saved_pos);
   return is_mixin_def;
 }
 
 
 void Parser::ParseTypedef(const GrowableObjectArray& pending_classes) {
   TRACE_PARSER("ParseTypedef");
   ExpectToken(Token::kTYPEDEF);
+  const intptr_t decl_begin_pos = TokenPos();
 
   if (IsMixinTypedef()) {
     ParseMixinTypedef(pending_classes);
     return;
   }
 
   // Parse the result type of the function type.
   AbstractType& result_type = Type::Handle(Type::DynamicType());
   if (CurrentToken() == Token::kVOID) {
     ConsumeToken();
@@ skipping 53 matching lines @@
   ExpectSemicolon();
   // The field 'is_static' has no meaning for signature functions.
   Function& signature_function = Function::Handle(
       Function::New(*alias_name,
                     RawFunction::kSignatureFunction,
                     /* is_static = */ false,
                     /* is_const = */ false,
                     /* is_abstract = */ false,
                     /* is_external = */ false,
                     function_type_alias,
-                    alias_name_pos));
+                    decl_begin_pos));
   signature_function.set_result_type(result_type);
   AddFormalParamsToFunction(&func_params, signature_function);
 
   // Patch the signature function in the signature class.
   function_type_alias.PatchSignatureFunction(signature_function);
 
   const String& signature = String::Handle(signature_function.Signature());
   if (FLAG_trace_parser) {
     OS::Print("TopLevel parsing function type alias '%s'\n",
               signature.ToCString());
@@ skipping 369 matching lines @@
     } else {
       ExpectSemicolon();  // Reports error.
     }
   }
 }
 
 
 void Parser::ParseTopLevelFunction(TopLevel* top_level,
                                    intptr_t metadata_pos) {
   TRACE_PARSER("ParseTopLevelFunction");
+  const intptr_t decl_begin_pos = TokenPos();
   AbstractType& result_type = Type::Handle(Type::DynamicType());
   const bool is_static = true;
   bool is_external = false;
   bool is_patch = false;
   if (is_patch_source() &&
       (CurrentToken() == Token::kIDENT) &&
       CurrentLiteral()->Equals("patch") &&
       (LookaheadToken(1) != Token::kLPAREN)) {
     ConsumeToken();
     is_patch = true;
@@ skipping 53 matching lines @@
     ErrorMsg("function block expected");
   }
   Function& func = Function::Handle(
       Function::New(func_name,
                     RawFunction::kRegularFunction,
                     is_static,
                     /* is_const = */ false,
                     /* is_abstract = */ false,
                     is_external,
                     current_class(),
-                    function_pos));
+                    decl_begin_pos));
   func.set_result_type(result_type);
   func.set_end_token_pos(function_end_pos);
   AddFormalParamsToFunction(&params, func);
   top_level->functions.Add(func);
   if (!is_patch) {
     library_.AddObject(func, func_name);
   } else {
     library_.ReplaceObject(func, func_name);
   }
   if (metadata_pos >= 0) {
     library_.AddFunctionMetadata(func, metadata_pos);
   }
 }
 
 
 void Parser::ParseTopLevelAccessor(TopLevel* top_level,
                                    intptr_t metadata_pos) {
   TRACE_PARSER("ParseTopLevelAccessor");
+  const intptr_t decl_begin_pos = TokenPos();
   const bool is_static = true;
   bool is_external = false;
   bool is_patch = false;
   AbstractType& result_type = AbstractType::Handle();
   if (is_patch_source() &&
       (CurrentToken() == Token::kIDENT) &&
       CurrentLiteral()->Equals("patch")) {
     ConsumeToken();
     is_patch = true;
   } else if (CurrentToken() == Token::kEXTERNAL) {
@@ skipping 86 matching lines @@
   }
   Function& func = Function::Handle(
       Function::New(accessor_name,
                     is_getter? RawFunction::kGetterFunction :
                                RawFunction::kSetterFunction,
                     is_static,
                     /* is_const = */ false,
                     /* is_abstract = */ false,
                     is_external,
                     current_class(),
-                    accessor_pos));
+                    decl_begin_pos));
   func.set_result_type(result_type);
   func.set_end_token_pos(accessor_end_pos);
   AddFormalParamsToFunction(&params, func);
   top_level->functions.Add(func);
   if (!is_patch) {
     library_.AddObject(func, accessor_name);
   } else {
     library_.ReplaceObject(func, accessor_name);
   }
   if (metadata_pos >= 0) {
@@ skipping 719 matching lines @@
   bool is_new_closure = false;
   // TODO(hausner): There could be two different closures at the given
   // function_pos, one enclosed in a closurized function and one enclosed in the
   // non-closurized version of this same function.
   function = current_class().LookupClosureFunction(function_pos);
   if (function.IsNull() || (function.token_pos() != function_pos) ||
       (function.parent_function() != innermost_function().raw())) {
     is_new_closure = true;
     function = Function::NewClosureFunction(*function_name,
                                             innermost_function(),
                                             function_pos);

[hausner, 2013/08/13 15:40:11]

This case is not covered with your change. function_pos points to the opening
paren.

[Michael Lippautz (Google), 2013/08/13 18:07:34]

Did miss this one. Done.

     function.set_result_type(result_type);
     current_class().AddClosureFunction(function);
   }
 
   // The function type needs to be finalized at compile time, since the closure
   // may be type checked at run time when assigned to a function variable,
   // passed as a function argument, or returned as a function result.
 
   LocalVariable* function_variable = NULL;
   Type& function_type = Type::ZoneHandle();
@@ skipping 4745 matching lines @@
   }
   if (CurrentToken() == Token::kLBRACE) {
     SkipBlock();
   } else if (CurrentToken() == Token::kARROW) {
     ConsumeToken();
     SkipExpr();
   }
 }
 
 
+// Skips function/method/constructor/getter/setter preambles until the formal
+// parameter list. It is enough to skip the tokens, since we have already
+// previously parsed the function.
+void Parser::SkipFunctionPreamble(const Function& func) {
+  if (func.IsImplicitConstructor()) return;
+  while (true) {
+    if (CurrentToken() == Token::kLPAREN ||
+        CurrentToken() == Token::kARROW ||
+        CurrentToken() == Token::kSEMICOLON ||
+        CurrentToken() == Token::kLBRACE) {
+      return;
+    }
+    // Case handles "native" keyword, but also return types of form
+    // native.SomeType where native is the name of a library.
+    if (CurrentToken() == Token::kIDENT &&
+        LookaheadToken(1) != Token::kPERIOD) {
+      String* lit = CurrentLiteral();
+      if (lit->Equals("native")) {

[hausner, 2013/08/13 15:40:11]

I think you can speed this loop up by declaring a known symbol for "native" in
symbols.h. The literals in the token stream are guaranteed to be symbols, so you
can directly compare the raw pointers to detect the native pseudo-keyword.

Can we really have a library prefix named after a pseudo keyword? That's sick.

[Michael Lippautz (Google), 2013/08/13 18:07:34]

Done.
Example:
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:1039: 
native.NativeEnqueuer nativeCodegenEnqueuer(Enqueuer world)

grep -Ir "native\." sdk

+        return;
+      }
+    }
+    ConsumeToken();
+  }
+}
+
+
 void Parser::SkipListLiteral() {
   if (CurrentToken() == Token::kINDEX) {
     // Empty list literal.
     ConsumeToken();
     return;
   }
   ExpectToken(Token::kLBRACK);
   while (CurrentToken() != Token::kRBRACK) {
     SkipNestedExpr();
     if (CurrentToken() == Token::kCOMMA) {
@@ skipping 240 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart