From: Hannu Niemistö Date: Thu, 28 Feb 2019 12:52:47 +0000 (+0200) Subject: Warn for existential variables in head pattern referred only once X-Git-Url: https://gerrit.simantics.org/r/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F49%2F2749%2F1;p=simantics%2Fplatform.git Warn for existential variables in head pattern referred only once #265 Change-Id: Ie022bbfb0b1829bc462bf88f44f65009464de096 (cherry picked from commit 413eb9409ab556f22293d28588b39f46d8449718) --- diff --git a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRQuery.java b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRQuery.java index 7bee6eae0..6b01d9953 100644 --- a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRQuery.java +++ b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/chr/CHRQuery.java @@ -10,8 +10,11 @@ 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.Expression; +import org.simantics.scl.compiler.elaboration.expressions.ExpressionVisitor; import org.simantics.scl.compiler.elaboration.expressions.Variable; import org.simantics.scl.compiler.elaboration.expressions.printing.ExpressionToStringVisitor; +import org.simantics.scl.compiler.elaboration.expressions.visitors.ForVariablesUsesVisitor; +import org.simantics.scl.compiler.elaboration.expressions.visitors.StandardExpressionVisitor; import org.simantics.scl.compiler.errors.Locations; import org.simantics.scl.compiler.internal.parsing.Symbol; @@ -92,4 +95,16 @@ public class CHRQuery extends Symbol { newLiterals[i] = literals[i].replace(context); return new CHRQuery(location, newLiterals); } + + public void accept(ExpressionVisitor visitor) { + for(CHRLiteral literal : literals) { + if(literal == null || literal.parameters == null) + continue; // FIXME why this happens? + for(Expression parameter : literal.parameters) { + if(parameter == null) + continue; // FIXME why this happens? + parameter.accept(visitor); + } + } + } } 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 78224ebb0..e698d72c6 100644 --- 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,9 +10,13 @@ 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.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; @@ -54,6 +59,52 @@ public class CHRRule extends Symbol { context.disallowNewExistentials(); body.resolve(context); existentialVariables = context.popExistentialFrame(); + + warnForExistentialsUsedOnlyOnce(context); + } + + private static final Object NEVER_USED = new Object(); + + private void warnForExistentialsUsedOnlyOnce(TranslationContext context) { + // Initialize the hash map + HashMap 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) { diff --git a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/contexts/TranslationContext.java b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/contexts/TranslationContext.java index d8346f41f..e0c256599 100644 --- a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/contexts/TranslationContext.java +++ b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/contexts/TranslationContext.java @@ -76,6 +76,8 @@ public class TranslationContext extends TypeTranslationContext implements Enviro TIntArrayList chrConstraintFrames = new TIntArrayList(); ArrayList chrConstraintEntries = new ArrayList(); + private THashSet expandedFromWildcard; + public CHRRuleset currentRuleset; public ModuleDebugInfo moduleDebugInfo; @@ -141,7 +143,7 @@ public class TranslationContext extends TypeTranslationContext implements Enviro variable = new Variable(name); variables.put(name, variable); existentialFrame.variables.add(name); - return new EVariable(variable); + return new EVariable(location, variable); } case '_': { if(name.length()==1) { @@ -583,7 +585,29 @@ public class TranslationContext extends TypeTranslationContext implements Enviro return Environments.getRuleset(environment, name); } + /** + * Tells that new existential variables are no longer allowed in this context. + */ public void disallowNewExistentials() { getCurrentExistentialFrame().disallowNewExistentials = true; } + + /** + * Marks that the expression is a result of expanding .. wildcard pattern in records. + */ + public void addExpandedFromWildcard(Expression expression) { + if(expandedFromWildcard == null) + expandedFromWildcard = new THashSet<>(); + expandedFromWildcard.add(expression); + } + + /** + * Asks if the expression is a result of expanding .. wildcard pattern in records. + */ + public boolean isExpandedFromWildcard(Expression expression) { + if(expandedFromWildcard == null) + return false; + else + return expandedFromWildcard.contains(expression); + } } diff --git a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/ERecord.java b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/ERecord.java index 47ef205df..cb34d0fb3 100644 --- a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/ERecord.java +++ b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/ERecord.java @@ -150,7 +150,9 @@ public class ERecord extends ASTExpression { String variableName = fieldNames[i]; if(chrLiteral) variableName = "?" + variableName; - parameters[i] = new EVar(wildcardField.location, variableName); + EVar expandedVar = new EVar(wildcardField.location, variableName); + parameters[i] = expandedVar; + context.addExpandedFromWildcard(expandedVar); } } if(!recordMap.isEmpty()) { diff --git a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/EVar.java b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/EVar.java index d78fa8565..2e4809a0c 100644 --- a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/EVar.java +++ b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/EVar.java @@ -49,7 +49,10 @@ public class EVar extends ASTExpression { @Override public Expression resolve(TranslationContext context) { - return context.resolveVariable(location, name); + Expression resolved = context.resolveVariable(location, name); + if(context.isExpandedFromWildcard(this)) + context.addExpandedFromWildcard(resolved); + return resolved; } @Override @@ -59,7 +62,10 @@ public class EVar extends ASTExpression { @Override public Expression resolveAsPattern(TranslationContext context) { - return context.resolvePattern(this); + Expression resolved = context.resolvePattern(this); + if(context.isExpandedFromWildcard(this)) + context.addExpandedFromWildcard(resolved); + return resolved; } @Override diff --git a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/VariableProcedure.java b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/VariableProcedure.java index 44e9b6ae7..af50692fa 100644 --- a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/VariableProcedure.java +++ b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/elaboration/expressions/VariableProcedure.java @@ -1,5 +1,6 @@ package org.simantics.scl.compiler.elaboration.expressions; +@FunctionalInterface public interface VariableProcedure { void execute(long location, Variable variable); } diff --git a/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR4.scl b/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR4.scl index dc0714973..90b82e79a 100644 --- a/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR4.scl +++ b/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR4.scl @@ -3,7 +3,9 @@ main = () when ?x <- ?y then True -- +3:10-3:12: Existential variable ?x is referred only once. Replace by _ if this is a wildcard. 3:10-3:18: Cannot solve the query. +3:16-3:18: Existential variable ?y is referred only once. Replace by _ if this is a wildcard. -- import "Prelude" diff --git a/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR8.scl b/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR8.scl index dd915ebb5..6929a0b92 100644 --- a/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR8.scl +++ b/tests/org.simantics.scl.compiler.tests/src/org/simantics/scl/compiler/tests/scl/CHR8.scl @@ -5,4 +5,5 @@ main = () where X ?x => Y ?y -- +6:7-6:9: Existential variable ?x is referred only once. Replace by _ if this is a wildcard. 6:15-6:17: New existential variables can be defined only in queries. \ No newline at end of file