Clean up the codegen by not re-computing parameter names.

sdk/lib/_internal/compiler/implementation/ssa/codegen.dart

 // 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.
 
 part of ssa;
 
 class SsaCodeGeneratorTask extends CompilerTask {
 
   final JavaScriptBackend backend;
 
@@ skipping 36 matching lines @@
     if (work.element.isField()) {
       return generateLazyInitializer(work, graph);
     } else {
       return generateMethod(work, graph);
     }
   }
 
   CodeBuffer generateLazyInitializer(work, graph) {
     return measure(() {
       compiler.tracer.traceGraph("codegen", graph);
-      List<js.Parameter> parameters = <js.Parameter>[];
-      SsaOptimizedCodeGenerator codegen = new SsaOptimizedCodeGenerator(
-          backend, work, parameters, new Map<Element, String>());
+      SsaOptimizedCodeGenerator codegen =
+          new SsaOptimizedCodeGenerator(backend, work);
       codegen.visitGraph(graph);
       js.Block body = codegen.body;
-      js.Fun fun = new js.Fun(parameters, body);
+      js.Fun fun = new js.Fun(codegen.parameters, body);
       return prettyPrint(fun);
     });
   }
 
   CodeBuffer generateMethod(WorkItem work, HGraph graph) {
     return measure(() {
       JavaScriptItemCompilationContext context = work.compilationContext;
       HTypeMap types = context.types;
       graph.exit.predecessors.forEach((block) {
         assert(block.last is HGoto || block.last is HReturn);
         if (block.last is HReturn) {
           backend.registerReturnType(work.element, types[block.last.inputs[0]]);
         } else {
           backend.registerReturnType(work.element, HType.NULL);
         }
       });
       compiler.tracer.traceGraph("codegen", graph);
-      Map<Element, String> parameterNames = getParameterNames(work);
-      // Use [work.element] to ensure that the parameter element come from
-      // the declaration.
-      FunctionElement function = work.element;
-      function.computeSignature(compiler).forEachParameter((element) {
-        compiler.enqueuer.codegen.addToWorkList(element, work.resolutionTree);
-      });
-      List<js.Parameter> parameters = <js.Parameter>[];
-      parameterNames.forEach((element, name) {
-        parameters.add(new js.Parameter(name));
-      });
-      addBackendParameters(work.element, parameters, parameterNames);
-      String parametersString = Strings.join(parameterNames.values, ", ");
-      SsaOptimizedCodeGenerator codegen = new SsaOptimizedCodeGenerator(
-          backend, work, parameters, parameterNames);
+      SsaOptimizedCodeGenerator codegen =
+          new SsaOptimizedCodeGenerator(backend, work);
       codegen.visitGraph(graph);
 
       FunctionElement element = work.element;
       js.Block body;
       ClassElement enclosingClass = element.getEnclosingClass();
       bool allowVariableMinification;
       if (element.isInstanceMember()
           && enclosingClass.isNative()
           && native.isOverriddenMethod(
               element, enclosingClass, nativeEmitter)) {
         // Record that this method is overridden. In case of optional
         // arguments, the emitter will generate stubs to handle them,
         // and needs to know if the method is overridden.
         nativeEmitter.overriddenMethods.add(element);
         StringBuffer buffer = new StringBuffer();
         String codeString = prettyPrint(codegen.body).toString();
+        String parametersString = Strings.join(codegen.parameterNames.values, ", ");        

[floitsch, 2012/11/07 14:44:37]

80chars.

[ngeoffray, 2012/11/07 14:47:48]

Done.

         native.generateMethodWithPrototypeCheckForElement(
             compiler, buffer, element, codeString, parametersString);
         js.Node nativeCode = new js.LiteralStatement(buffer.toString());
         body = new js.Block(<js.Statement>[nativeCode]);
         allowVariableMinification = false;
       } else {
         body = codegen.body;
         allowVariableMinification = !codegen.visitedForeignCode;
       }
-      js.Fun fun = buildJavaScriptFunction(element, parameters, body);
+      js.Fun fun = buildJavaScriptFunction(element, codegen.parameters, body);
       return prettyPrint(fun,
                          allowVariableMinification: allowVariableMinification);
     });
   }
 
-  void addBackendParameter(Element element,
-                           List<js.Parameter> parameters,
-                           Map<Element, String> parameterNames) {
-    String name = element.name.slowToString();
-    String prefix = '';
-    // Avoid collisions with real parameters of the method.
-    do {
-      name = JsNames.getValid('$prefix$name');
-      prefix = '\$$prefix';
-    } while (parameterNames.containsValue(name));
-    parameterNames[element] = name;
-    parameters.add(new js.Parameter(name));
-  }
-
-  void addBackendParameters(Element element,
-                            List<js.Parameter> parameters,
-                            Map<Element, String> parameterNames) {
-    // TODO(ngeoffray): We should infer this information from the
-    // graph, instead of recomputing what the builder did.
-    if (element.isConstructor()) {
-      // Put the type parameters.
-      ClassElement cls = element.enclosingElement;
-      if (!compiler.world.needsRti(cls)) return;
-      cls.typeVariables.forEach((TypeVariableType typeVariable) {
-        addBackendParameter(typeVariable.element, parameters, parameterNames);
-      });
-    } else if (element.isGenerativeConstructorBody()) {
-      // Put the parameter checks parameters.
-      Node node = element.implementation.parseNode(compiler);
-      ClosureClassMap closureData =
-          compiler.closureToClassMapper.getMappingForNestedFunction(node);
-      FunctionElement functionElement = element;
-      FunctionSignature params = functionElement.computeSignature(compiler);
-      TreeElements elements =
-          compiler.enqueuer.resolution.getCachedElements(element);
-      params.orderedForEachParameter((Element element) {
-        if (elements.isParameterChecked(element)) {
-          Element checkResultElement =
-              closureData.parametersWithSentinel[element];
-          addBackendParameter(checkResultElement, parameters, parameterNames);
-        }
-      });
-      // Put the box parameter.
-      ClosureScope scopeData = closureData.capturingScopes[node];
-      if (scopeData != null) {
-        addBackendParameter(scopeData.boxElement, parameters, parameterNames);
-      }
-    }
-  }
-
   CodeBuffer generateBailoutMethod(WorkItem work, HGraph graph) {
     return measure(() {
       compiler.tracer.traceGraph("codegen-bailout", graph);
 
-      Map<Element, String> parameterNames = getParameterNames(work);
-      List<js.Parameter> parameters = <js.Parameter>[];
-      parameterNames.forEach((element, name) {
-        parameters.add(new js.Parameter(name));
-      });
-      addBackendParameters(work.element, parameters, parameterNames);
-
-      SsaUnoptimizedCodeGenerator codegen = new SsaUnoptimizedCodeGenerator(
-          backend, work, parameters, parameterNames);
+      SsaUnoptimizedCodeGenerator codegen =
+          new SsaUnoptimizedCodeGenerator(backend, work);
       codegen.visitGraph(graph);
 
       js.Block body = new js.Block(<js.Statement>[]);
       if (codegen.setup != null) body.statements.add(codegen.setup);
       body.statements.add(codegen.body);
       js.Fun fun =
           buildJavaScriptFunction(work.element, codegen.newParameters, body);
       return prettyPrint(fun);
     });
   }
-
-  Map<Element, String> getParameterNames(WorkItem work) {
-    // Make sure the map preserves insertion order, so that fetching
-    // the values will keep the order of parameters.
-    Map<Element, String> parameterNames = new LinkedHashMap<Element, String>();
-    FunctionElement function = work.element.implementation;
-
-    // The dom/html libraries have inline JS code that reference
-    // parameter names directly. Long-term such code will be rejected.
-    // Now, just don't mangle the parameter name.
-    FunctionSignature signature = function.computeSignature(compiler);
-    signature.orderedForEachParameter((Element element) {
-      parameterNames[element] = function.isNative()
-          ? element.name.slowToString()
-          : JsNames.getValid('${element.name.slowToString()}');
-    });
-    return parameterNames;
-  }
 }
 
 // Stop-gap until the core classes have such a class.
 class OrderedSet<T> {
   final LinkedHashMap<T, bool> map = new LinkedHashMap<T, bool>();
 
   void add(T x) {
     if (!map.containsKey(x)) {
       map[x] = true;
     }
@@ skipping 41 matching lines @@
 
   final JavaScriptBackend backend;
   final WorkItem work;
   final HTypeMap types;
 
   final Set<HInstruction> generateAtUseSite;
   final Set<HInstruction> controlFlowOperators;
   final Map<Element, ElementAction> breakAction;
   final Map<Element, ElementAction> continueAction;
   final Map<Element, String> parameterNames;
+  final List<js.Parameter> parameters;
 
   js.Block currentContainer;
   js.Block get body => currentContainer;
   List<js.Expression> expressionStack;
   List<js.Block> oldContainerStack;
 
   /**
    * Contains the names of the instructions, as well as the parallel
    * copies to perform on block transitioning.
    */
@@ skipping 19 matching lines @@
   HGraph currentGraph;
   HBasicBlock currentBlock;
 
   // Records a block-information that is being handled specially.
   // Used to break bad recursion.
   HBlockInformation currentBlockInformation;
   // The subgraph is used to delimit traversal for some constructions, e.g.,
   // if branches.
   SubGraph subGraph;
 
-  SsaCodeGenerator(this.backend,
-                   WorkItem work,
-                   this.parameterNames)
+  SsaCodeGenerator(this.backend, WorkItem work)
     : this.work = work,
       this.types =
           (work.compilationContext as JavaScriptItemCompilationContext).types,
+      parameterNames = new LinkedHashMap<Element, String>(),
       declaredLocals = new Set<String>(),
       collectedVariableDeclarations = new OrderedSet<String>(),
       currentContainer = new js.Block.empty(),
+      parameters = <js.Parameter>[],
       expressionStack = <js.Expression>[],
       oldContainerStack = <js.Block>[],
       generateAtUseSite = new Set<HInstruction>(),
       controlFlowOperators = new Set<HInstruction>(),
       breakAction = new Map<Element, ElementAction>(),
       continueAction = new Map<Element, ElementAction>();
 
   LibraryElement get currentLibrary => work.element.getLibrary();
   Compiler get compiler => backend.compiler;
   NativeEmitter get nativeEmitter => backend.emitter.nativeEmitter;
@@ skipping 2282 matching lines @@
       }
       String helperName = backend.namer.isolateAccess(helperElement);
       push(new js.Call(new js.VariableUse(helperName), arguments));
     } else {
       use(node.checkedInput);
     }
   }
 }
 
 class SsaOptimizedCodeGenerator extends SsaCodeGenerator {
-  SsaOptimizedCodeGenerator(backend, work, parameters, parameterNames)
-    : super(backend, work, parameterNames) {
+  SsaOptimizedCodeGenerator(backend, work) : super(backend, work);
+
+  int maxBailoutParameters;
+
+  HBasicBlock beginGraph(HGraph graph) {
     // Declare the parameter names only for the optimized version. The
     // unoptimized version has different parameters.
     parameterNames.forEach((Element element, String name) {
+      parameters.add(new js.Parameter(name));
       declaredLocals.add(name);
     });
+    return graph.entry;
   }
 
-  int maxBailoutParameters;
-
-  HBasicBlock beginGraph(HGraph graph) => graph.entry;
   void endGraph(HGraph graph) {}
 
   js.Statement bailout(HTypeGuard guard, String reason) {
     if (maxBailoutParameters == null) {
       maxBailoutParameters = 0;
       work.guards.forEach((HTypeGuard workGuard) {
         HBailoutTarget target = workGuard.bailoutTarget;
         int inputLength = target.inputs.length;
         if (inputLength > maxBailoutParameters) {
           maxBailoutParameters = inputLength;
@@ skipping 165 matching lines @@
   /**
    * Keeps track if a bailout switch already used its [:default::] clause. New
    * bailout-switches just push [:false:] on the stack and replace it when
    * they used the [:default::] clause.
    */
   final List<bool> defaultClauseUsedInBailoutStack;
 
   SsaBailoutPropagator propagator;
   HInstruction savedFirstInstruction;
 
-  SsaUnoptimizedCodeGenerator(backend, work, parameters, parameterNames)
-    : super(backend, work, parameterNames),
+  SsaUnoptimizedCodeGenerator(backend, work)
+    : super(backend, work),
       oldBailoutSwitches = <js.Switch>[],
       newParameters = <js.Parameter>[],
       labels = <String>[],
       defaultClauseUsedInBailoutStack = <bool>[];
 
   String pushLabel() {
     String label = 'L${labelId++}';
     labels.addLast(label);
     return label;
   }
@@ skipping 307 matching lines @@
   if (leftType.canBeNull() && rightType.canBeNull()) {
     if (left.isConstantNull() || right.isConstantNull() ||
         (leftType.isPrimitive() && leftType == rightType)) {
       return '==';
     }
     return null;
   } else {
     return '===';
   }
 }