commit a867e2cc591e5224a26dd2c88ef2a747829344cf
author Alexander Markov <alexmarkov@google.com> Fri Aug 31 22:27:20 2018 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Fri Aug 31 22:27:20 2018 +0000
tree 9247bc9a05222ce711f38854ab0ebeed2f201554
parent e9504d28572e74ff3232a5ee639d751dbb66619b

[vm/bytecode] Avoid sorting named parameters in kernel AST when generating bytecode

When generating bytecode, kernel AST was modifed to sort optional named
parameters. This has unfortunate side-effect of breaking default
(non-interpreter) mode if it uses platform.dill with bytecode.

Bytecode generator is changed to avoid modifications to kernel AST.
Instead, sorted named parameters are kept on a side.

Fixes https://github.com/dart-lang/sdk/issues/34305

Change-Id: Iba473f09af0af515b2117e39d63363412433d538
Reviewed-on: https://dart-review.googlesource.com/72546
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>

pkg/vm/lib/bytecode/gen_bytecode.dart

diff --git a/pkg/vm/lib/bytecode/gen_bytecode.dart b/pkg/vm/lib/bytecode/gen_bytecode.dart
index 0ec28f7..62fa462 100644
--- a/pkg/vm/lib/bytecode/gen_bytecode.dart
+++ b/pkg/vm/lib/bytecode/gen_bytecode.dart
@@ -741,7 +741,7 @@
       } else {
         assert(numOptionalNamed != 0);
         for (int i = 0; i < numOptionalNamed; i++) {
-          final param = function.namedParameters[i];
+          final param = locals.sortedNamedParameters[i];
           asm.emitLoadConstant(
               numFixed + i, cp.add(new ConstantString(param.name)));
           asm.emitLoadConstant(numFixed + i, _getDefaultParamConstIndex(param));
@@ -867,7 +867,7 @@
         }
       }
       function.positionalParameters.forEach(_copyParamIfCaptured);
-      function.namedParameters.forEach(_copyParamIfCaptured);
+      locals.sortedNamedParameters.forEach(_copyParamIfCaptured);
     }
   }
 
@@ -893,7 +893,7 @@
       }
     }
     function.positionalParameters.forEach(_genArgumentTypeCheck);
-    function.namedParameters.forEach(_genArgumentTypeCheck);
+    locals.sortedNamedParameters.forEach(_genArgumentTypeCheck);
   }
 
   void _genArgumentTypeCheck(VariableDeclaration variable) {
@@ -956,7 +956,7 @@
     // as default value expressions could use local const variables which
     // are not available in bytecode.
     function.positionalParameters.forEach(_evaluateDefaultParameterValue);
-    function.namedParameters.forEach(_evaluateDefaultParameterValue);
+    locals.sortedNamedParameters.forEach(_evaluateDefaultParameterValue);
 
     final int closureFunctionIndex =
         cp.add(new ConstantClosureFunction(name, function));

pkg/vm/lib/bytecode/local_vars.dart

diff --git a/pkg/vm/lib/bytecode/local_vars.dart b/pkg/vm/lib/bytecode/local_vars.dart
index 1a780646..45d8351 100644
--- a/pkg/vm/lib/bytecode/local_vars.dart
+++ b/pkg/vm/lib/bytecode/local_vars.dart
@@ -165,6 +165,8 @@
 
   List<VariableDeclaration> get originalNamedParameters =>
       _currentFrame.originalNamedParameters;
+  List<VariableDeclaration> get sortedNamedParameters =>
+      _currentFrame.sortedNamedParameters;
 
   LocalVariables(Member node) {
     final scopeBuilder = new _ScopeBuilder(this);
@@ -214,6 +216,7 @@
   Scope topScope;
 
   List<VariableDeclaration> originalNamedParameters;
+  List<VariableDeclaration> sortedNamedParameters;
   int numParameters = 0;
   int numTypeArguments = 0;
   bool hasOptionalParameters = false;
@@ -271,10 +274,11 @@
 
   _ScopeBuilder(this.locals);
 
-  void _sortNamedParameters(FunctionNode function) {
-    function.namedParameters.sort(
-        (VariableDeclaration a, VariableDeclaration b) =>
-            a.name.compareTo(b.name));
+  List<VariableDeclaration> _sortNamedParameters(FunctionNode function) {
+    final params = function.namedParameters.toList();
+    params.sort((VariableDeclaration a, VariableDeclaration b) =>
+        a.name.compareTo(b.name));
+    return params;
   }
 
   void _visitFunction(TreeNode node) {
@@ -337,11 +341,11 @@
         _declareVariable(_currentFrame.closureVar);
       }
 
-      _currentFrame.originalNamedParameters = function.namedParameters.toList();
-      _sortNamedParameters(function);
+      _currentFrame.originalNamedParameters = function.namedParameters;
+      _currentFrame.sortedNamedParameters = _sortNamedParameters(function);
 
       visitList(function.positionalParameters, this);
-      visitList(function.namedParameters, this);
+      visitList(_currentFrame.sortedNamedParameters, this);
 
       if (_currentFrame.isSyncYielding) {
         // The following variables from parent frame are used implicitly and need
@@ -903,7 +907,7 @@
     for (var param in function.positionalParameters) {
       _allocateParameter(param, count++);
     }
-    for (var param in function.namedParameters) {
+    for (var param in _currentFrame.sortedNamedParameters) {
       _allocateParameter(param, count++);
     }
     assert(count == _currentFrame.numParameters);

pkg/vm/testcases/bytecode/optional_params.dart.expect

diff --git a/pkg/vm/testcases/bytecode/optional_params.dart.expect b/pkg/vm/testcases/bytecode/optional_params.dart.expect
index a8ae7f0..306a642 100644
--- a/pkg/vm/testcases/bytecode/optional_params.dart.expect
+++ b/pkg/vm/testcases/bytecode/optional_params.dart.expect
@@ -214,7 +214,7 @@
   [25] = StaticICData target 'dart.core::_StringBase::_interpolate', arg-desc CP#12
   [26] = StaticICData target 'dart.core::print', arg-desc CP#12
 }
-]static method foo2(dynamic y, dynamic z, {dynamic a = 42, dynamic b = const <core::String>["default_b"], dynamic c = "default_c"}) → void {
+]static method foo2(dynamic y, dynamic z, {dynamic c = "default_c", dynamic a = 42, dynamic b = const <core::String>["default_b"]}) → void {
   core::print("y = ${y}");
   core::print("z = ${z}");
   core::print("a = ${a}");