ceylon: allow comprehensions to start with ifs (ceylon/ceylon-spec#869) WIP (the ceylon-js patch is just to make ceylon-dist buildable)

ceylon-compiler.patch

diff --git a/src/com/redhat/ceylon/compiler/java/codegen/ExpressionTransformer.java b/src/com/redhat/ceylon/compiler/java/codegen/ExpressionTransformer.java
index f4d815a..3a54ba6 100755
--- a/src/com/redhat/ceylon/compiler/java/codegen/ExpressionTransformer.java
+++ b/src/com/redhat/ceylon/compiler/java/codegen/ExpressionTransformer.java
@@ -1401,7 +1401,7 @@ public class ExpressionTransformer extends AbstractTransformer {
     
     public JCExpression comprehensionAsSequential(Tree.Comprehension comprehension, ProducedType expectedType) {
         JCExpression sequential = iterableToSequential(transformComprehension(comprehension));
-        ProducedType elementType = comprehension.getForComprehensionClause().getTypeModel();
+        ProducedType elementType = comprehension.getInitialComprehensionClause().getTypeModel();
         ProducedType sequentialType = typeFact().getSequentialType(elementType);
         return sequentialEmptiness(sequential, expectedType, sequentialType);
     }
@@ -1435,7 +1435,7 @@ public class ExpressionTransformer extends AbstractTransformer {
             } else if (expr instanceof Tree.Comprehension) {
                 Tree.Comprehension comp = (Tree.Comprehension) expr;
                 ProducedType elementType = expr.getTypeModel(); 
-                ProducedType expectedType = comp.getForComprehensionClause().getPossiblyEmpty() 
+                ProducedType expectedType = comp.getInitialComprehensionClause().getPossiblyEmpty() 
                         ? typeFact().getSequentialType(elementType)
                         : typeFact().getSequenceType(elementType);
                 tail = comprehensionAsSequential(comp, expectedType);
@@ -4386,7 +4386,7 @@ public class ExpressionTransformer extends AbstractTransformer {
     }
 
     JCExpression transformComprehension(Tree.Comprehension comp, ProducedType expectedType) {
-        ProducedType elementType = comp.getForComprehensionClause().getTypeModel();
+        ProducedType elementType = comp.getInitialComprehensionClause().getTypeModel();
         // get rid of anonymous types
         elementType = typeFact().denotableType(elementType);
         elementType = wrapInOptionalForInterop(elementType, expectedType);
@@ -4427,7 +4427,7 @@ public class ExpressionTransformer extends AbstractTransformer {
         public ComprehensionTransformation(final Tree.Comprehension comp, ProducedType elementType) {
             this.comp = comp;
             targetIterType = typeFact().getIterableType(elementType);
-            absentIterType = comp.getForComprehensionClause().getFirstTypeModel();
+            absentIterType = comp.getInitialComprehensionClause().getFirstTypeModel();
         }
     
         public JCExpression transformComprehension() {
@@ -4435,7 +4435,7 @@ public class ExpressionTransformer extends AbstractTransformer {
             // make sure "this" will be qualified since we're introducing a new surrounding class
             boolean oldWithinSyntheticClassBody = withinSyntheticClassBody(true);
             try{
-                Tree.ComprehensionClause clause = comp.getForComprehensionClause();
+                Tree.ComprehensionClause clause = comp.getInitialComprehensionClause();
                 while (clause != null) {
                     final Naming.SyntheticName iterVar = naming.synthetic(Prefix.$iterator$, idx);
                     Naming.SyntheticName itemVar = null;
@@ -4459,7 +4459,7 @@ public class ExpressionTransformer extends AbstractTransformer {
                         at(excc);
                         clause = null;
                     } else {
-                        return makeErroneous(clause, "compiler bug: comprehension clausees of type " + clause.getClass().getName() + " are not yet supported");
+                        return makeErroneous(clause, "compiler bug: comprehension clauses of type " + clause.getClass().getName() + " are not yet supported");
                     }
                     idx++;
                     if (itemVar != null) prevItemVar = itemVar;
@@ -4549,12 +4549,49 @@ public class ExpressionTransformer extends AbstractTransformer {
         class IfComprehensionCondList extends CondList {
 
             private final ListBuffer<JCStatement> varDecls = ListBuffer.lb();
-            private final JCExpression condExpr;
+            /**
+             * A list of statements that are placed in the main body, before the conditions.
+             */
+            private final List<JCStatement> preCheck;
+            /**
+             * A list of statements that are placed in the innermost condition's body.
+             */
+            private final List<JCStatement> insideCheck;
+            /**
+             * A list of statements that are placed in the main body, after the conditions.
+             */
+            private final List<JCStatement> postCheck;
             
+            /**
+             * An IfComprehensionCondList suitable for "inner" if comprehension clauses.
+             * Checks {@code condExpr} before checking the {@code conditions}, and {@code break;}s if the conditions apply.
+             * Intended to be placed in a {@code while (true) } loop, to keep checking the conditions until they apply
+             * or {@code condExpr} doesn't.
+             */
             public IfComprehensionCondList(java.util.List<Tree.Condition> conditions, JCExpression condExpr) {
+                this(conditions,
+                    // check condExpr before the conditions
+                    List.<JCStatement>of(make().If(make().Unary(JCTree.NOT, condExpr), make().Break(null), null)),
+                    // break if a condition matches
+                    List.<JCStatement>of(make().Break(null)),
+                    null);
+            }
+            
+            /**
+             * General-purpose constructor. Places {@code precheck} before the conditions and their variable declarations,
+             * {@code insideCheck} in the body of the innermost condition (executed only if all {@code conditions} apply), and
+             * {@code postCheck} after the conditions.
+             */
+            public IfComprehensionCondList(java.util.List<Tree.Condition> conditions,
+                    List<JCStatement> preCheck, List<JCStatement> insideCheck, List<JCStatement> postCheck) {
                 statementGen().super(conditions, null);
-                this.condExpr = condExpr;
-            }     
+                if(preCheck == null) preCheck = List.<JCStatement>nil();
+                if(insideCheck == null) insideCheck = List.<JCStatement>nil();
+                if(postCheck == null) postCheck = List.<JCStatement>nil();
+                this.preCheck = preCheck;
+                this.insideCheck = insideCheck;
+                this.postCheck = postCheck;
+            }
 
             @Override
             protected List<JCStatement> transformInnermost(Tree.Condition condition) {
@@ -4566,7 +4603,7 @@ public class ExpressionTransformer extends AbstractTransformer {
                 SyntheticName resultVarName = addVarSubs(transformedCond);
                 return transformCommon(transformedCond,
                         test,
-                        List.<JCStatement>of(make().Break(null)),
+                        insideCheck,
                         resultVarName);
             }
             
@@ -4611,29 +4648,64 @@ public class ExpressionTransformer extends AbstractTransformer {
             public List<JCStatement> getResult() {
                 List<JCStatement> stmts = transformList(conditions);
                 ListBuffer<JCStatement> result = ListBuffer.lb();
-                result.append(make().If(make().Unary(JCTree.NOT, condExpr), make().Break(null), null));
+                result.appendList(preCheck);
                 result.appendList(varDecls);
                 result.appendList(stmts);
+                result.appendList(postCheck);
                 return result.toList();   
             }
 
         }
         
         private void transformIfClause(Tree.IfComprehensionClause clause) {
-            //Filter contexts need to check if the previous context applies and then check the condition
-            JCExpression condExpr = make().Apply(null,
-                ctxtName.makeIdentWithThis(), List.<JCExpression>nil());
-            ctxtName = naming.synthetic(Prefix.$next$, idx);
-            
-            IfComprehensionCondList ifComprehensionCondList = new IfComprehensionCondList(clause.getConditionList().getConditions(), condExpr);
-            List<JCStatement> ifs = ifComprehensionCondList.getResult();
-            JCStatement loop = make().WhileLoop(makeBoolean(true), make().Block(0, ifs));
+            List<JCStatement> body;
+            if (clause == comp.getInitialComprehensionClause()) {
+                //No previous context
+                ctxtName = naming.synthetic(Prefix.$next$, idx);
+                
+                SyntheticName exhaustedName = ctxtName.suffixedBy(Suffix.$exhausted$);
+                JCVariableDecl exhaustedDef = make().VarDef(make().Modifiers(Flags.PRIVATE),
+                        exhaustedName.asName(), makeJavaType(typeFact().getBooleanDeclaration().getType()), null);
+                fields.add(exhaustedDef);
+                JCStatement returnIfExhausted = make().If(exhaustedName.makeIdent(), make().Return(makeBoolean(false)), null);
+                SyntheticName resultName = naming.temp("result");
+                JCVariableDecl declareResult = make().VarDef(make().Modifiers(0),
+                        resultName.asName(), makeJavaType(typeFact().getBooleanDeclaration().getType()), makeBoolean(false));
+                JCStatement setResultTrue = make().Exec(make().Assign(resultName.makeIdent(), makeBoolean(true)));
+                JCStatement setExhaustedTrue = make().Exec(make().Assign(exhaustedName.makeIdent(), makeBoolean(true)));
+                JCStatement returnResult = make().Return(resultName.makeIdent());
+                
+                body = new IfComprehensionCondList(clause.getConditionList().getConditions(),
+                    List.<JCStatement>of(
+                        //if we already evaluated the expression, return
+                        returnIfExhausted,
+                        //declare the result variable that's going to store the result of the conditions (init to false)
+                        declareResult),
+                    List.<JCStatement>of(
+                        //if the conditions apply: set the result variable to true
+                        setResultTrue),
+                    List.<JCStatement>of(
+                        //we evaluated the expression
+                        setExhaustedTrue,
+                        //and return the result
+                        returnResult)).getResult();
+            } else {
+        	    //Filter contexts need to check if the previous context applies and then check the condition
+                JCExpression condExpr = make().Apply(null,
+                    ctxtName.makeIdentWithThis(), List.<JCExpression>nil());
+                ctxtName = naming.synthetic(Prefix.$next$, idx);
+                
+                IfComprehensionCondList ifComprehensionCondList = new IfComprehensionCondList(clause.getConditionList().getConditions(), condExpr);
+                List<JCStatement> ifs = ifComprehensionCondList.getResult();
+                JCStatement loop = make().WhileLoop(makeBoolean(true), make().Block(0, ifs));
+                body = List.<JCStatement>of(loop,
+                    make().Return(make().Unary(JCTree.NOT, prevItemVar.suffixedBy(Suffix.$exhausted$).makeIdent())));
+        	}
             MethodDefinitionBuilder mb = MethodDefinitionBuilder.systemMethod(ExpressionTransformer.this, ctxtName.getName())
                 .ignoreModelAnnotations()
                 .modifiers(Flags.PRIVATE | Flags.FINAL)
                 .resultType(null, makeJavaType(typeFact().getBooleanDeclaration().getType()))
-                .body(loop)
-                .body(make().Return(make().Unary(JCTree.NOT, prevItemVar.suffixedBy(Suffix.$exhausted$).makeIdent())));
+                .body(body);
             fields.add(mb.build());
         }
 
@@ -4645,7 +4717,7 @@ public class ExpressionTransformer extends AbstractTransformer {
             ProducedType iterType = specexpr.getExpression().getTypeModel();
             JCExpression iterTypeExpr = makeJavaType(typeFact().getIteratorType(
                     typeFact().getIteratedType(iterType)));
-            if (clause == comp.getForComprehensionClause()) {
+            if (clause == comp.getInitialComprehensionClause()) {
                 //The first iterator can be initialized as a field
                 fields.add(make().VarDef(make().Modifiers(Flags.PRIVATE | Flags.FINAL), iterVar.asName(), iterTypeExpr,
                     null));
@@ -4658,9 +4730,9 @@ public class ExpressionTransformer extends AbstractTransformer {
                 fields.add(make().VarDef(make().Modifiers(Flags.PRIVATE), iterVar.asName(), iterTypeExpr, null));
                 fieldNames.add(iterVar.getName());
                 JCBlock body = make().Block(0l, List.<JCStatement>of(
-                        make().If(lastIteratorCtxtName.suffixedBy(Suffix.$exhausted$).makeIdent(),
+                        /*make().If(lastIteratorCtxtName.suffixedBy(Suffix.$exhausted$).makeIdent(),
                                   make().Return(makeBoolean(false)),
-                                  null),
+                                  null),*/ // TODO temporarily removed for #869 since it seems to be non-necessary optimization
                         make().If(make().Binary(JCTree.NE, iterVar.makeIdent(), makeNull()),
                                   make().Return(makeBoolean(true)),
                                   null),
@@ -4703,7 +4775,7 @@ public class ExpressionTransformer extends AbstractTransformer {
                 fieldNames.add(kdec.getName());
                 fieldNames.add(vdec.getName());
             } else {
-                error = makeErroneous(fcl, "compiler bug: iterators of type " + fcl.getForIterator().getNodeType() + " not yet suuported");
+                error = makeErroneous(fcl, "compiler bug: iterators of type " + fcl.getForIterator().getNodeType() + " not yet supported");
                 return null;
             }
             fields.add(make().VarDef(make().Modifiers(Flags.PRIVATE), itemVar.suffixedBy(Suffix.$exhausted$).asName(),
diff --git a/src/com/redhat/ceylon/compiler/java/loader/MethodOrValueReferenceVisitor.java b/src/com/redhat/ceylon/compiler/java/loader/MethodOrValueReferenceVisitor.java
index 6b055e9..70685af 100644
--- a/src/com/redhat/ceylon/compiler/java/loader/MethodOrValueReferenceVisitor.java
+++ b/src/com/redhat/ceylon/compiler/java/loader/MethodOrValueReferenceVisitor.java
@@ -288,7 +288,7 @@ public class MethodOrValueReferenceVisitor extends Visitor {
     @Override public void visit(Tree.Comprehension that) {
         super.visit(that);
         boolean cs = enterCapturingScope();
-        that.getForComprehensionClause().visit(this);
+        that.getInitialComprehensionClause().visit(this);
         exitCapturingScope(cs);
     }

ceylon-js.patch

diff --git a/src/main/java/com/redhat/ceylon/compiler/js/ComprehensionGenerator.java b/src/main/java/com/redhat/ceylon/compiler/js/ComprehensionGenerator.java
index 9dd773e..bea5f5c 100644
--- a/src/main/java/com/redhat/ceylon/compiler/js/ComprehensionGenerator.java
+++ b/src/main/java/com/redhat/ceylon/compiler/js/ComprehensionGenerator.java
@@ -13,6 +13,7 @@ import com.redhat.ceylon.compiler.typechecker.tree.Tree.ExpressionComprehensionC
 import com.redhat.ceylon.compiler.typechecker.tree.Tree.ForComprehensionClause;
 import com.redhat.ceylon.compiler.typechecker.tree.Tree.ForIterator;
 import com.redhat.ceylon.compiler.typechecker.tree.Tree.IfComprehensionClause;
+import com.redhat.ceylon.compiler.typechecker.tree.Tree.InitialComprehensionClause;
 import com.redhat.ceylon.compiler.typechecker.tree.Tree.KeyValueIterator;
 import com.redhat.ceylon.compiler.typechecker.tree.Tree.ValueIterator;
 import com.redhat.ceylon.compiler.typechecker.tree.Tree.Variable;
@@ -47,7 +48,9 @@ class ComprehensionGenerator {
         // gather information about all loops and conditions in the comprehension
         List<ComprehensionLoopInfo> loops = new ArrayList<ComprehensionLoopInfo>();
         Expression expression = null;
-        ForComprehensionClause forClause = that.getForComprehensionClause();
+        ForComprehensionClause forClause = new ForComprehensionClause(null); // TODO
+        // TODO we can handle initial ifs properly, or we can also just prepend a for ( $unused in {null} )
+        forClause.setComprehensionClause(that.getInitialComprehensionClause());
         while (forClause != null) {
             ComprehensionLoopInfo loop = new ComprehensionLoopInfo(that, forClause.getForIterator());
             ComprehensionClause clause = forClause.getComprehensionClause();

ceylon-spec.patch

diff --git a/Ceylon.g b/Ceylon.g
index 1ef1226..3dee8ef 100644
--- a/Ceylon.g
+++ b/Ceylon.g
@@ -1848,7 +1848,9 @@ anonymousFunction returns [Expression expression]
 comprehension returns [Comprehension comprehension]
     @init { $comprehension = new Comprehension(null); }
     : forComprehensionClause
-      { $comprehension.setForComprehensionClause($forComprehensionClause.comprehensionClause); }
+      { $comprehension.setInitialComprehensionClause($forComprehensionClause.comprehensionClause); }
+    | ifComprehensionClause
+      { $comprehension.setInitialComprehensionClause($ifComprehensionClause.comprehensionClause); }
     ;
 
 comprehensionClause returns [ComprehensionClause comprehensionClause]
diff --git a/Ceylon.nodes b/Ceylon.nodes
index f1101ec..3074dc9 100644
--- a/Ceylon.nodes
+++ b/Ceylon.nodes
@@ -823,7 +823,7 @@
 
 "A comprehension."
 ^(COMPREHENSION:POSITIONAL_ARGUMENT
-    FOR_COMPREHENSION_CLAUSE)
+    INITIAL_COMPREHENSION_CLAUSE)
 
 "A single clause of comprehension"
 ^(abstract COMPREHENSION_CLAUSE:CONTROL_CLAUSE
@@ -831,16 +831,18 @@
     ProducedType firstTypeModel;
     boolean possiblyEmpty;)
 
+"A clause that can appear at the beginning of a comprehension."
+^(abstract INITIAL_COMPREHENSION_CLAUSE:COMPREHENSION_CLAUSE
+    COMPREHENSION_CLAUSE)
+
 "The expression at the end of a comprehension."
 ^(EXPRESSION_COMPREHENSION_CLAUSE:COMPREHENSION_CLAUSE
     EXPRESSION)
 
 "A quantifier clause in a comprehension."
-^(FOR_COMPREHENSION_CLAUSE:COMPREHENSION_CLAUSE
-    FOR_ITERATOR
-    COMPREHENSION_CLAUSE)
+^(FOR_COMPREHENSION_CLAUSE:INITIAL_COMPREHENSION_CLAUSE
+    FOR_ITERATOR)
 
 "A filter clause in a comprehension."
-^(IF_COMPREHENSION_CLAUSE:COMPREHENSION_CLAUSE
-    CONDITION_LIST
-    COMPREHENSION_CLAUSE)
+^(IF_COMPREHENSION_CLAUSE:INITIAL_COMPREHENSION_CLAUSE
+    CONDITION_LIST)
diff --git a/en/modules/expressions.xml b/en/modules/expressions.xml
index 77bde28..31c3360 100644
--- a/en/modules/expressions.xml
+++ b/en/modules/expressions.xml
@@ -858,13 +858,14 @@ Digit{1,2} ":" Digit{2} ( ":" Digit{2} ( ":" Digit{3} )? )?
                 </listitem>
             </itemizedlist>
             
-            <para>Every comprehension begins with a <literal>for</literal> clause, and 
-            ends with an expression clause. There may be any number of intervening 
+            <para>Every comprehension begins with a <literal>for</literal> or <literal>if</literal>
+            clause, and ends with an expression clause. There may be any number of intervening 
             <literal>for</literal> or <literal>if</literal> clauses. Each clause in the
             comprehension is considered a child of the clause that immediately precedes 
             it.</para>
             
-            <synopsis>Comprehension: ForComprehensionClause</synopsis>
+            <synopsis>Comprehension: InitialComprehensionClause</synopsis>
+            <synopsis>InitialComprehensionClause: ForComprehensionClause | IfComprehensionClause</synopsis>
             <synopsis>ForComprehensionClause: "for" ForIterator ComprehensionClause</synopsis>
             <synopsis>IfComprehensionClause: "if" ConditionList ComprehensionClause</synopsis>
             <synopsis>ComprehensionClause: ForComprehensionClause | IfComprehensionClause | Expression</synopsis>
diff --git a/src/com/redhat/ceylon/compiler/typechecker/analyzer/ExpressionVisitor.java b/src/com/redhat/ceylon/compiler/typechecker/analyzer/ExpressionVisitor.java
index b93bdfe..eca37fc 100644
--- a/src/com/redhat/ceylon/compiler/typechecker/analyzer/ExpressionVisitor.java
+++ b/src/com/redhat/ceylon/compiler/typechecker/analyzer/ExpressionVisitor.java
@@ -2755,8 +2755,8 @@ public class ExpressionVisitor extends Visitor {
     
     private void checkComprehensionIndirectArgument(Tree.Comprehension c, 
             ProducedType paramType, boolean atLeastOne) {
-        Tree.ForComprehensionClause fcc = ((Tree.Comprehension) c).getForComprehensionClause();
-        if (fcc.getPossiblyEmpty() && atLeastOne) {
+        Tree.InitialComprehensionClause icc = ((Tree.Comprehension) c).getInitialComprehensionClause();
+        if (icc.getPossiblyEmpty() && atLeastOne) {
             c.addError("variadic parameter is required but comprehension is possibly empty");
         }
         ProducedType at = c.getTypeModel();
@@ -2802,8 +2802,8 @@ public class ExpressionVisitor extends Visitor {
     
     private void checkComprehensionPositionalArgument(Parameter p, ProducedReference pr,
             Tree.Comprehension c, boolean atLeastOne) {
-        Tree.ForComprehensionClause fcc = ((Tree.Comprehension) c).getForComprehensionClause();
-        if (fcc.getPossiblyEmpty() && atLeastOne) {
+        Tree.InitialComprehensionClause icc = ((Tree.Comprehension) c).getInitialComprehensionClause();
+        if (icc.getPossiblyEmpty() && atLeastOne) {
             c.addError("variadic parameter is required but comprehension is possibly empty");
         }
         ProducedType paramType = pr.getTypedParameter(p).getFullType();
@@ -2847,7 +2847,7 @@ public class ExpressionVisitor extends Visitor {
     
     @Override public void visit(Tree.Comprehension that) {
         super.visit(that);
-        that.setTypeModel(that.getForComprehensionClause().getTypeModel());
+        that.setTypeModel(that.getInitialComprehensionClause().getTypeModel());
     }
 
     @Override public void visit(Tree.SpreadArgument that) {
@@ -4629,13 +4629,13 @@ public class ExpressionVisitor extends Visitor {
                 }
                 else if (a instanceof Tree.Comprehension) {
                     ut = et;
-                    Tree.ForComprehensionClause fcc = ((Tree.Comprehension) a).getForComprehensionClause();
-                    result = fcc.getPossiblyEmpty() ? 
+                    Tree.InitialComprehensionClause icc = ((Tree.Comprehension) a).getInitialComprehensionClause();
+                    result = icc.getPossiblyEmpty() ? 
                             unit.getSequentialType(et) : 
                             unit.getSequenceType(et);
                     if (!requireSequential) {
                         ProducedType it = producedType(unit.getIterableDeclaration(), 
-                                et, fcc.getFirstTypeModel());
+                                et, icc.getFirstTypeModel());
                         result = intersectionType(result, it, unit);
                     }
                 }