Fix issue 18435 (2nd attempt).

Add regression test.

R=hausner@google.com

Review URL: https://codereview.chromium.org//293013005

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@36402 260f80e4-7a28-3924-810f-c04153c831b5

runtime/vm/ast_printer.cc

@@ -391,29 +391,37 @@ void AstPrinter::PrintNode(AstNode* node) {
 }
 
 
+void AstPrinter::PrintLocalScopeVariable(const LocalScope* scope,
+                                         LocalVariable* var) {
+  ASSERT(scope != NULL);
+  ASSERT(var != NULL);
+  OS::Print("(%s%s '%s'",
+            var->is_final() ? "final " : "",
+            String::Handle(var->type().Name()).ToCString(),
+            var->name().ToCString());
+  if (var->owner() != scope) {
+    OS::Print(" alias");
+  }
+  if (var->HasIndex()) {
+    OS::Print(" @%d", var->index());
+    if (var->is_captured()) {
+      OS::Print(" ctx %d", var->owner()->context_level());
+    }
+  } else if (var->owner()->function_level() != 0) {
+    OS::Print(" lev %d", var->owner()->function_level());
+  }
+  OS::Print(" valid %" Pd "-%" Pd ")",
+            var->token_pos(),
+            scope->end_token_pos());
+}
+
+
 void AstPrinter::PrintLocalScope(const LocalScope* scope,
                                  int start_index) {
   ASSERT(scope != NULL);
   for (int i = start_index; i < scope->num_variables(); i++) {
     LocalVariable* var = scope->VariableAt(i);
-    OS::Print("(%s%s '%s'",
-              var->is_final() ? "final " : "",
-              String::Handle(var->type().Name()).ToCString(),
-              var->name().ToCString());
-    if (var->owner() != scope) {
-      OS::Print(" alias");
-    }
-    if (var->HasIndex()) {
-      OS::Print(" @%d", var->index());
-      if (var->is_captured()) {
-        OS::Print(" ctx %d", var->owner()->context_level());
-      }
-    } else if (var->owner()->function_level() != 0) {
-      OS::Print(" lev %d", var->owner()->function_level());
-    }
-    OS::Print(" valid %" Pd "-%" Pd ")",
-              var->token_pos(),
-              scope->end_token_pos());
+    PrintLocalScopeVariable(scope, var);
   }
   const LocalScope* child = scope->child();
   while (child != NULL) {
@@ -455,7 +463,7 @@ void AstPrinter::PrintFunctionScope(const ParsedFunction& parsed_function) {
   int pos = 0;  // Current position of variable in scope.
   while (pos < num_params) {
     LocalVariable* param = scope->VariableAt(pos);
-    ASSERT(param->owner() == scope);
+    ASSERT(param->owner() == scope);  // No aliases should precede parameters.
     OS::Print("(param %s%s '%s'",
               param->is_final() ? "final " : "",
               String::Handle(param->type().Name()).ToCString(),

runtime/vm/ast_printer.h

@@ -18,8 +18,6 @@ class AstPrinter : public AstNodeVisitor {
   static void PrintNode(AstNode* node);
   static void PrintFunctionScope(const ParsedFunction& parsed_function);
   static void PrintFunctionNodes(const ParsedFunction& parsed_function);
-  static void PrintLocalScope(const LocalScope* scope, int variable_index);
-
 
 #define DECLARE_VISITOR_FUNCTION(BaseName)                                     \
   virtual void Visit##BaseName##Node(BaseName##Node* node);
@@ -31,6 +29,10 @@ class AstPrinter : public AstNodeVisitor {
   AstPrinter();
   ~AstPrinter();
 
+  static void PrintLocalScopeVariable(const LocalScope* scope,
+                                      LocalVariable* var);
+  static void PrintLocalScope(const LocalScope* scope, int variable_index);
+
   void VisitGenericAstNode(AstNode* node);
   void VisitGenericLocalNode(AstNode* node, const LocalVariable& local);
   void VisitGenericFieldNode(AstNode* node, const Field& field);

runtime/vm/code_generator_test.cc

@@ -58,7 +58,7 @@ CODEGEN_TEST_GENERATE(ReturnParameterCodegen, test) {
   const int num_params = 1;
   LocalVariable* parameter = NewTestLocalVariable("parameter");
   LocalScope* local_scope = node_seq->scope();
-  local_scope->AddVariable(parameter);
+  local_scope->InsertParameterAt(0, parameter);
   ASSERT(local_scope->num_variables() == num_params);
   const Function& function = test->function();
   function.set_num_fixed_parameters(num_params);
@@ -88,8 +88,8 @@ CODEGEN_TEST_GENERATE(SmiParamSumCodegen, test) {
   const int num_locals = 1;
   LocalVariable* sum = NewTestLocalVariable("sum");
   LocalScope* local_scope = node_seq->scope();
-  local_scope->AddVariable(param1);
-  local_scope->AddVariable(param2);
+  local_scope->InsertParameterAt(0, param1);
+  local_scope->InsertParameterAt(1, param2);
   local_scope->AddVariable(sum);
   ASSERT(local_scope->num_variables() == num_params + num_locals);
   const Function& function = test->function();
@@ -206,8 +206,8 @@ CODEGEN_TEST_GENERATE(NativeDecCodegen, test) {
   const int num_opt_params = 1;
   const int num_params = num_fixed_params + num_opt_params;
   LocalScope* local_scope = node_seq->scope();
-  local_scope->AddVariable(NewTestLocalVariable("a"));
-  local_scope->AddVariable(NewTestLocalVariable("b"));
+  local_scope->InsertParameterAt(0, NewTestLocalVariable("a"));
+  local_scope->InsertParameterAt(1, NewTestLocalVariable("b"));
   ASSERT(local_scope->num_variables() == num_params);
   const Array& default_values = Array::ZoneHandle(Array::New(num_opt_params));
   default_values.SetAt(0, Smi::ZoneHandle(Smi::New(1)));  // b = 1.
@@ -377,11 +377,11 @@ CODEGEN_TEST_GENERATE(NativeSumCodegen, test) {
   const int num_opt_params = 3;
   const int num_params = num_fixed_params + num_opt_params;
   LocalScope* local_scope = node_seq->scope();
-  local_scope->AddVariable(NewTestLocalVariable("a"));
-  local_scope->AddVariable(NewTestLocalVariable("b"));
-  local_scope->AddVariable(NewTestLocalVariable("c"));
-  local_scope->AddVariable(NewTestLocalVariable("d"));
-  local_scope->AddVariable(NewTestLocalVariable("e"));
+  local_scope->InsertParameterAt(0, NewTestLocalVariable("a"));
+  local_scope->InsertParameterAt(1, NewTestLocalVariable("b"));
+  local_scope->InsertParameterAt(2, NewTestLocalVariable("c"));
+  local_scope->InsertParameterAt(3, NewTestLocalVariable("d"));
+  local_scope->InsertParameterAt(4, NewTestLocalVariable("e"));
   ASSERT(local_scope->num_variables() == num_params);
   const Array& default_values = Array::ZoneHandle(Array::New(num_opt_params));
   default_values.SetAt(0, Smi::ZoneHandle(Smi::New(10)));
@@ -474,9 +474,9 @@ CODEGEN_TEST_GENERATE(NativeNonNullSumCodegen, test) {
   SequenceNode* node_seq = test->node_sequence();
   const int num_params = 3;
   LocalScope* local_scope = node_seq->scope();
-  local_scope->AddVariable(NewTestLocalVariable("a"));
-  local_scope->AddVariable(NewTestLocalVariable("b"));
-  local_scope->AddVariable(NewTestLocalVariable("c"));
+  local_scope->InsertParameterAt(0, NewTestLocalVariable("a"));
+  local_scope->InsertParameterAt(1, NewTestLocalVariable("b"));
+  local_scope->InsertParameterAt(2, NewTestLocalVariable("c"));
   ASSERT(local_scope->num_variables() == num_params);
   const Function& function = test->function();
   function.set_is_native(true);

runtime/vm/parser.cc

@@ -2476,13 +2476,13 @@ SequenceNode* Parser::MakeImplicitConstructor(const Function& func) {
 
   LocalVariable* receiver = new LocalVariable(
       Scanner::kNoSourcePos, Symbols::This(), *ReceiverType(current_class()));
-  current_block_->scope->AddVariable(receiver);
+  current_block_->scope->InsertParameterAt(0, receiver);
 
   LocalVariable* phase_parameter =
       new LocalVariable(Scanner::kNoSourcePos,
                         Symbols::PhaseParameter(),
                         Type::ZoneHandle(Type::SmiType()));
-  current_block_->scope->AddVariable(phase_parameter);
+  current_block_->scope->InsertParameterAt(1, phase_parameter);
 
   // Parse expressions of instance fields that have an explicit
   // initializer expression.
@@ -2523,7 +2523,7 @@ SequenceNode* Parser::MakeImplicitConstructor(const Function& func) {
           Scanner::kNoSourcePos,
           String::ZoneHandle(func.ParameterNameAt(i)),
           Type::ZoneHandle(Type::DynamicType()));
-      current_block_->scope->AddVariable(param);
+      current_block_->scope->InsertParameterAt(i, param);
       forwarding_args->Add(new LoadLocalNode(Scanner::kNoSourcePos, param));
     }
   }
@@ -5393,7 +5393,7 @@ void Parser::AddFormalParamsToScope(const ParamList* params,
     const String* name = param_desc.name;
     LocalVariable* parameter = new LocalVariable(
         param_desc.name_pos, *name, *param_desc.type);
-    if (!scope->AddVariable(parameter)) {
+    if (!scope->InsertParameterAt(i, parameter)) {
       ErrorMsg(param_desc.name_pos,
                "name '%s' already exists in scope",
                param_desc.name->ToCString());

runtime/vm/scopes.cc

@@ -66,6 +66,19 @@ bool LocalScope::AddVariable(LocalVariable* variable) {
 }
 
 
+bool LocalScope::InsertParameterAt(intptr_t pos, LocalVariable* parameter) {
+  ASSERT(parameter != NULL);
+  if (LocalLookupVariable(parameter->name()) != NULL) {
+    return false;
+  }
+  variables_.InsertAt(pos, parameter);
+  // InsertParameterAt is not used to add aliases of parameters.
+  ASSERT(parameter->owner() == NULL);
+  parameter->set_owner(this);
+  return true;
+}
+
+
 bool LocalScope::AddLabel(SourceLabel* label) {
   if (LocalLookupLabel(label->name()) != NULL) {
     return false;
@@ -171,6 +184,10 @@ int LocalScope::AllocateVariables(int first_parameter_index,
   while (pos < num_parameters) {
     LocalVariable* parameter = VariableAt(pos);
     pos++;
+    // Parsing formal parameter default values may add local variable aliases
+    // to the local scope before the formal parameters are added. However,
+    // the parameters get inserted in front of the aliases, therefore, no
+    // aliases can be encountered among the first num_parameters variables.
     ASSERT(parameter->owner() == this);
     if (parameter->is_captured()) {
       // A captured parameter has a slot allocated in the frame and one in the

runtime/vm/scopes.h

@@ -234,6 +234,11 @@ class LocalScope : public ZoneAllocated {
   // same name is already present.
   bool AddVariable(LocalVariable* variable);
 
+  // Insert a formal parameter variable to the scope at the given position,
+  // possibly in front of aliases already added with AddVariable.
+  // Returns false if a variable with the same name is already present.
+  bool InsertParameterAt(intptr_t pos, LocalVariable* parameter);
+
   // Add a label to the scope. Returns false if a label with the same name
   // is already present.
   bool AddLabel(SourceLabel* label);

tests/language/regress_18435_test.dart

@@ -0,0 +1,24 @@
+// Copyright (c) 2014, 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.
+
+// Regression test for issue 18435.
+
+import "package:expect/expect.dart";
+
+main() {
+  const MISSING_VALUE = "MISSING_VALUE";
+
+  void foo([var p1 = MISSING_VALUE, var p2 = MISSING_VALUE]) {
+    Expect.equals("P1", p1);
+    Expect.equals("P2", p2);
+  }
+
+  void bar([var p1 = "MISSING_VALUE", var p2 = "MISSING_VALUE"]) {
+    Expect.equals("P1", p1);
+    Expect.equals("P2", p2);
+  }
+
+  foo("P1", "P2");
+  bar("P1", "P2");
+}