Repository: incubator-systemml Updated Branches: refs/heads/master c7eebddb1 -> 3aa32e50c
[SYSTEMML-259] Function with no return value not require lvalue If a user-defined function does not return a value, don't require that the function is assigned to a variable since there is nothing to assign. Add corresponding MLContext tests. Closes #411. Project: http://git-wip-us.apache.org/repos/asf/incubator-systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-systemml/commit/3aa32e50 Tree: http://git-wip-us.apache.org/repos/asf/incubator-systemml/tree/3aa32e50 Diff: http://git-wip-us.apache.org/repos/asf/incubator-systemml/diff/3aa32e50 Branch: refs/heads/master Commit: 3aa32e50c0c1d6aef3aba86473b6c0aa1fcd42a8 Parents: c7eebdd Author: Deron Eriksson <de...@us.ibm.com> Authored: Mon Mar 6 15:26:37 2017 -0800 Committer: Deron Eriksson <de...@us.ibm.com> Committed: Mon Mar 6 15:26:37 2017 -0800 ---------------------------------------------------------------------- docs/beginners-guide-to-dml-and-pydml.md | 1 - .../sysml/parser/AssignmentStatement.java | 7 ++- .../org/apache/sysml/parser/StatementBlock.java | 29 +++++++++- .../parser/common/CommonSyntacticValidator.java | 7 +-- .../integration/mlcontext/MLContextTest.java | 60 ++++++++++++++++++++ 5 files changed, 93 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/3aa32e50/docs/beginners-guide-to-dml-and-pydml.md ---------------------------------------------------------------------- diff --git a/docs/beginners-guide-to-dml-and-pydml.md b/docs/beginners-guide-to-dml-and-pydml.md index e82909d..9d19cc8 100644 --- a/docs/beginners-guide-to-dml-and-pydml.md +++ b/docs/beginners-guide-to-dml-and-pydml.md @@ -641,7 +641,6 @@ parfor(i in 0:nrow(A)-1): Functions encapsulate useful functionality in SystemML. In addition to built-in functions, users can define their own functions. Functions take 0 or more parameters and return 0 or more values. -Currently, if a function returns nothing, it still needs to be assigned to a variable. <div class="codetabs2"> http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/3aa32e50/src/main/java/org/apache/sysml/parser/AssignmentStatement.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/AssignmentStatement.java b/src/main/java/org/apache/sysml/parser/AssignmentStatement.java index b4d82f6..b59887b 100644 --- a/src/main/java/org/apache/sysml/parser/AssignmentStatement.java +++ b/src/main/java/org/apache/sysml/parser/AssignmentStatement.java @@ -121,14 +121,17 @@ public class AssignmentStatement extends Statement // add target to updated list for (DataIdentifier target : _targetList) - result.addVariable(target.getName(), target); + if (target != null) { + result.addVariable(target.getName(), target); + } return result; } public String toString(){ StringBuilder sb = new StringBuilder(); for (int i=0; i< _targetList.size(); i++){ - sb.append(_targetList.get(i).toString()); + DataIdentifier di = _targetList.get(i); + sb.append(di); } sb.append(" = "); if (_source instanceof StringIdentifier) { http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/3aa32e50/src/main/java/org/apache/sysml/parser/StatementBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/StatementBlock.java b/src/main/java/org/apache/sysml/parser/StatementBlock.java index 74e707a..8115166 100644 --- a/src/main/java/org/apache/sysml/parser/StatementBlock.java +++ b/src/main/java/org/apache/sysml/parser/StatementBlock.java @@ -466,7 +466,23 @@ public class StatementBlock extends LiveVariableAnalysis Statement rewrittenStmt = stmt.rewriteStatement(prefix); newStatements.add(rewrittenStmt); } - + + if (current instanceof AssignmentStatement) { + if (fstmt.getOutputParams().size() == 0) { + AssignmentStatement as = (AssignmentStatement) current; + if ((as.getTargetList().size() == 1) && (as.getTargetList().get(0) != null)) { + raiseValidateError("Function '" + fcall.getName() + + "' does not return a value but is assigned to " + as.getTargetList().get(0), + true); + } + } + } else if (current instanceof MultiAssignmentStatement) { + if (fstmt.getOutputParams().size() == 0) { + MultiAssignmentStatement mas = (MultiAssignmentStatement) current; + raiseValidateError("Function '" + fcall.getName() + + "' does not return a value but is assigned to " + mas.getTargetList(), true); + } + } // handle the return values for (int i = 0; i < fstmt.getOutputParams().size(); i++){ @@ -482,7 +498,16 @@ public class StatementBlock extends LiveVariableAnalysis if (i > 0) { fstmt.raiseValidateError("Assignment statement cannot return multiple values", false); } - newTarget = new DataIdentifier(((AssignmentStatement)current).getTarget()); + AssignmentStatement as = (AssignmentStatement) current; + DataIdentifier targ = as.getTarget(); + if (targ == null) { + Expression exp = as.getSource(); + FunctionCallIdentifier fci = (FunctionCallIdentifier) exp; + String functionName = fci.getName(); + fstmt.raiseValidateError(functionName + " requires LHS value", false); + } else { + newTarget = new DataIdentifier(((AssignmentStatement)current).getTarget()); + } } else{ newTarget = new DataIdentifier(((MultiAssignmentStatement)current).getTargetList().get(i)); http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/3aa32e50/src/main/java/org/apache/sysml/parser/common/CommonSyntacticValidator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/common/CommonSyntacticValidator.java b/src/main/java/org/apache/sysml/parser/common/CommonSyntacticValidator.java index e114f5e..f76694e 100644 --- a/src/main/java/org/apache/sysml/parser/common/CommonSyntacticValidator.java +++ b/src/main/java/org/apache/sysml/parser/common/CommonSyntacticValidator.java @@ -704,16 +704,11 @@ public abstract class CommonSyntacticValidator { } } - if (!hasLHS){ - notifyErrorListeners("function call needs to have lvalue (Quickfix: change it to \'tmpVar = " + functionName + "(...)\')", nameToken); - return; - } - DataIdentifier target = null; if(dataInfo instanceof DataIdentifier) { target = (DataIdentifier) dataInfo; } - else { + else if (dataInfo != null) { notifyErrorListeners("incorrect lvalue for function call ", targetListToken); return; } http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/3aa32e50/src/test/java/org/apache/sysml/test/integration/mlcontext/MLContextTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/sysml/test/integration/mlcontext/MLContextTest.java b/src/test/java/org/apache/sysml/test/integration/mlcontext/MLContextTest.java index f6ef208..c8d3450 100644 --- a/src/test/java/org/apache/sysml/test/integration/mlcontext/MLContextTest.java +++ b/src/test/java/org/apache/sysml/test/integration/mlcontext/MLContextTest.java @@ -2665,6 +2665,66 @@ public class MLContextTest extends AutomatedTestBase { ml.execute(script); } + @Test + public void testFunctionNoReturnValueDML() { + System.out.println("MLContextTest - function with no return value DML"); + + String s = "hello=function(){print('no return value')}\nhello();"; + Script script = dml(s); + setExpectedStdOut("no return value"); + ml.execute(script); + } + + @Test + public void testFunctionNoReturnValuePYDML() { + System.out.println("MLContextTest - function with no return value PYDML"); + + String s = "def hello():\n\tprint('no return value')\nhello()"; + Script script = pydml(s); + setExpectedStdOut("no return value"); + ml.execute(script); + } + + @Test + public void testFunctionReturnValueDML() { + System.out.println("MLContextTest - function with return value DML"); + + String s = "hello=function()return(string s){s='return value'}\na=hello();\nprint(a);"; + Script script = dml(s); + setExpectedStdOut("return value"); + ml.execute(script); + } + + @Test + public void testFunctionReturnValuePYDML() { + System.out.println("MLContextTest - function with return value PYDML"); + + String s = "def hello()->(s:str):\n\ts='return value'\na=hello()\nprint(a)"; + Script script = pydml(s); + setExpectedStdOut("return value"); + ml.execute(script); + } + + @Test + public void testFunctionTwoReturnValuesDML() { + System.out.println("MLContextTest - function with two return values DML"); + + String s = "hello=function()return(string s1,string s2){s1='return'; s2='values'}\n[a,b]=hello();\nprint(a+' '+b);"; + Script script = dml(s); + setExpectedStdOut("return values"); + ml.execute(script); + } + + @Test + public void testFunctionTwoReturnValuesPYDML() { + System.out.println("MLContextTest - function with two return values PYDML"); + + String s = "def hello()->(s1:str,s2:str):\n\ts1='return'\n\ts2='values'\n[a,b]=hello()\nprint(a+' '+b)"; + Script script = pydml(s); + setExpectedStdOut("return values"); + ml.execute(script); + } + // NOTE: Uncomment these tests once they work // @SuppressWarnings({ "rawtypes", "unchecked" })
