Repository: incubator-systemml Updated Branches: refs/heads/master 6b950fcf5 -> f9d10cfba
[SYSTEMML-648] Deprecate castAsScalar Add castAsScalar deprecation warnings. Add Expression javadocs. Remove unused enums from Expression. Display parse warnings if they occur without parse errors. Don't chain ParseException in JMLC Connection. Closes #130. Project: http://git-wip-us.apache.org/repos/asf/incubator-systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-systemml/commit/f9d10cfb Tree: http://git-wip-us.apache.org/repos/asf/incubator-systemml/tree/f9d10cfb Diff: http://git-wip-us.apache.org/repos/asf/incubator-systemml/diff/f9d10cfb Branch: refs/heads/master Commit: f9d10cfba029a7e2418bc4c0b57b06f2d7495efa Parents: 6b950fc Author: Deron Eriksson <[email protected]> Authored: Tue May 3 22:55:27 2016 -0700 Committer: Deron Eriksson <[email protected]> Committed: Tue May 3 22:55:27 2016 -0700 ---------------------------------------------------------------------- .../org/apache/sysml/api/jmlc/Connection.java | 5 + .../org/apache/sysml/parser/AParserWrapper.java | 18 ++ .../sysml/parser/BuiltinFunctionExpression.java | 8 +- .../org/apache/sysml/parser/Expression.java | 276 +++++++++++++++---- .../org/apache/sysml/parser/ParseException.java | 51 +--- .../parser/common/CommonSyntacticValidator.java | 23 +- .../parser/common/CustomErrorListener.java | 92 ++++++- .../sysml/parser/dml/DMLParserWrapper.java | 13 +- .../sysml/parser/dml/DmlSyntacticValidator.java | 11 +- .../sysml/parser/pydml/PyDMLParserWrapper.java | 11 +- .../parser/pydml/PydmlSyntacticValidator.java | 11 + 11 files changed, 386 insertions(+), 133 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/api/jmlc/Connection.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/api/jmlc/Connection.java b/src/main/java/org/apache/sysml/api/jmlc/Connection.java index 50e1aa0..e29d3c3 100644 --- a/src/main/java/org/apache/sysml/api/jmlc/Connection.java +++ b/src/main/java/org/apache/sysml/api/jmlc/Connection.java @@ -43,6 +43,7 @@ import org.apache.sysml.parser.AParserWrapper; import org.apache.sysml.parser.DMLProgram; import org.apache.sysml.parser.DMLTranslator; import org.apache.sysml.parser.DataExpression; +import org.apache.sysml.parser.ParseException; import org.apache.sysml.runtime.DMLRuntimeException; import org.apache.sysml.runtime.controlprogram.Program; import org.apache.sysml.runtime.controlprogram.caching.CacheableData; @@ -189,6 +190,10 @@ public class Connection implements Closeable //System.out.println(Explain.explain(rtprog)); } + catch(ParseException pe) { + // don't chain ParseException (for cleaner error output) + throw pe; + } catch(Exception ex) { throw new DMLException(ex); http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/AParserWrapper.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/AParserWrapper.java b/src/main/java/org/apache/sysml/parser/AParserWrapper.java index 7620ce3..6ae299b 100644 --- a/src/main/java/org/apache/sysml/parser/AParserWrapper.java +++ b/src/main/java/org/apache/sysml/parser/AParserWrapper.java @@ -23,6 +23,7 @@ import java.io.BufferedReader; import java.io.FileReader; import java.io.IOException; import java.io.InputStreamReader; +import java.util.List; import java.util.Map; import org.apache.commons.logging.Log; @@ -30,6 +31,7 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.sysml.conf.ConfigurationManager; import org.apache.sysml.parser.common.CommonSyntacticValidator; +import org.apache.sysml.parser.common.CustomErrorListener.ParseIssue; import org.apache.sysml.parser.dml.DMLParserWrapper; import org.apache.sysml.parser.pydml.PyDMLParserWrapper; import org.apache.sysml.runtime.util.LocalFileUtils; @@ -40,6 +42,10 @@ import org.apache.sysml.runtime.util.LocalFileUtils; */ public abstract class AParserWrapper { + protected boolean atLeastOneError = false; + protected boolean atLeastOneWarning = false; + protected List<ParseIssue> parseIssues; + /** * * @param fileName @@ -160,4 +166,16 @@ public abstract class AParserWrapper return dmlScriptStr; } + + public boolean isAtLeastOneError() { + return atLeastOneError; + } + + public boolean isAtLeastOneWarning() { + return atLeastOneWarning; + } + + public List<ParseIssue> getParseIssues() { + return parseIssues; + } } http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/BuiltinFunctionExpression.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/BuiltinFunctionExpression.java b/src/main/java/org/apache/sysml/parser/BuiltinFunctionExpression.java index 6658868..cba1daf 100644 --- a/src/main/java/org/apache/sysml/parser/BuiltinFunctionExpression.java +++ b/src/main/java/org/apache/sysml/parser/BuiltinFunctionExpression.java @@ -1497,10 +1497,12 @@ public class BuiltinFunctionExpression extends DataIdentifier } // end method getBuiltinFunctionExpression /** + * Convert a value type (double, int, or boolean) to a built-in function operator. * - * @param vt - * @return - * @throws LanguageException + * @param vt Value type ({@code ValueType.DOUBLE}, {@code ValueType.INT}, or {@code ValueType.BOOLEAN}). + * @return Built-in function operator ({@code BuiltinFunctionOp.AS_DOUBLE}, + * {@code BuiltinFunctionOp.AS_INT}, or {@code BuiltinFunctionOp.AS_BOOLEAN}). + * @throws LanguageException thrown if ValueType not accepted */ public static BuiltinFunctionOp getValueTypeCastOperator( ValueType vt ) throws LanguageException http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/Expression.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/Expression.java b/src/main/java/org/apache/sysml/parser/Expression.java index e080d76..e41654c 100644 --- a/src/main/java/org/apache/sysml/parser/Expression.java +++ b/src/main/java/org/apache/sysml/parser/Expression.java @@ -30,34 +30,50 @@ import org.apache.sysml.hops.Hop.FileFormatTypes; public abstract class Expression { + /** + * The kind of expression. Can be an operator (unary operator, binary operator, boolean operator, built-in function operator, + * parameterized built-in function operator, data operator, relational operator, external built-in function operator, function call operator), data, or literal. + */ public enum Kind { UnaryOp, BinaryOp, BooleanOp, BuiltinFunctionOp, ParameterizedBuiltinFunctionOp, DataOp, Data, Literal, RelationalOp, ExtBuiltinFunctionOp, FunctionCallOp }; + /** + * Binary operators. + */ public enum BinaryOp { PLUS, MINUS, MULT, DIV, MODULUS, INTDIV, MATMULT, POW, INVALID }; + /** + * Relational operators. + */ public enum RelationalOp { LESSEQUAL, LESS, GREATEREQUAL, GREATER, EQUAL, NOTEQUAL, INVALID }; + /** + * Boolean operators. + */ public enum BooleanOp { CONDITIONALAND, CONDITIONALOR, LOGICALAND, LOGICALOR, NOT, INVALID }; + /** + * Built-in function operators. + */ public enum BuiltinFunctionOp { - ABS, + ABS, ACOS, - ASIN, + ASIN, ATAN, AVG, CAST_AS_BOOLEAN, CAST_AS_DOUBLE, + CAST_AS_FRAME, CAST_AS_INT, CAST_AS_MATRIX, CAST_AS_SCALAR, - CAST_AS_FRAME, CBIND, //previously APPEND CEIL, CHOLESKY, @@ -122,6 +138,9 @@ public abstract class Expression VAR }; + /** + * Parameterized built-in function operators. + */ public enum ParameterizedBuiltinFunctionOp { GROUPEDAGG, RMEMPTY, REPLACE, ORDER, // Distribution Functions @@ -130,30 +149,30 @@ public abstract class Expression INVALID }; + /** + * Data operators. + */ public enum DataOp { READ, WRITE, RAND, MATRIX, INVALID } + /** + * Function call operators. + */ public enum FunctCallOp { INTERNAL, EXTERNAL }; + /** + * External built-in function operators. + */ public enum ExtBuiltinFunctionOp { EIGEN, CHOLESKY }; - public enum AggOp { - SUM, MIN, MAX, INVALID - }; - - public enum ReorgOp { - TRANSPOSE, DIAG - }; - - //public enum DataOp { - // READ, WRITE - //}; - + /** + * Data types (matrix, scalar, frame, object, unknown). + */ public enum DataType { MATRIX, SCALAR, FRAME, OBJECT, UNKNOWN; @@ -165,10 +184,16 @@ public abstract class Expression } }; + /** + * Value types (int, double, string, boolean, object, unknown). + */ public enum ValueType { INT, DOUBLE, STRING, BOOLEAN, OBJECT, UNKNOWN }; + /** + * Format types (text, binary, matrix market, csv, unknown). + */ public enum FormatType { TEXT, BINARY, MM, CSV, UNKNOWN }; @@ -198,6 +223,11 @@ public abstract class Expression return _kind; } + /** + * Obtain identifier. + * + * @return Identifier + */ public Identifier getOutput() { if (_outputs != null && _outputs.length > 0) return _outputs[0]; @@ -205,6 +235,10 @@ public abstract class Expression return null; } + /** Obtain identifiers. + * + * @return Identifiers + */ public Identifier[] getOutputs() { return _outputs; } @@ -220,7 +254,16 @@ public abstract class Expression { raiseValidateError("Should never be invoked in Baseclass 'Expression'", false); } - + + /** + * Convert string value to binary operator. + * + * @param val String value ('+', '-', '*', '/', '%%', '%/%', '^', %*%') + * @return Binary operator ({@code BinaryOp.PLUS}, {@code BinaryOp.MINUS}, + * {@code BinaryOp.MULT}, {@code BinaryOp.DIV}, {@code BinaryOp.MODULUS}, + * {@code BinaryOp.INTDIV}, {@code BinaryOp.POW}, {@code BinaryOp.MATMULT}). + * Returns {@code BinaryOp.INVALID} if string value not recognized. + */ public static BinaryOp getBinaryOp(String val) { if (val.equalsIgnoreCase("+")) return BinaryOp.PLUS; @@ -241,6 +284,15 @@ public abstract class Expression return BinaryOp.INVALID; } + /** + * Convert string value to relational operator. + * + * @param val String value ('<', '<=', '>', '>=', '==', '!=') + * @return Relational operator ({@code RelationalOp.LESS}, {@code RelationalOp.LESSEQUAL}, + * {@code RelationalOp.GREATER}, {@code RelationalOp.GREATEREQUAL}, {@code RelationalOp.EQUAL}, + * {@code RelationalOp.NOTEQUAL}). + * Returns {@code RelationalOp.INVALID} if string value not recognized. + */ public static RelationalOp getRelationalOp(String val) { if (val == null) return null; @@ -259,6 +311,14 @@ public abstract class Expression return RelationalOp.INVALID; } + /** + * Convert string value to boolean operator. + * + * @param val String value ('&&', '&', '||', '|', '!') + * @return Boolean operator ({@code BooleanOp.CONDITIONALAND}, {@code BooleanOp.LOGICALAND}, + * {@code BooleanOp.CONDITIONALOR}, {@code BooleanOp.LOGICALOR}, {@code BooleanOp.NOT}). + * Returns {@code BooleanOp.INVALID} if string value not recognized. + */ public static BooleanOp getBooleanOp(String val) { if (val.equalsIgnoreCase("&&")) return BooleanOp.CONDITIONALAND; @@ -274,22 +334,27 @@ public abstract class Expression } /** - * Convert format types from parser to Hops enum : default is text + * Convert string format type to {@code Hop.FileFormatTypes}. + * + * @param format String format type ("text", "binary", "mm", "csv") + * @return Format as {@code Hop.FileFormatTypes}. Can be + * {@code FileFormatTypes.TEXT}, {@code FileFormatTypes.BINARY}, + * {@code FileFormatTypes.MM}, or {@code FileFormatTypes.CSV}. Unrecognized + * type is set to {@code FileFormatTypes.TEXT}. */ - - public static FileFormatTypes convertFormatType(String fn) { - if (fn == null) + public static FileFormatTypes convertFormatType(String format) { + if (format == null) return FileFormatTypes.TEXT; - if (fn.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_TEXT)) { + if (format.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_TEXT)) { return FileFormatTypes.TEXT; } - if (fn.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_BINARY)) { + if (format.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_BINARY)) { return FileFormatTypes.BINARY; } - if (fn.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_MATRIXMARKET)) { + if (format.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_MATRIXMARKET)) { return FileFormatTypes.MM; } - if (fn.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_CSV)) { + if (format.equalsIgnoreCase(DataExpression.FORMAT_TYPE_VALUE_CSV)) { return FileFormatTypes.CSV; } // ToDo : throw parse exception for invalid / unsupported format type @@ -297,7 +362,10 @@ public abstract class Expression } /** - * Construct Hops from parse tree : Create temporary views in expressions + * Obtain temporary name ("parsertemp" + _tempId) for expression. Used to construct Hops from + * parse tree. + * + * @return Temporary name of expression. */ public static String getTempName() { return "parsertemp" + _tempId++; @@ -307,13 +375,36 @@ public abstract class Expression public abstract VariableSet variablesUpdated(); - public static DataType computeDataType(Expression e1, Expression e2, boolean cast) throws LanguageException { - return computeDataType(e1.getOutput(), e2.getOutput(), cast); + /** + * Compute data type based on expressions. The identifier for each expression is obtained and passed to + * {@link #computeDataType(Identifier, Identifier, boolean)}. If the identifiers have the same data type, the shared data type is + * returned. Otherwise, if {@code cast} is {@code true} and one of the identifiers is a matrix and the other + * identifier is a scalar, return {@code DataType.MATRIX}. Otherwise, throw a LanguageException. + * + * @param expression1 First expression + * @param expression2 Second expression + * @param cast Whether a cast should potentially be performed + * @return The data type ({@link DataType}) + * @throws LanguageException + */ + public static DataType computeDataType(Expression expression1, Expression expression2, boolean cast) throws LanguageException { + return computeDataType(expression1.getOutput(), expression2.getOutput(), cast); } - public static DataType computeDataType(Identifier id1, Identifier id2, boolean cast) throws LanguageException { - DataType d1 = id1.getDataType(); - DataType d2 = id2.getDataType(); + /** + * Compute data type based on identifiers. If the identifiers have the same data type, the shared data type is + * returned. Otherwise, if {@code cast} is {@code true} and one of the identifiers is a matrix and the other + * identifier is a scalar, return {@code DataType.MATRIX}. Otherwise, throw a LanguageException. + * + * @param identifier1 First identifier + * @param identifier2 Second identifier + * @param cast Whether a cast should potentially be performed + * @return The data type ({@link DataType}) + * @throws LanguageException + */ + public static DataType computeDataType(Identifier identifier1, Identifier identifier2, boolean cast) throws LanguageException { + DataType d1 = identifier1.getDataType(); + DataType d2 = identifier2.getDataType(); if (d1 == d2) return d1; @@ -326,18 +417,43 @@ public abstract class Expression } //raise error with id1 location - id1.raiseValidateError("Invalid Datatypes for operation "+d1+" "+d2, false, + identifier1.raiseValidateError("Invalid Datatypes for operation "+d1+" "+d2, false, LanguageException.LanguageErrorCodes.INVALID_PARAMETERS); return null; //never reached because unconditional } - public static ValueType computeValueType(Expression e1, Expression e2, boolean cast) throws LanguageException { - return computeValueType(e1.getOutput(), e2.getOutput(), cast); + /** + * Compute value type based on expressions. The identifier for each expression is obtained and passed to + * {@link #computeValueType(Identifier, Identifier, boolean)}. If the identifiers have the same value type, the shared value type is + * returned. Otherwise, if {@code cast} is {@code true} and one value type is a double and the other is an int, + * return {@code ValueType.DOUBLE}. If {@code cast} is {@code true} and one value type is a string or the other value type is a string, return + * {@code ValueType.STRING}. Otherwise, throw a LanguageException. + * + * @param expression1 First expression + * @param expression2 Second expression + * @param cast Whether a cast should potentially be performed + * @return The value type ({@link ValueType}) + * @throws LanguageException + */ + public static ValueType computeValueType(Expression expression1, Expression expression2, boolean cast) throws LanguageException { + return computeValueType(expression1.getOutput(), expression2.getOutput(), cast); } - - public static ValueType computeValueType(Identifier id1, Identifier id2, boolean cast) throws LanguageException { - ValueType v1 = id1.getValueType(); - ValueType v2 = id2.getValueType(); + + /** + * Compute value type based on identifiers. If the identifiers have the same value type, the shared value type is + * returned. Otherwise, if {@code cast} is {@code true} and one value type is a double and the other is an int, + * return {@code ValueType.DOUBLE}. If {@code cast} is {@code true} and one value type is a string or the other value type is a string, return + * {@code ValueType.STRING}. Otherwise, throw a LanguageException. + * + * @param identifier1 First identifier + * @param identifier2 Second identifier + * @param cast Whether a cast should potentially be performed + * @return The value type ({@link ValueType}) + * @throws LanguageException + */ + public static ValueType computeValueType(Identifier identifier1, Identifier identifier2, boolean cast) throws LanguageException { + ValueType v1 = identifier1.getValueType(); + ValueType v2 = identifier2.getValueType(); if (v1 == v2) return v1; @@ -355,7 +471,7 @@ public abstract class Expression } //raise error with id1 location - id1.raiseValidateError("Invalid Valuetypes for operation "+v1+" "+v2, false, + identifier1.raiseValidateError("Invalid Valuetypes for operation "+v1+" "+v2, false, LanguageException.LanguageErrorCodes.INVALID_PARAMETERS); return null; //never reached because unconditional } @@ -387,33 +503,51 @@ public abstract class Expression // validate error handling (consistent for all expressions) - public void raiseValidateError( String msg ) throws LanguageException { - raiseValidateError(msg, false, null); + /** + * Throw a LanguageException with the message. + * + * @param message the error message + * @throws LanguageException + */ + public void raiseValidateError( String message ) throws LanguageException { + raiseValidateError(message, false, null); } - public void raiseValidateError( String msg, boolean conditional ) throws LanguageException { - raiseValidateError(msg, conditional, null); + /** + * Throw a LanguageException with the message if conditional is {@code false}; + * otherwise log the message as a warning. + * + * @param message the error (or warning) message + * @param conditional if {@code true}, display log warning message. Otherwise, the message + * will be thrown as a LanguageException + * @throws LanguageException thrown if conditional is {@code false}. + */ + public void raiseValidateError( String message, boolean conditional ) throws LanguageException { + raiseValidateError(message, conditional, null); } /** + * Throw a LanguageException with the message (and optional error code) if conditional is {@code false}; + * otherwise log the message as a warning. * - * @param msg - * @param conditional - * @param code - * @throws LanguageException + * @param message the error (or warning) message + * @param conditional if {@code true}, display log warning message. Otherwise, the message (and optional + * error code) will be thrown as a LanguageException + * @param errorCode optional error code + * @throws LanguageException thrown if conditional is {@code false}. */ - public void raiseValidateError( String msg, boolean conditional, String errorCode ) + public void raiseValidateError( String message, boolean conditional, String errorCode ) throws LanguageException { if( conditional ) //warning if conditional { - String fullMsg = this.printWarningLocation() + msg; + String fullMsg = this.printWarningLocation() + message; LOG.warn( fullMsg ); } else //error and exception if unconditional { - String fullMsg = this.printErrorLocation() + msg; + String fullMsg = this.printErrorLocation() + message; //LOG.error( fullMsg ); //no redundant error if( errorCode != null ) @@ -428,18 +562,20 @@ public abstract class Expression * Returns the matrix characteristics for scalar-scalar, scalar-matrix, matrix-scalar, matrix-matrix * operations. This method is aware of potentially unknowns and matrix-vector (col/row) operations. * - * Format: rlen, clen, brlen, bclen. * - * @param left - * @param right - * @return + * @param expression1 The first expression + * @param expression2 The second expression + * @return long array of 4 values, where [0] is the number of rows (rlen), + * [1] is the number of columns (clen), [2] is the number of rows in a block (brlen), + * and [3] is the number of columns in a block (bclen). Default (unknown) values are + * -1. Scalar values are all 0. */ - public static long[] getBinaryMatrixCharacteristics( Expression left, Expression right ) + public static long[] getBinaryMatrixCharacteristics(Expression expression1, Expression expression2) { long[] ret = new long[]{ -1, -1, -1, -1 }; - Identifier idleft = left.getOutput(); - Identifier idright = right.getOutput(); + Identifier idleft = expression1.getOutput(); + Identifier idright = expression2.getOutput(); if( idleft.getDataType()==DataType.SCALAR && idright.getDataType()==DataType.SCALAR ) { ret[0] = 0; @@ -488,6 +624,15 @@ public abstract class Expression public void setEndColumn(int passed) { _endColumn = passed; } public void setParseExceptionList(ArrayList<String> passed) { _parseExceptionList = passed;} + /** + * Set the filename, the beginning line/column positions, and the ending line/column positions. + * + * @param filename The DML/PYDML filename (if it exists) + * @param blp Beginning line position + * @param bcp Beginning column position + * @param elp Ending line position + * @param ecp Ending column position + */ public void setAllPositions(String filename, int blp, int bcp, int elp, int ecp){ _filename = filename; _beginLine = blp; @@ -503,18 +648,29 @@ public abstract class Expression public int getEndColumn() { return _endColumn; } public ArrayList<String> getParseExceptionList() { return _parseExceptionList; } + /** + * Return error message containing the filename, the beginning line position, and the beginning column position. + * + * @return the error message + */ public String printErrorLocation(){ return "ERROR: " + _filename + " -- line " + _beginLine + ", column " + _beginColumn + " -- "; } - public String printErrorLocation(int beginLine, int beginColumn){ - return "ERROR: " + _filename + " -- line " + beginLine + ", column " + beginColumn + " -- "; - } - + /** + * Return warning message containing the filename, the beginning line position, and the beginning column position. + * + * @return the warning message + */ public String printWarningLocation(){ return "WARNING: " + _filename + " -- line " + _beginLine + ", column " + _beginColumn + " -- "; } + /** + * Return info message containing the filename, the beginning line position, and the beginning column position. + * + * @return the info message + */ public String printInfoLocation(){ return "INFO: " + _filename + " -- line " + _beginLine + ", column " + _beginColumn + " -- "; } http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/ParseException.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/ParseException.java b/src/main/java/org/apache/sysml/parser/ParseException.java index d4338bf..90691b6 100644 --- a/src/main/java/org/apache/sysml/parser/ParseException.java +++ b/src/main/java/org/apache/sysml/parser/ParseException.java @@ -22,6 +22,7 @@ package org.apache.sysml.parser; import java.util.List; import org.apache.sysml.api.DMLException; +import org.apache.sysml.parser.common.CustomErrorListener; import org.apache.sysml.parser.common.CustomErrorListener.ParseIssue; /** @@ -148,53 +149,7 @@ public class ParseException extends DMLException * @return String representing the list of parse issues. */ private String generateParseIssuesMessage() { - if (_scriptString == null) - return "No script string available."; - - String[] scriptLines = _scriptString.split("\\n"); - StringBuilder sb = new StringBuilder(); - sb.append("\n--------------------------------------------------------------"); - if (_parseIssues.size() == 1) - sb.append("\nThe following parse issue was encountered:\n"); - else - sb.append("\nThe following " + _parseIssues.size() + " parse issues were encountered:\n"); - int count = 1; - for (ParseIssue parseIssue : _parseIssues) { - if (_parseIssues.size() > 1) { - sb.append("#"); - sb.append(count++); - sb.append(" "); - } - - int issueLineNum = parseIssue.getLine(); - boolean displayScriptLine = false; - String scriptLine = null; - if ((issueLineNum > 0) && (issueLineNum <= scriptLines.length)) { - displayScriptLine = true; - scriptLine = scriptLines[issueLineNum - 1]; - } - - String name = parseIssue.getFileName(); - if (name != null) { - sb.append(name); - sb.append(" "); - } - sb.append("[line "); - sb.append(issueLineNum); - sb.append(":"); - sb.append(parseIssue.getCharPositionInLine()); - sb.append("] ["); - sb.append(parseIssue.getParseIssueType().getText()); - sb.append("]"); - if (displayScriptLine) { - sb.append(" -> "); - sb.append(scriptLine); - } - sb.append("\n "); - sb.append(parseIssue.getMessage()); - sb.append("\n"); - } - sb.append("--------------------------------------------------------------"); - return sb.toString(); + return CustomErrorListener.generateParseIssuesMessage(_scriptString, _parseIssues); } + } http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/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 8d5bb2b..e564f49 100644 --- a/src/main/java/org/apache/sysml/parser/common/CommonSyntacticValidator.java +++ b/src/main/java/org/apache/sysml/parser/common/CommonSyntacticValidator.java @@ -93,10 +93,29 @@ public abstract class CommonSyntacticValidator { errorListener.validationWarning(op.getLine(), op.getCharPositionInLine(), message); } - // Different namespaces for DML (::) and PyDml (.) + /** + * Obtain the namespace separator ({@code ::} for DML and {@code .} for + * PYDML) that is used to specify a namespace and a function in that + * namespace. + * + * @return The namespace separator + */ public abstract String namespaceResolutionOp(); - // Returns list of two elements <namespace, function names>, else null + /** + * Obtain the namespace and the function name as a two-element array based + * on the fully-qualified function name. If no namespace is supplied in + * front of the function name, the default namespace will be used. + * + * @param fullyQualifiedFunctionName + * Namespace followed by separator ({@code ::} for DML and + * {@code .} for PYDML) followed by function name (for example, + * {@code mynamespace::myfunctionname}), or only function name if + * the default namespace is used (for example, + * {@code myfunctionname}). + * @return Two-element array consisting of namespace and function name, or + * {@code null}. + */ protected String[] getQualifiedNames(String fullyQualifiedFunctionName) { String splitStr = Pattern.quote(namespaceResolutionOp()); String [] fnNames = fullyQualifiedFunctionName.split(splitStr); http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/common/CustomErrorListener.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/common/CustomErrorListener.java b/src/main/java/org/apache/sysml/parser/common/CustomErrorListener.java index 6b62499..bd7c548 100644 --- a/src/main/java/org/apache/sysml/parser/common/CustomErrorListener.java +++ b/src/main/java/org/apache/sysml/parser/common/CustomErrorListener.java @@ -34,7 +34,8 @@ public class CustomErrorListener extends BaseErrorListener { private static final Log log = LogFactory.getLog(DMLScript.class.getName()); - private boolean atleastOneError = false; + private boolean atLeastOneError = false; + private boolean atLeastOneWarning = false; private String currentFileName = null; /** @@ -71,7 +72,7 @@ public class CustomErrorListener extends BaseErrorListener { //TODO MB: we should not redundantly log errors here //(this also applies for the two other methods below). try { - setAtleastOneError(true); + setAtLeastOneError(true); // Print error messages with file name if (currentFileName == null) { log.error("line " + line + ":" + charPositionInLine + " " + msg); @@ -99,8 +100,7 @@ public class CustomErrorListener extends BaseErrorListener { ParseIssueType.VALIDATION_WARNING)); try { - // atleastOneError = true; ---> not an error, just warning - // Print error messages with file name + setAtLeastOneWarning(true); if (currentFileName == null) log.warn("line " + line + ":" + charPositionInLine + " " + msg); else { @@ -120,7 +120,7 @@ public class CustomErrorListener extends BaseErrorListener { String msg, RecognitionException e) { parseIssues.add(new ParseIssue(line, charPositionInLine, msg, currentFileName, ParseIssueType.SYNTAX_ERROR)); try { - setAtleastOneError(true); + setAtLeastOneError(true); // Print error messages with file name if (currentFileName == null) log.error("line " + line + ":" + charPositionInLine + " " + msg); @@ -133,14 +133,24 @@ public class CustomErrorListener extends BaseErrorListener { } } - public boolean isAtleastOneError() { - return atleastOneError; + public boolean isAtLeastOneError() { + return atLeastOneError; } - public void setAtleastOneError(boolean atleastOneError) { - this.atleastOneError = atleastOneError; + public void setAtLeastOneError(boolean atleastOneError) { + this.atLeastOneError = atleastOneError; + } + + public boolean isAtLeastOneWarning() { + return atLeastOneWarning; } + public void setAtLeastOneWarning(boolean atLeastOneWarning) { + this.atLeastOneWarning = atLeastOneWarning; + } + + + /** * A parse issue (such as an parse error or a parse warning). * @@ -279,6 +289,7 @@ public class CustomErrorListener extends BaseErrorListener { public enum ParseIssueType { //TODO MB: This classification is misleading as it only refers to variations of parsing issues. //We need to consolidate the handling of parsing issues and actual validation issues (see validateParseTree). + // DE: MB, please feel free to rename. SYNTAX_ERROR("Syntax error"), VALIDATION_ERROR("Validation error"), VALIDATION_WARNING("Validation warning"); ParseIssueType(String text) { @@ -326,4 +337,67 @@ public class CustomErrorListener extends BaseErrorListener { parseIssues.clear(); } + /** + * Generate a message displaying information about the parse issues that + * occurred. + * + * @param scriptString The DML or PYDML script string. + * @param parseIssues The list of parse issues. + * @return String representing the list of parse issues. + */ + public static String generateParseIssuesMessage(String scriptString, List<ParseIssue> parseIssues) { + if (scriptString == null) { + return "No script string available."; + } + if (parseIssues == null) { + return "No parse issues available."; + } + + String[] scriptLines = scriptString.split("\\n"); + StringBuilder sb = new StringBuilder(); + sb.append("\n--------------------------------------------------------------"); + if (parseIssues.size() == 1) + sb.append("\nThe following parse issue was encountered:\n"); + else + sb.append("\nThe following " + parseIssues.size() + " parse issues were encountered:\n"); + int count = 1; + for (ParseIssue parseIssue : parseIssues) { + if (parseIssues.size() > 1) { + sb.append("#"); + sb.append(count++); + sb.append(" "); + } + + int issueLineNum = parseIssue.getLine(); + boolean displayScriptLine = false; + String scriptLine = null; + if ((issueLineNum > 0) && (issueLineNum <= scriptLines.length)) { + displayScriptLine = true; + scriptLine = scriptLines[issueLineNum - 1]; + } + + String name = parseIssue.getFileName(); + if (name != null) { + sb.append(name); + sb.append(" "); + } + sb.append("[line "); + sb.append(issueLineNum); + sb.append(":"); + sb.append(parseIssue.getCharPositionInLine()); + sb.append("] ["); + sb.append(parseIssue.getParseIssueType().getText()); + sb.append("]"); + if (displayScriptLine) { + sb.append(" -> "); + sb.append(scriptLine); + } + sb.append("\n "); + sb.append(parseIssue.getMessage()); + sb.append("\n"); + } + sb.append("--------------------------------------------------------------"); + return sb.toString(); + } + } http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/dml/DMLParserWrapper.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/dml/DMLParserWrapper.java b/src/main/java/org/apache/sysml/parser/dml/DMLParserWrapper.java index 733066a..4109add 100644 --- a/src/main/java/org/apache/sysml/parser/dml/DMLParserWrapper.java +++ b/src/main/java/org/apache/sysml/parser/dml/DMLParserWrapper.java @@ -23,7 +23,6 @@ import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.util.List; import java.util.Map; import org.antlr.v4.runtime.ANTLRInputStream; @@ -44,7 +43,6 @@ import org.apache.sysml.parser.ImportStatement; import org.apache.sysml.parser.LanguageException; import org.apache.sysml.parser.ParseException; import org.apache.sysml.parser.common.CustomErrorListener; -import org.apache.sysml.parser.common.CustomErrorListener.ParseIssue; import org.apache.sysml.parser.dml.DmlParser.FunctionStatementContext; import org.apache.sysml.parser.dml.DmlParser.ProgramrootContext; import org.apache.sysml.parser.dml.DmlParser.StatementContext; @@ -179,10 +177,15 @@ public class DMLParserWrapper extends AParserWrapper DmlSyntacticValidator validator = new DmlSyntacticValidator(errorListener, argVals, sourceNamespace); walker.walk(validator, tree); errorListener.unsetCurrentFileName(); - if (errorListener.isAtleastOneError()) { - List<ParseIssue> parseIssues = errorListener.getParseIssues(); + this.parseIssues = errorListener.getParseIssues(); + this.atLeastOneWarning = errorListener.isAtLeastOneWarning(); + this.atLeastOneError = errorListener.isAtLeastOneError(); + if (atLeastOneError) { throw new ParseException(parseIssues, dmlScript); } + if (atLeastOneWarning) { + LOG.warn(CustomErrorListener.generateParseIssuesMessage(dmlScript, parseIssues)); + } dmlPgm = createDMLProgram(ast, sourceNamespace); return dmlPgm; @@ -249,6 +252,4 @@ public class DMLParserWrapper extends AParserWrapper dmlPgm.mergeStatementBlocks(); return dmlPgm; } - - } http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/dml/DmlSyntacticValidator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/dml/DmlSyntacticValidator.java b/src/main/java/org/apache/sysml/parser/dml/DmlSyntacticValidator.java index e2e1d47..56a7297 100644 --- a/src/main/java/org/apache/sysml/parser/dml/DmlSyntacticValidator.java +++ b/src/main/java/org/apache/sysml/parser/dml/DmlSyntacticValidator.java @@ -426,12 +426,19 @@ public class DmlSyntacticValidator extends CommonSyntacticValidator implements D String functionName = fnNames[1]; ArrayList<ParameterExpression> paramExpression = getParameterExpressionList(ctx.paramExprs); + castAsScalarDeprecationCheck(functionName, ctx); + boolean hasLHS = ctx.targetList != null; functionCallAssignmentStatementHelper(ctx, printStatements, outputStatements, hasLHS ? ctx.targetList.dataInfo.expr : null, ctx.info, ctx.name, hasLHS ? ctx.targetList.start : null, namespace, functionName, paramExpression, hasLHS); } - + // TODO: remove this when castAsScalar has been removed from DML/PYDML + private void castAsScalarDeprecationCheck(String functionName, ParserRuleContext ctx) { + if ("castAsScalar".equalsIgnoreCase(functionName)) { + raiseWarning("castAsScalar() has been deprecated. Please use as.scalar().", ctx.start); + } + } @Override public void exitBuiltinFunctionExpression(BuiltinFunctionExpressionContext ctx) { @@ -446,6 +453,8 @@ public class DmlSyntacticValidator extends CommonSyntacticValidator implements D ArrayList<ParameterExpression> paramExpression = getParameterExpressionList(ctx.paramExprs); + castAsScalarDeprecationCheck(functionName, ctx); + ConvertedDMLSyntax convertedSyntax = convertToDMLSyntax(ctx, namespace, functionName, paramExpression, ctx.name); if(convertedSyntax == null) { return; http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/pydml/PyDMLParserWrapper.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/pydml/PyDMLParserWrapper.java b/src/main/java/org/apache/sysml/parser/pydml/PyDMLParserWrapper.java index 0c2bd71..ed5ee5b 100644 --- a/src/main/java/org/apache/sysml/parser/pydml/PyDMLParserWrapper.java +++ b/src/main/java/org/apache/sysml/parser/pydml/PyDMLParserWrapper.java @@ -23,7 +23,6 @@ import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.util.List; import java.util.Map; import org.antlr.v4.runtime.ANTLRInputStream; @@ -45,7 +44,6 @@ import org.apache.sysml.parser.LanguageException; import org.apache.sysml.parser.ParseException; import org.apache.sysml.parser.Statement; import org.apache.sysml.parser.common.CustomErrorListener; -import org.apache.sysml.parser.common.CustomErrorListener.ParseIssue; import org.apache.sysml.parser.dml.DMLParserWrapper; import org.apache.sysml.parser.pydml.PydmlParser.FunctionStatementContext; import org.apache.sysml.parser.pydml.PydmlParser.ProgramrootContext; @@ -166,10 +164,15 @@ public class PyDMLParserWrapper extends AParserWrapper PydmlSyntacticValidator validator = new PydmlSyntacticValidator(errorListener, argVals, sourceNamespace); walker.walk(validator, tree); errorListener.unsetCurrentFileName(); - if(errorListener.isAtleastOneError()) { - List<ParseIssue> parseIssues = errorListener.getParseIssues(); + this.parseIssues = errorListener.getParseIssues(); + this.atLeastOneWarning = errorListener.isAtLeastOneWarning(); + this.atLeastOneError = errorListener.isAtLeastOneError(); + if (atLeastOneError) { throw new ParseException(parseIssues, dmlScript); } + if (atLeastOneWarning) { + LOG.warn(CustomErrorListener.generateParseIssuesMessage(dmlScript, parseIssues)); + } dmlPgm = createDMLProgram(ast, sourceNamespace); return dmlPgm; http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/f9d10cfb/src/main/java/org/apache/sysml/parser/pydml/PydmlSyntacticValidator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/pydml/PydmlSyntacticValidator.java b/src/main/java/org/apache/sysml/parser/pydml/PydmlSyntacticValidator.java index 6b2561f..ac4b7f7 100644 --- a/src/main/java/org/apache/sysml/parser/pydml/PydmlSyntacticValidator.java +++ b/src/main/java/org/apache/sysml/parser/pydml/PydmlSyntacticValidator.java @@ -963,11 +963,20 @@ public class PydmlSyntacticValidator extends CommonSyntacticValidator implements String functionName = fnNames[1]; ArrayList<ParameterExpression> paramExpression = getParameterExpressionList(ctx.paramExprs); + castAsScalarDeprecationCheck(functionName, ctx); + boolean hasLHS = ctx.targetList != null; functionCallAssignmentStatementHelper(ctx, printStatements, outputStatements, hasLHS ? ctx.targetList.dataInfo.expr : null, ctx.info, ctx.name, hasLHS ? ctx.targetList.start : null, namespace, functionName, paramExpression, hasLHS); } + // TODO: remove this when castAsScalar has been removed from DML/PYDML + private void castAsScalarDeprecationCheck(String functionName, ParserRuleContext ctx) { + if ("castAsScalar".equalsIgnoreCase(functionName)) { + raiseWarning("castAsScalar() has been deprecated. Please use scalar().", ctx.start); + } + } + @Override public void exitBuiltinFunctionExpression(BuiltinFunctionExpressionContext ctx) { // Double verification: verify passed function name is a (non-parameterized) built-in function. @@ -981,6 +990,8 @@ public class PydmlSyntacticValidator extends CommonSyntacticValidator implements ArrayList<ParameterExpression> paramExpression = getParameterExpressionList(ctx.paramExprs); + castAsScalarDeprecationCheck(functionName, ctx); + ConvertedDMLSyntax convertedSyntax = convertToDMLSyntax(ctx, namespace, functionName, paramExpression, ctx.name); if(convertedSyntax == null) { return;
