Require timeout clause for check, add timeout never keyword

.github/actions/spelling/expect.txt

@@ -14,6 +14,7 @@ capsys
 cbranch
 Ccsds
 Cdh
+cfgv
 chans
 cls
 codegen

README.md

@@ -286,14 +286,16 @@ if CdhCore.cmdDisp.CommandsDispatched >= 1:
 ## Check statement
 
 A `check` statement is like an [`if`](#ifelifelse), but its condition has to hold true (or "persist") for some amount of time.
+
+Every `check` must have a `timeout` clause saying how long it is willing to wait. Use `timeout never` to wait forever:
 ```py
-check CdhCore.cmdDisp.CommandsDispatched > 30 persist {seconds: 15}:
+check CdhCore.cmdDisp.CommandsDispatched > 30 timeout never persist {seconds: 15}:
     log("more than 30 commands for 15 seconds!")
 ```
 
 If you don't specify a value for `persist`, the condition only has to be true once.
 
-You can specify a `timeout`: a `Fw.TimeInterval` duration, measured from when the `check` is entered, after which the `check` gives up:
+Instead of `never`, you can give `timeout` a `Fw.TimeInterval` duration, measured from when the `check` is entered, after which the `check` gives up:
 ```py
 check CdhCore.cmdDisp.CommandsDispatched > 30 timeout {seconds: 60} persist {seconds: 2}:
     log("more than 30 commands for 2 seconds!")
@@ -309,7 +311,7 @@ timeout:
 
 Finally, you can specify a `period` at which the condition should be checked:
 ```py
-check CdhCore.cmdDisp.CommandsDispatched > 30 period {seconds: 1}: # check every 1 second
+check CdhCore.cmdDisp.CommandsDispatched > 30 timeout never period {seconds: 1}: # check every 1 second
     log("more than 30 commands!")
 ```
 

SPEC.md

@@ -591,29 +591,37 @@ Rule:
 
 `check_stmt: "check" expr check_clause* ":" stmt_list ["timeout" ":" stmt_list]`
 
-`check_clause: "timeout" expr | "persist" expr | "freq" expr`
+`check_clause: "timeout" timeout_value | "persist" expr | "freq" expr`
 
-The clauses can appear in any order, and can be spread across multiple indented lines (with the colon after the last clause).
+`timeout_value: expr | "never"`
+
+The clauses can appear in any order, and can be spread across multiple indented lines (with the colon after the last clause). Exactly one `timeout` clause is required.
 
 Name:
 
 `check_stmt: "check" condition "timeout" timeout "persist" persist "freq" freq ":" body "timeout" ":" timeout_body`
 
-`condition`, `timeout`, `persist`, and `freq` are resolved in the value name group.
+`condition`, `persist`, and `freq` are resolved in the value name group. `timeout` is either resolved in the value name group or is the keyword `never`.
 
 ## Semantics
 
 If `condition` cannot be [coerced](#type-coercion) to [`bool`](#boolean-type), an error is raised.
 
-If `timeout` is provided, and cannot be coerced to [`Fw.Time`](todo), an error is raised.
+A `timeout` clause is required. If no `timeout` clause is provided, an error is raised. This forces the author to make an explicit choice about how long the check may run.
+
+The `timeout` clause is either an expression or the keyword `never`:
+* If it is an expression, and it cannot be coerced to [`Fw.TimeIntervalValue`](todo), an error is raised. The expression is a relative interval, measured from the moment the check is entered.
+* If it is `never`, the check never times out.
+
+If `timeout` is `never` and a `timeout_body` is provided, the `unreachable-timeout-body` warning is emitted.
 
 If `persist` or `freq` is provided, and they cannot be coerced to [`Fw.TimeIntervalValue`](todo), an error is raised.
 
 At execution:
-1. If provided, `timeout`, `persist` and `freq` are evaluated and stored.
+1. `persist` and `freq` are evaluated and stored. If `timeout` is not `never`, it is evaluated as a relative interval and the timeout deadline is stored as that interval added to the current time.
 2. If `persist` is not provided, its stored value is a zero-duration `Fw.TimeIntervalValue`.
 3. If `freq` is not provided, its stored value is a one-second `Fw.TimeIntervalValue`.
-4. If `timeout` was provided and the current time is [greater](todo) than `timeout`'s stored value, the check times out.
+4. If `timeout` is not `never` and the current time is [greater](todo) than the stored timeout deadline, the check times out.
 5. Evaluate `condition`.
 6. If `condition` has evaluated to `True` for duration greater than or equal to `persist`'s stored value, execute `body`, then continue execution after the check statement.
 7. Otherwise, [sleep](todo) for `freq`'s stored duration.
@@ -624,7 +632,8 @@ If the check times out during execution:
 2. Execution continues after the check statement.
 
 > Not providing `persist`, or providing a zero-duration `persist`, means the `condition` only needs to evaluate to `True` once.
-> The timeout defaults to never, and the frequency defaults to once per second.
+> A `timeout` of `never` means the check runs until `condition` persists, however long that takes.
+> The frequency defaults to once per second.
 
 If at any point during execution, two times which are [incomparable](todo) are attempted to be compared, the check statement will halt the program as if by an [assertion](#assert-statement), and display an error code.
 

src/fpy/desugaring.py

@@ -34,6 +34,7 @@
     CompileState,
     ForLoopAnalysis,
 )
+from fpy.error import WarningType
 from fpy.symbols import (
     FieldAccess,
     Symbol,
@@ -465,9 +466,18 @@ def visit_AstCheck(self, node: AstCheck, state: CompileState) -> list[Ast]:
         timed_out_name = self.new_var_name()
         succeeded_name = self.new_var_name()
 
-        # Check if timeout is specified (None means no timeout)
+        # Check if timeout is specified. `never` and an absent clause both leave
+        # node.timeout as None, so the per-iteration timeout check is skipped.
         has_timeout = node.timeout is not None
 
+        # `timeout never` can never time out, so a timeout body is dead code.
+        if node.timeout_never and node.timeout_body is not None:
+            state.warn(
+                WarningType.UNREACHABLE_TIMEOUT_BODY,
+                "This check has 'timeout never', so its timeout body can never run",
+                node.timeout_body,
+            )
+
         # Helper to reference check_state members
         def cs(attr: str):
             return self.member(self.ident(check_state_name), attr)

src/fpy/error.py

@@ -197,6 +197,7 @@ class WarningType(str, Enum):
 
     EMPTY_RANGE = "empty-range"
     IMPORT_SIDE_EFFECTS = "import-side-effects"
+    UNREACHABLE_TIMEOUT_BODY = "unreachable-timeout-body"
 
     @classmethod
     def from_value(cls, value: str) -> "WarningType":

src/fpy/grammar.lark

@@ -66,14 +66,17 @@ while_stmt: "while" expr ":" block
 # The clauses can appear on the same line as the condition, on separate indented lines, or mixed.
 # A body-less check (no ":" and no body) waits until the condition is satisfied, then continues.
 check_timeout: "timeout" expr
+# "timeout never" opts the check out of ever timing out (no expression)
+check_timeout_never: "timeout" "never"
 check_persist: "persist" expr
 check_period: "period" expr
 # For multi-line format: the last clause can have the colon attached
 check_timeout_final: "timeout" expr ":"
+check_timeout_never_final: "timeout" "never" ":"
 check_persist_final: "persist" expr ":"
 check_period_final: "period" expr ":"
-check_clause: check_timeout | check_persist | check_period
-check_clause_final: check_timeout_final | check_persist_final | check_period_final
+check_clause: check_timeout | check_timeout_never | check_persist | check_period
+check_clause_final: check_timeout_final | check_timeout_never_final | check_persist_final | check_period_final
 # Multi-line clauses: zero or more non-final clauses (each followed by newline),
 # followed by one final clause with colon, then statements at same indentation
 # Body-less variant: one or more clauses without a final colon, no body statements

src/fpy/syntax.py

@@ -286,11 +286,12 @@ class AstWhile(Ast):
 @dataclass
 class AstCheck(Ast):
     condition: AstExpr
-    timeout: Union[AstExpr, None]  # Default: no timeout
+    timeout: Union[AstExpr, None]  # The timeout interval, or None if `never`/absent
     persist: Union[AstExpr, None]  # Default: 0 second interval
     period: Union[AstExpr, None]  # Default: 1 second interval
     body: Union["AstBlock", None]  # None for body-less check
     timeout_body: Union["AstBlock", None] = None
+    timeout_never: bool = False  # True if the timeout clause is `never`
 
 
 @dataclass
@@ -430,6 +431,16 @@ def wrapper(self, meta, expr):
     return wrapper
 
 
+def handle_check_never_clause():
+    """Create a handler for the `timeout never` clause, which has no expression."""
+
+    @v_args(meta=True, inline=True)
+    def wrapper(self, meta):
+        return ("timeout_never", None)
+
+    return wrapper
+
+
 def handle_check_clauses(meta, children):
     """Parse multi-line check clauses and body statements.
 
@@ -458,20 +469,27 @@ def handle_check_stmt(meta, children):
 
     condition = children[0]
     timeout = None
+    timeout_never = False
     persist = None
     period = None
     body = None
     timeout_body = None
     body_set = False  # distinguish "body not yet assigned" from "body intentionally None (body-less)"
 
     def set_clause(clause_type, expr):
-        nonlocal timeout, persist, period
+        nonlocal timeout, timeout_never, persist, period
         if clause_type == "timeout":
-            if timeout is not None:
+            if timeout is not None or timeout_never:
                 raise SyntaxErrorDuringTransform(
                     f"Duplicate 'timeout' clause in check statement", expr
                 )
             timeout = expr
+        elif clause_type == "timeout_never":
+            if timeout is not None or timeout_never:
+                raise SyntaxErrorDuringTransform(
+                    f"Duplicate 'timeout' clause in check statement", condition
+                )
+            timeout_never = True
         elif clause_type == "persist":
             if persist is not None:
                 raise SyntaxErrorDuringTransform(
@@ -507,7 +525,23 @@ def set_clause(clause_type, expr):
             else:
                 timeout_body = child
 
-    return AstCheck(meta, condition, timeout, persist, period, body, timeout_body)
+    if timeout is None and not timeout_never:
+        raise SyntaxErrorDuringTransform(
+            "check statement requires a 'timeout' clause "
+            "(use 'timeout never' to never time out)",
+            condition,
+        )
+
+    return AstCheck(
+        meta,
+        condition,
+        timeout,
+        persist,
+        period,
+        body,
+        timeout_body,
+        timeout_never=timeout_never,
+    )
 
 
 def handle_parameter(meta, args):
@@ -543,9 +577,11 @@ class FpyTransformer(Transformer):
     if_stmt = AstIf
 
     check_timeout = handle_check_clause("timeout")
+    check_timeout_never = handle_check_never_clause()
     check_persist = handle_check_clause("persist")
     check_period = handle_check_clause("period")
     check_timeout_final = handle_check_clause("timeout")
+    check_timeout_never_final = handle_check_never_clause()
     check_persist_final = handle_check_clause("persist")
     check_period_final = handle_check_clause("period")