Merge ea3244f into 713013c

chanzuckerberg · Jun 16, 2019 · 118acca · 118acca
2 parents 713013c + ea3244f
commit 118acca
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 44 deletions.
diff --git a/WDL/Error.py b/WDL/Error.py
@@ -109,6 +109,10 @@ class InvalidType(ValidationError):
     pass
 
 
+class IndeterminateType(ValidationError):
+    pass
+
+
 class NoSuchTask(ValidationError):
     def __init__(self, node: Union[SourceNode, SourcePosition], name: str) -> None:
         super().__init__(node, "No such task/workflow: " + name)
@@ -154,16 +158,6 @@ def __init__(self, node: SourceNode, message: str) -> None:
         super().__init__(node, message)
 
 
-class OutOfBounds(ValidationError):
-    def __init__(self, node: SourceNode) -> None:
-        super().__init__(node, "Array index out of bounds")
-
-
-class EmptyArray(ValidationError):
-    def __init__(self, node: SourceNode) -> None:
-        super().__init__(node, "Empty array for Array+ input/declaration")
-
-
 class UnknownIdentifier(ValidationError):
     def __init__(self, node: SourceNode) -> None:
         # avoiding circular dep:
@@ -298,6 +292,16 @@ def __init__(self, node: Union[SourceNode, SourcePosition], message: str) -> Non
         super().__init__(message)
 
 
+class OutOfBounds(EvalError):
+    def __init__(self, node: SourceNode) -> None:
+        super().__init__(node, "Array index out of bounds")
+
+
+class EmptyArray(EvalError):
+    def __init__(self, node: SourceNode) -> None:
+        super().__init__(node, "Empty array for Array+ input/declaration")
+
+
 class NullValue(EvalError):
     def __init__(self, node: Union[SourceNode, SourcePosition]) -> None:
         super().__init__(node, "Null value")

diff --git a/WDL/Expr.py b/WDL/Expr.py
@@ -93,13 +93,23 @@ def typecheck(self, expected: T.Base) -> "Base":
         return self
 
     @abstractmethod
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+        # to be overridden by subclasses. eval() calls this and deals with any
+        # exceptions raised
+        pass
+
     def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         """
         Evaluate the expression in the given environment
         
         :param stdlib: a context-specific standard function library implementation
         """
-        pass
+        try:
+            return self._eval(env, stdlib)
+        except Error.EvalError:
+            raise
+        except Exception as exn:
+            raise Error.EvalError(self, str(exn)) from exn
 
 
 class Boolean(Base):
@@ -121,7 +131,7 @@ def __init__(self, pos: SourcePosition, literal: bool) -> None:
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         return T.Boolean()
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Boolean:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Boolean:
         ""
         return V.Boolean(self.value)
 
@@ -145,7 +155,7 @@ def __init__(self, pos: SourcePosition, literal: int) -> None:
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         return T.Int()
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Int:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Int:
         ""
         return V.Int(self.value)
 
@@ -172,7 +182,7 @@ def __init__(self, pos: SourcePosition, literal: float) -> None:
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         return T.Float()
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Float:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Float:
         ""
         return V.Float(self.value)
 
@@ -237,7 +247,7 @@ def _infer_type(self, type_env: Env.Types) -> T.Base:
                 )
         return T.String()
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.String:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.String:
         ""
         v = self.expr.eval(env, stdlib)
         if isinstance(v, V.Null):
@@ -285,7 +295,7 @@ def typecheck(self, expected: Optional[T.Base]) -> Base:
         ""
         return super().typecheck(expected)  # pyre-ignore
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.String:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.String:
         ""
         ans = []
         for part in self.parts:
@@ -371,7 +381,7 @@ def typecheck(self, expected: Optional[T.Base]) -> Base:
             return self
         return super().typecheck(expected)  # pyre-ignore
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Array:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Array:
         ""
         assert isinstance(self.type, T.Array)
         return V.Array(
@@ -410,7 +420,7 @@ def children(self) -> Iterable[SourceNode]:
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         return T.Pair(self.left.type, self.right.type)
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         ""
         assert isinstance(self.type, T.Pair)
         lv = self.left.eval(env, stdlib)
@@ -457,7 +467,7 @@ def _infer_type(self, type_env: Env.Types) -> T.Base:
         assert vty is not None
         return T.Map((kty, vty))
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         ""
         assert isinstance(self.type, T.Map)
         eitems = []
@@ -498,7 +508,7 @@ def _infer_type(self, type_env: Env.Types) -> T.Base:
             member_types[k] = v.type
         return T.Object(member_types)
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         ans = {}
         for k, v in self.members.items():
             ans[k] = v.eval(env, stdlib)
@@ -583,7 +593,7 @@ def _infer_type(self, type_env: Env.Types) -> T.Base:
             ) from None
         return self_type
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         ""
         try:
             if self.condition.eval(env, stdlib).expect(T.Boolean()).value:
@@ -640,7 +650,7 @@ def _infer_type(self, type_env: Env.Types) -> T.Base:
         self.ctx = Env.resolve_ctx(type_env, self.namespace, self.name)
         return ans
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         ""
         ans: V.Base = Env.resolve(env, self.namespace, self.name)
         return ans
@@ -669,7 +679,7 @@ def __init__(self, pos: SourcePosition, name: str) -> None:
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         raise NotImplementedError()
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         raise NotImplementedError()
 
     @property
@@ -787,7 +797,7 @@ def _infer_type(self, type_env: Env.Types) -> T.Base:
                 pass
         raise Error.NoSuchMember(self, self.member)
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         innard_value = self.expr.eval(env, stdlib)
         if not self.member:
             return innard_value
@@ -844,7 +854,7 @@ def _infer_type(self, type_env: Env.Types) -> T.Base:
         assert isinstance(f, StdLibFunction)
         return f.infer_type(self)
 
-    def eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
+    def _eval(self, env: Env.Values, stdlib: "Optional[WDL.StdLib.Base]" = None) -> V.Base:
         ""
         from WDL.StdLib import Base as StdLibBase, Function as StdLibFunction
 

diff --git a/WDL/StdLib.py b/WDL/StdLib.py
@@ -313,11 +313,7 @@ def infer_type(self, expr: E.Apply) -> T.Base:
 
     def _call_eager(self, expr: E.Apply, arguments: List[V.Base]) -> V.Base:
         ans_type = self.infer_type(expr)
-        try:
-            ans = self.op(arguments[0].coerce(ans_type).value, arguments[1].coerce(ans_type).value)
-        except ZeroDivisionError:
-            # TODO: different runtime error?
-            raise Error.IncompatibleOperand(expr.arguments[1], "Division by zero") from None
+        ans = self.op(arguments[0].coerce(ans_type).value, arguments[1].coerce(ans_type).value)
         if ans_type == T.Int():
             assert isinstance(ans, int)
             return V.Int(ans)
@@ -454,8 +450,7 @@ def infer_type(self, expr: E.Apply) -> T.Base:
                 expr.arguments[0], T.Array(T.Any()), expr.arguments[0].type
             )
         if isinstance(expr.arguments[0].type.item_type, T.Any):
-            # TODO: error for 'indeterminate type'
-            raise Error.EmptyArray(expr.arguments[0])
+            raise Error.IndeterminateType(expr.arguments[0], "can't infer item type of empty array")
         ty = expr.arguments[0].type.item_type
         assert isinstance(ty, T.Base)
         return ty.copy(optional=False)
@@ -480,8 +475,7 @@ def infer_type(self, expr: E.Apply) -> T.Base:
                 expr.arguments[0], T.Array(T.Any()), expr.arguments[0].type
             )
         if isinstance(expr.arguments[0].type.item_type, T.Any):
-            # TODO: error for 'indeterminate type'
-            raise Error.EmptyArray(expr.arguments[0])
+            raise Error.IndeterminateType(expr.arguments[0], "can't infer item type of empty array")
         ty = expr.arguments[0].type.item_type
         assert isinstance(ty, T.Base)
         return T.Array(ty.copy(optional=False))
@@ -503,14 +497,12 @@ def infer_type(self, expr: E.Apply) -> T.Base:
         if not isinstance(arg0ty, T.Array) or (expr._check_quant and arg0ty.optional):
             raise Error.StaticTypeMismatch(expr.arguments[0], T.Array(T.Any()), arg0ty)
         if isinstance(arg0ty.item_type, T.Any):
-            # TODO: error for 'indeterminate type'
-            raise Error.EmptyArray(expr.arguments[0])
+            raise Error.IndeterminateType(expr.arguments[0], "can't infer item type of empty array")
         arg1ty: T.Base = expr.arguments[1].type
         if not isinstance(arg1ty, T.Array) or (expr._check_quant and arg1ty.optional):
             raise Error.StaticTypeMismatch(expr.arguments[1], T.Array(T.Any()), arg1ty)
         if isinstance(arg1ty.item_type, T.Any):
-            # TODO: error for 'indeterminate type'
-            raise Error.EmptyArray(expr.arguments[1])
+            raise Error.IndeterminateType(expr.arguments[1], "can't infer item type of empty array")
         return T.Array(
             T.Pair(arg0ty.item_type, arg1ty.item_type),
             nonempty=(arg0ty.nonempty or arg1ty.nonempty),

diff --git a/WDL/Tree.py b/WDL/Tree.py
@@ -1144,8 +1144,8 @@ def _build_workflow_type_env(
         self.expr.infer_type(type_env, check_quant=check_quant)
         if not isinstance(self.expr.type, T.Array):
             raise Err.NotAnArray(self.expr)
-        if self.expr.type.item_type is None:
-            raise Err.EmptyArray(self.expr)
+        if isinstance(self.expr.type.item_type, T.Any):
+            raise Err.IndeterminateType(self.expr, "can't infer item type of empty array")
         # bind the scatter variable to the array item type within the body
         try:
             Env.resolve(type_env, [], self.variable)

diff --git a/test_corpi/contrived/scatter_collisions.wdl b/test_corpi/contrived/scatter_collisions.wdl
@@ -11,9 +11,9 @@ struct popular {
 }
 
 workflow contrived {
-    scatter (popular in []) {
+    scatter (popular in [1]) {
     }
-    scatter (contrived in []) {
+    scatter (contrived in [2]) {
     }
 }
 

diff --git a/tests/test_0eval.py b/tests/test_0eval.py
@@ -284,9 +284,9 @@ def test_errors(self):
 
     def test_short_circuit(self):
         self._test_tuples(
-            ("true && 1/0 == 1", "", WDL.Error.IncompatibleOperand),
+            ("true && 1/0 == 1", "", WDL.Error.EvalError),
             ("false && 1/0 == 1", "false"),
-            ("false || 1/0 == 1", "", WDL.Error.IncompatibleOperand),
+            ("false || 1/0 == 1", "", WDL.Error.EvalError),
             ("true || 1/0 == 1", "true"),
         )