ensure line numbers are given when errors arise because of bad defaul…

…ts (fix #37)

panc/.settings/org.eclipse.core.resources.prefs

@@ -2,6 +2,4 @@ eclipse.preferences.version=1
 encoding//src/main/java=UTF-8
 encoding//src/main/resources=UTF-8
 encoding//src/test/java=UTF-8
-encoding//target/generated-sources/javacc=UTF-8
-encoding//target/generated-sources/jjtree=UTF-8
 encoding/<project>=UTF-8

panc/src/main/java/org/quattor/pan/CompilerOptions.java

@@ -346,7 +346,7 @@ public static HashResource createRootElement(String rootElement)
 
                 PanParser parser = new PanParser(new StringReader(rootElement));
                 ASTOperation ast = parser.dml();
-                Operation dml = PanParserAstUtils.astToDml(ast);
+                Operation dml = PanParserAstUtils.astToDml(ast, true);
                 return (HashResource) dml.execute(new CompileTimeContext());
 
             } catch (SyntaxException e) {

panc/src/main/java/org/quattor/pan/dml/DML.java

@@ -65,7 +65,28 @@ protected DML(SourceRange sourceRange, Operation... operations) {
 	public static Operation getInstance(SourceRange sourceRange,
 			Operation... operations) {
 
-		Operation result = null;
+		DML dml = getUnoptimizedInstance(sourceRange, operations);
+		if (dml.ops.length == 1) {
+			return dml.ops[0];
+		} else {
+			return dml;
+		}
+
+	}
+
+	/**
+	 * Factory method to create a new DML block, although this may return
+	 * another Operation because of optimization.
+	 * 
+	 * @param sourceRange
+	 *            location of this block in the source file
+	 * @param operations
+	 *            the operations that make up this block
+	 * 
+	 * @return optimized Operation representing DML block
+	 */
+	public static DML getUnoptimizedInstance(SourceRange sourceRange,
+			Operation... operations) {
 
 		// Build a duplicate list to "flatten" the DML block.
 		List<Operation> list = new LinkedList<Operation>();
@@ -82,16 +103,8 @@ public static Operation getInstance(SourceRange sourceRange,
 			}
 		}
 
-		// If the DML block contains only a single operation, then just return
-		// that operation. Otherwise create a new DML block and return that.
-		if (list.size() == 1) {
-			result = list.get(0);
-		} else {
-			Operation[] dmlOps = list.toArray(new Operation[list.size()]);
-			result = new DML(sourceRange, dmlOps);
-		}
-
-		return result;
+		Operation[] dmlOps = list.toArray(new Operation[list.size()]);
+		return new DML(sourceRange, dmlOps);
 	}
 
 	/**

panc/src/main/java/org/quattor/pan/parser/PanParserAstUtils.java

@@ -40,6 +40,7 @@ Centre National de la Recherche Scientifique (CNRS).
 import java.util.Set;
 import java.util.TreeMap;
 
+import org.quattor.pan.dml.AbstractOperation;
 import org.quattor.pan.dml.DML;
 import org.quattor.pan.dml.Operation;
 import org.quattor.pan.dml.data.Element;
@@ -436,7 +437,7 @@ static private Statement convertAstToBindStatement(String source,
 		if (child instanceof ASTFullTypeSpec) {
 			fullType = astToFullType(source, (ASTFullTypeSpec) child);
 		} else if (child instanceof ASTOperation) {
-			Operation dml = astToDml((ASTOperation) child);
+			Operation dml = astToDml((ASTOperation) child, true);
 			AliasType elementType = new AliasType(null, child.getSourceRange(),
 					"element", null);
 			fullType = new FullType(source, child.getSourceRange(),
@@ -488,7 +489,7 @@ static private Statement convertAstToAssignStatement(ASTStatement ast,
 
 			// This is a normal assignment statement.
 			ASTOperation child = (ASTOperation) ast.jjtGetChild(0);
-			Operation dml = astToDml(child);
+			Operation dml = astToDml(child, true);
 			statement = AssignmentStatement.createAssignmentStatement(
 					ast.getSourceRange(), path, dml, ast.getConditionalFlag(),
 					!ast.getFinalFlag());
@@ -510,7 +511,7 @@ static private Statement convertAstToVariableStatement(ASTStatement ast)
 
 		// Create the assignment statement.
 		ASTOperation child = (ASTOperation) ast.jjtGetChild(0);
-		Operation dml = astToDml(child);
+		Operation dml = astToDml(child, true);
 		return VariableStatement.getInstance(ast.getSourceRange(), vname, dml,
 				ast.getConditionalFlag(), !ast.getFinalFlag());
 	}
@@ -545,7 +546,7 @@ static private Statement convertAstToFunctionStatement(ASTStatement ast)
 
 		// Create the assignment statement.
 		ASTOperation child = (ASTOperation) ast.jjtGetChild(0);
-		Operation dml = astToDml(child);
+		Operation dml = astToDml(child, true);
 		return new FunctionStatement(ast.getSourceRange(), fname, dml);
 	}
 
@@ -561,7 +562,7 @@ static private Statement convertAstToIncludeStatement(ASTStatement ast)
 		assert (ast.jjtGetNumChildren() == 1);
 
 		ASTOperation child = (ASTOperation) ast.jjtGetChild(0);
-		Operation dml = astToDml(child);
+		Operation dml = astToDml(child, true);
 
 		return IncludeStatement.newIncludeStatement(ast.getSourceRange(), dml);
 
@@ -596,7 +597,7 @@ static private Path convertAstToPrefixStatement(ASTStatement ast)
 	static private FullType astToFullType(String source, ASTFullTypeSpec ast)
 			throws SyntaxException {
 		Operation withDml = null;
-		Operation defaultDml = null;
+		Element defaultValue = null;
 
 		// Sanity checks.
 		assert (ast.getId() == PanParserTreeConstants.JJTFULLTYPESPEC);
@@ -624,7 +625,11 @@ static private FullType astToFullType(String source, ASTFullTypeSpec ast)
 				assert (op.jjtGetChild(0) instanceof ASTOperation);
 				ASTOperation dml = (ASTOperation) op.jjtGetChild(0);
 				assert (dml.getOperationType() == OperationType.DML);
-				defaultDml = astToDml(dml);
+
+				// Do not optimize DML. This guarantees that the returned value
+				// is actually a DML object with the SourceRange information.
+				DML defaultDml = (DML) astToDml(dml, false);
+				defaultValue = runDefaultDml(defaultDml);
 				sourceRange = SourceRange.combineSourceRanges(sourceRange,
 						dml.getSourceRange());
 				break;
@@ -633,7 +638,7 @@ static private FullType astToFullType(String source, ASTFullTypeSpec ast)
 				assert (op.jjtGetChild(0) instanceof ASTOperation);
 				ASTOperation with = (ASTOperation) op.jjtGetChild(0);
 				assert (with.getOperationType() == OperationType.DML);
-				withDml = astToDml(with);
+				withDml = astToDml(with, false);
 				sourceRange = SourceRange.combineSourceRanges(sourceRange,
 						with.getSourceRange());
 				break;
@@ -643,7 +648,7 @@ static private FullType astToFullType(String source, ASTFullTypeSpec ast)
 		}
 
 		return new FullType(source, ast.getSourceRange(), baseType,
-				runDefaultDml(defaultDml), withDml);
+				defaultValue, withDml);
 	}
 
 	static private Operation astToOperation(SimpleNode node)
@@ -656,7 +661,7 @@ static private Operation astToOperation(SimpleNode node)
 			ASTOperation onode = (ASTOperation) node;
 			switch (onode.getOperationType()) {
 			case DML:
-				op = astToDml(onode);
+				op = astToDml(onode, true);
 				break;
 			case PLUS:
 				op = UnaryPlus.newOperation(onode.getSourceRange(),
@@ -800,7 +805,8 @@ static private Operation astToOperation(SimpleNode node)
 		return op;
 	}
 
-	static public Operation astToDml(ASTOperation node) throws SyntaxException {
+	static public Operation astToDml(ASTOperation node, boolean optimized)
+			throws SyntaxException {
 
 		// Verify that this really is a DML block.
 		assert (node.getOperationType() == OperationType.DML);
@@ -828,7 +834,11 @@ static public Operation astToDml(ASTOperation node) throws SyntaxException {
 			}
 		}
 
-		return DML.getInstance(sourceRange, operations);
+		if (optimized) {
+			return DML.getInstance(sourceRange, operations);
+		} else {
+			return DML.getUnoptimizedInstance(sourceRange, operations);
+		}
 	}
 
 	static private Operation astToIfElse(ASTOperation node)
@@ -837,13 +847,14 @@ static private Operation astToIfElse(ASTOperation node)
 		int count = node.jjtGetNumChildren();
 		assert (count >= 2);
 
-		Operation condition = astToDml((ASTOperation) node.jjtGetChild(0));
+		Operation condition = astToDml((ASTOperation) node.jjtGetChild(0), true);
 
-		Operation trueClause = astToDml((ASTOperation) node.jjtGetChild(1));
+		Operation trueClause = astToDml((ASTOperation) node.jjtGetChild(1),
+				true);
 
 		Operation falseClause = Undef.VALUE;
 		if (count == 3) {
-			falseClause = astToDml((ASTOperation) node.jjtGetChild(2));
+			falseClause = astToDml((ASTOperation) node.jjtGetChild(2), true);
 		}
 
 		return IfElse.newOperation(node.getSourceRange(), condition,
@@ -858,7 +869,7 @@ static private Operation astToFunction(ASTFunction node)
 		int count = node.jjtGetNumChildren();
 		Operation[] ops = new Operation[count];
 		for (int i = 0; i < count; i++) {
-			ops[i] = (astToDml((ASTOperation) node.jjtGetChild(i)));
+			ops[i] = (astToDml((ASTOperation) node.jjtGetChild(i), true));
 			ops[i].checkRestrictedContext();
 		}
 
@@ -923,7 +934,7 @@ static private Operation astToSetValue(ASTVariable node)
 		// Convert each of the children to a DML block and ensure that all of
 		// the contained operations can appear in a restricted context.
 		for (int i = 0; i < count; i++) {
-			ops[i] = astToDml((ASTOperation) node.jjtGetChild(i));
+			ops[i] = astToDml((ASTOperation) node.jjtGetChild(i), true);
 			ops[i].checkRestrictedContext();
 		}
 
@@ -964,7 +975,7 @@ static private Operation astToVariable(ASTVariable node, boolean lookupOnly)
 		// Convert each child index and ensure all operations are valid in a
 		// restricted context.
 		for (int i = 0; i < count; i++) {
-			ops[i] = (astToDml((ASTOperation) node.jjtGetChild(i)));
+			ops[i] = (astToDml((ASTOperation) node.jjtGetChild(i), true));
 			ops[i].checkRestrictedContext();
 		}
 
@@ -1072,14 +1083,20 @@ static private Element runDefaultDml(Operation dml) throws SyntaxException {
 		Context context = new CompileTimeContext();
 
 		Element value = null;
+
+		// IF this is an AbstractOperation, pull out the source location.
+		SourceRange sourceRange = null;
+		if (dml instanceof AbstractOperation) {
+			AbstractOperation op = (AbstractOperation) dml;
+			sourceRange = op.getSourceRange();
+		}
 
 		// Execute the DML block. The block must evaluate to an Element. Any
 		// error is fatal for the compilation.
 		try {
 			value = context.executeDmlBlock(dml);
 		} catch (EvaluationException ee) {
-			// FIXME: Need to include source information here.
-			SyntaxException se = SyntaxException.create(null,
+			SyntaxException se = SyntaxException.create(sourceRange,
 					MSG_DEF_VALUE_NOT_CONSTANT);
 			se.initCause(ee);
 			throw se;
@@ -1088,11 +1105,9 @@ static private Element runDefaultDml(Operation dml) throws SyntaxException {
 		// The default value cannot be either undef or null. Throw a syntax
 		// error if that is the case.
 		if (value instanceof Undef) {
-			// FIXME: Need to include source information here.
-			throw SyntaxException.create(null, MSG_DEF_VALUE_CANNOT_BE_UNDEF);
+			throw SyntaxException.create(sourceRange, MSG_DEF_VALUE_CANNOT_BE_UNDEF);
 		} else if (value instanceof Null) {
-			// FIXME: Need to include source information here.
-			throw SyntaxException.create(null, MSG_DEF_VALUE_CANNOT_BE_UNDEF);
+			throw SyntaxException.create(sourceRange, MSG_DEF_VALUE_CANNOT_BE_UNDEF);
 		}
 
 		// Looks Ok; return the value.

panc/src/test/pan/Functionality/bugs/bug-gh-37.tpl

@@ -0,0 +1,13 @@
+# 
+# The error message for a non-constant default value
+# must contain the line number in the error message.
+#
+# See GitHub Issue #37.
+#
+# @expect=org.quattor.pan.exceptions.SyntaxException ".*bug-gh-37\.tpl:12\.18\-12\.28.*"
+#
+object template bug-gh-37;
+
+type t = {
+    "a" : long = value("/a")
+};