Merge "Warn for existential variables in head pattern referred only once"

[simantics/platform.git] / bundles / org.simantics.scl.compiler / src / org / simantics / scl / compiler / elaboration / chr / CHRRule.java

diff --git a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRRule.java b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRRule.java
index 0c81daafd8d3ddfa3b36b5ec327b5ca73f0c8a43..e698d72c618ee79fca251e8689ae0267f057e5a4 100644 (file)
--- a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRRule.java
+++ b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRRule.java
@@ -1,6 +1,7 @@
 package org.simantics.scl.compiler.elaboration.chr;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 
 import org.simantics.scl.compiler.compilation.CompilationContext;
 import org.simantics.scl.compiler.elaboration.chr.plan.CHRSearchPlan;
@@ -9,17 +10,19 @@ import org.simantics.scl.compiler.elaboration.chr.relations.CHRConstraint;
 import org.simantics.scl.compiler.elaboration.contexts.SimplificationContext;
 import org.simantics.scl.compiler.elaboration.contexts.TranslationContext;
 import org.simantics.scl.compiler.elaboration.contexts.TypingContext;
+import org.simantics.scl.compiler.elaboration.expressions.EAsPattern;
 import org.simantics.scl.compiler.elaboration.expressions.EVariable;
+import org.simantics.scl.compiler.elaboration.expressions.Expression;
+import org.simantics.scl.compiler.elaboration.expressions.ExpressionVisitor;
 import org.simantics.scl.compiler.elaboration.expressions.Variable;
-import org.simantics.scl.compiler.elaboration.expressions.VariableProcedure;
 import org.simantics.scl.compiler.elaboration.expressions.printing.ExpressionToStringVisitor;
+import org.simantics.scl.compiler.elaboration.expressions.visitors.StandardExpressionVisitor;
 import org.simantics.scl.compiler.errors.Locations;
 import org.simantics.scl.compiler.internal.parsing.Symbol;
 import org.simantics.scl.compiler.types.Types;
 import org.simantics.scl.compiler.types.kinds.Kinds;
 
 import gnu.trove.map.hash.TObjectIntHashMap;
-import gnu.trove.set.hash.THashSet;
 import gnu.trove.set.hash.TIntHashSet;
 
 public class CHRRule extends Symbol {
@@ -45,6 +48,10 @@ public class CHRRule extends Symbol {
         this.body = body;
         this.existentialVariables = existentialVariables;
     }
+    
+    public CHRRule(long location, CHRQuery head, CHRQuery body) {
+        this(location, head, body, null);
+    }
 
     public void resolve(TranslationContext context) {
         context.pushExistentialFrame();
@@ -52,11 +59,52 @@ public class CHRRule extends Symbol {
         context.disallowNewExistentials();
         body.resolve(context);
         existentialVariables = context.popExistentialFrame();
+        
+        warnForExistentialsUsedOnlyOnce(context);
     }
 
-    public void collectRefs(TObjectIntHashMap<Object> allRefs, TIntHashSet refs) {
-        head.collectRefs(allRefs, refs);
-        body.collectRefs(allRefs, refs);
+    private static final Object NEVER_USED = new Object();
+    
+    private void warnForExistentialsUsedOnlyOnce(TranslationContext context) {
+        // Initialize the hash map
+        HashMap<Variable, Object> usageCount = new HashMap<>(existentialVariables.length);
+        for(Variable var : existentialVariables)
+            if(!var.getName().equals("_"))
+                usageCount.put(var, NEVER_USED);
+    
+        // Collect variable uses
+        ExpressionVisitor visitor = new StandardExpressionVisitor() {
+            private void handle(Expression expression, Variable variable) {
+                Object object = usageCount.remove(variable);
+                if(object == NEVER_USED)
+                    usageCount.put(variable, expression);
+            }
+            @Override
+            public void visit(EVariable expression) {
+                if(expression.variable != null)
+                    handle(expression, expression.variable);
+            }
+            @Override
+            public void visit(EAsPattern expression) {
+                expression.pattern.accept(this);
+                handle(expression, expression.var);
+            }
+        };
+        head.accept(visitor);
+        body.accept(visitor);
+        
+        // Report as warnings
+        usageCount.forEach((variable, expression_) -> {
+            if(!(expression_ instanceof Expression))
+                return; // Should never happen
+            Expression expression = (Expression)expression_;
+            if(context.isExpandedFromWildcard(expression))
+                return;
+            
+            context.getErrorLog().logWarning(expression.location,
+                    "Existential variable " + variable.getName() + " is referred only once. Replace by _ if this is a wildcard.");
+        });
+        
     }
 
     public void checkType(TypingContext context) {
@@ -71,16 +119,6 @@ public class CHRRule extends Symbol {
         body.collectVars(allVars, vars);
     }
 
-    public void forVariables(VariableProcedure procedure) {
-        head.forVariables(procedure);
-        body.forVariables(procedure);
-    }
-
-    public void collectFreeVariables(THashSet<Variable> vars) {
-        head.collectFreeVariables(vars);
-        body.collectFreeVariables(vars);
-    }
-
     public void setLocationDeep(long loc) {
         if(location == Locations.NO_LOCATION) {
             this.location = loc;
@@ -104,7 +142,7 @@ public class CHRRule extends Symbol {
             
             Variable activeFact = new Variable("activeFact", constraint.factType);
             QueryPlanningContext context = new QueryPlanningContext(compilationContext, existentialVariables);
-            if(!head.createQueryPlan(context, new EVariable(activeFact), i))
+            if(!head.createQueryPlan(context, new EVariable(activeFact), i, initConstraint))
                 return;
             body.createEnforcePlan(context, priority);
             addPlan(new CHRSearchPlan(constraint, activeFact, context.getPlanOps()));
@@ -115,7 +153,7 @@ public class CHRRule extends Symbol {
         if(!hasLocalActiveLiteral) {
             Variable activeFact = new Variable("activeFact", initConstraint.factType);
             QueryPlanningContext context = new QueryPlanningContext(compilationContext, existentialVariables);
-            if(!head.createQueryPlan(context, null, -1))
+            if(!head.createQueryPlan(context, new EVariable(activeFact), -1, initConstraint))
                 return;
             body.createEnforcePlan(context, priority);
             /*System.out.println(this);