Don't lose the expected precedence during && compilation.

lib/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.
 
 class SsaCodeGeneratorTask extends CompilerTask {
   final JavaScriptBackend backend;
   SsaCodeGeneratorTask(JavaScriptBackend backend)
       : this.backend = backend,
         super(backend.compiler);
   String get name() => 'SSA code generator';
@@ skipping 375 matching lines @@
     }
     generationState = oldState;
   }
 
   /**
    * Generate expressions from block information.
    */
   void generateExpression(HExpressionInformation expression) {
     // Currently we only handle sub-expression graphs.
     assert(expression is HSubExpressionBlockInformation);
+    // [visitSubGraph] will reset the [expectedPrecedence]. Make sure we don't
+    // need parenthesis. I.e., this only expects to be called for top-level
+    // expressions, not sub-expressions.
+    assert(expectedPrecedence == JSPrecedence.STATEMENT_PRECEDENCE
+        || expectedPrecedence == JSPrecedence.EXPRESSION_PRECEDENCE);
+
     HSubExpressionBlockInformation expressionSubGraph = expression;
 
     int oldState = generationState;
     generationState = STATE_FIRST_EXPRESSION;
     visitSubGraph(expressionSubGraph.subExpression);
     generationState = oldState;
   }
 
   void generateDeclaration(HExpressionInformation expression) {
     // Currently we only handle sub-expression graphs.
@@ skipping 158 matching lines @@
     String inputName = variableNames.getName(instruction.checkedInput);
     if (name != inputName) return false;
     visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
     return true;
   }
 
   void define(HInstruction instruction) {
     if (isGeneratingExpression()) {
       addExpressionSeparator();
     } else {
+      assert(expectedPrecedence == JSPrecedence.STATEMENT_PRECEDENCE);
       addIndentation();
     }
     if (!instruction.isControlFlow() && variableNames.hasName(instruction)) {
       var name = variableNames.getName(instruction);
       if (!handleSimpleUpdateDefinition(instruction, name)
           && !handleTypeConversion(instruction, name)) {
-        declareInstruction(instruction);
-        buffer.add(" = ");
-        visit(instruction, JSPrecedence.ASSIGNMENT_PRECEDENCE);
+        withPrecedence(JSPrecedence.ASSIGNMENT_PRECEDENCE, () {

[sra1, 2012/06/29 02:03:57]

It bothers me that there are >200 places in this file that say JSPrecedence.XXX
withPrecedence simplifies a few cases and I'm sure it could be used more widely.
 However, the closure passed to withPrecedence has an allocation cost and is
really just an ad-hoc temporary AST that knows only how to emit itself.

The visitor pattern seems error-prone and suffers from being excessively
state-sensitive.

Why not embrace the idea of a temporary AST and generate all expressions via a
lightweight AST for the specific task of emitting well formed expressions? 
There would be no mention of precedence except in the code that emits ASTs. 
Since about half of AST nodes are leaves, a String should be permitted as a
terminal AST, assumed to have a high (primary?) precedence.

+          declareInstruction(instruction);
+          buffer.add(" = ");
+          visit(instruction, JSPrecedence.ASSIGNMENT_PRECEDENCE);
+        });
       }
     } else {
-      visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
+      visit(instruction, expectedPrecedence);
     }
     if (!isGeneratingExpression()) buffer.add(';\n');
   }
 
   void use(HInstruction argument, int expectedPrecedenceForArgument) {
     if (isGenerateAtUseSite(argument)) {
       visit(argument, expectedPrecedenceForArgument);
     } else if (argument is HCheck && argument.isControlFlow()) {
       // A [HCheck] that has control flow can never be used as an
       // expression and may not have a name. Therefore we just use the
@@ skipping 508 matching lines @@
       }
     }
   }
 
   void iterateBasicBlock(HBasicBlock node) {
     HInstruction instruction = node.first;
     while (instruction !== node.last) {
       if (instruction is HTypeGuard) {
         visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
       } else if (!isGenerateAtUseSite(instruction)) {
+        expectedPrecedence = JSPrecedence.STATEMENT_PRECEDENCE;
         define(instruction);
       }
       instruction = instruction.next;
     }
     assignPhisOfSuccessors(node);
     if (instruction is HLoopBranch && isGeneratingExpression()) {
       addExpressionSeparator();
     }
     visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
   }
@@ skipping 326 matching lines @@
     int thenKind = analyzeGraphForCodegen(thenGraph);
     int elseKind = analyzeGraphForCodegen(elseGraph);
 
     void visitWithoutIndent(HStatementInformation toVisit) {
       int oldIndent = indent;
       indent = 0;
       generateStatements(toVisit);
       indent = oldIndent;
     }
 
+    void visitExpression(HStatementInformation toVisit) {
+      // [generateExpression] only works if the [expectedPrecedence] is a
+      // statement or an expression. We therefore have to duplicate some
+      // work here.
+      assert(toVisit.start == toVisit.end);
+      assert(toVisit.start.last is HGoto);
+      // Find the expression (there must only be one).
+      HInstruction expression = toVisit.start.first;
+      while (generateAtUseSite.contains(expression)) {
+        expression = expression.next;
+      }
+      assert(() {
+        HInstruction remaining = expression.next;
+        while (remaining is !HGoto) {
+          if (!generateAtUseSite.contains(remaining)) return false;
+          remaining = remaining.next;
+        }
+        return true;
+      });
+
+      int oldState = generationState;
+      generationState = STATE_FIRST_EXPRESSION;
+      define(expression);
+      generationState = oldState;
+    }
+
     void visitWithIndent(HStatementInformation toVisit) {
       buffer.add('{\n');
       indent++;
       generateStatements(toVisit);
       indent--;
       addIndented('}');
     }
 
     void emitIf() {
       addIndented('if (');
       use(node.inputs[0], JSPrecedence.EXPRESSION_PRECEDENCE);
       buffer.add(') ');
     }
 
     JSBinaryOperatorPrecedence operatorPrecedence = JSPrecedence.binary['&&'];
     void generateAnd(HStatementInformation toVisit, Function condition) {
       addIndentation();
       beginExpression(operatorPrecedence.precedence);
+      var oldPrecedence = expectedPrecedence;
+      expectedPrecedence = operatorPrecedence.left;
       condition();
       buffer.add(" && ");
-      var oldPrecedence = expectedPrecedence;
       expectedPrecedence = operatorPrecedence.right;
-      visitWithoutIndent(toVisit);
+      visitExpression(toVisit);
       expectedPrecedence = oldPrecedence;
       endExpression(operatorPrecedence.precedence);
+      buffer.add(";\n");
     }
 
     List<HBasicBlock> thenSuccessors = thenGraph.end.successors;
     bool thenGraphHasSuccessor = thenSuccessors.length != 0
         && thenSuccessors[0] !== currentGraph.exit;
 
     switch (thenKind) {
       case EMPTY:
         switch (elseKind) {
           case EMPTY:
@@ skipping 1539 matching lines @@
       startBailoutSwitch();
     }
   }
 
   void endLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
     if (labeledBlockInfo.body.start.hasGuards()) {
       endBailoutSwitch();
     }
   }
 }