diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index edb98c2..bb0a7da 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -14,6 +14,7 @@ capsys cbranch Ccsds Cdh +cfgv chans cls codegen diff --git a/README.md b/README.md index 732f366..b78da73 100644 --- a/README.md +++ b/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!") ``` diff --git a/SPEC.md b/SPEC.md index 12c1e4c..b6dff8a 100644 --- a/SPEC.md +++ b/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. diff --git a/src/fpy/desugaring.py b/src/fpy/desugaring.py index 1a8bec3..5e2d3be 100644 --- a/src/fpy/desugaring.py +++ b/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) diff --git a/src/fpy/error.py b/src/fpy/error.py index c80bd43..38902ff 100644 --- a/src/fpy/error.py +++ b/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": diff --git a/src/fpy/grammar.lark b/src/fpy/grammar.lark index 313f9cf..5a970d0 100644 --- a/src/fpy/grammar.lark +++ b/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 diff --git a/src/fpy/syntax.py b/src/fpy/syntax.py index 72a3654..57492c1 100644 --- a/src/fpy/syntax.py +++ b/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,6 +469,7 @@ def handle_check_stmt(meta, children): condition = children[0] timeout = None + timeout_never = False persist = None period = None body = None @@ -465,13 +477,19 @@ def handle_check_stmt(meta, children): 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") diff --git a/test/fpy/test_check.py b/test/fpy/test_check.py index b703cb5..f79defe 100644 --- a/test/fpy/test_check.py +++ b/test/fpy/test_check.py @@ -1,4 +1,10 @@ -from fpy.test_helpers import assert_compile_failure, assert_run_success +from fpy.test_helpers import ( + CompilationFailed, + assert_compile_failure, + assert_run_success, + compile_seq, +) +from fpy.error import WarningType class TestCheckBehavior: @@ -125,21 +131,6 @@ def test_check_timeout_body_runs_on_timeout(self, fprime_test_api): class TestCheckClauses: - def test_check_no_timeout_clause(self, fprime_test_api): - """Test check without timeout clause (runs indefinitely until success).""" - seq = """ -# Without timeout, check runs until condition succeeds -call_count: I64 = 0 - -def eventually_true() -> bool: - call_count = call_count + 1 - return call_count >= 3 - -check eventually_true(): - exit(0) -""" - assert_run_success(fprime_test_api, seq, timeout_s=10) - def test_check_only_timeout_specified(self, fprime_test_api): """Test check with only timeout specified (uses default persist=0 and period=1s).""" seq = """ @@ -167,6 +158,167 @@ def test_check_absolute_timeout(self, fprime_test_api): assert_run_success(fprime_test_api, seq) +class TestCheckTimeoutRequired: + """A timeout clause is required on every check statement.""" + + def test_check_missing_timeout_is_error(self, fprime_test_api): + """Test that a check with no timeout clause is a compile error.""" + seq = """ +check True: + pass +""" + assert_compile_failure(fprime_test_api, seq) + + def test_check_missing_timeout_with_other_clauses_is_error(self, fprime_test_api): + """Test that a check with persist and period but no timeout is a compile error.""" + seq = """ +check True persist Fw.TimeIntervalValue(0, 0) period Fw.TimeIntervalValue(0, 10000): + pass +""" + assert_compile_failure(fprime_test_api, seq) + + def test_check_missing_timeout_multiline_is_error(self, fprime_test_api): + """Test that a multi-line check with no timeout clause is a compile error.""" + seq = """ +check True + persist Fw.TimeIntervalValue(0, 0) + period Fw.TimeIntervalValue(0, 10000): + pass +""" + assert_compile_failure(fprime_test_api, seq) + + def test_check_missing_timeout_bodyless_is_error(self, fprime_test_api): + """Test that a body-less check with no timeout clause is a compile error.""" + seq = """ +check True +""" + assert_compile_failure(fprime_test_api, seq) + + +class TestCheckTimeoutNever: + """The `never` keyword opts a check out of ever timing out.""" + + def test_check_timeout_never_runs_until_success(self, fprime_test_api): + """Test check with `timeout never` runs indefinitely until the condition succeeds.""" + seq = """ +# With `timeout never`, check runs until condition succeeds +call_count: I64 = 0 + +def eventually_true() -> bool: + call_count = call_count + 1 + return call_count >= 3 + +check eventually_true() timeout never: + exit(0) +""" + assert_run_success(fprime_test_api, seq, timeout_s=10) + + def test_check_timeout_never_passes_immediately(self, fprime_test_api): + """Test that `timeout never` still runs the body once the condition holds.""" + seq = """ +body_ran: bool = False +check True timeout never: + body_ran = True +assert body_ran +""" + assert_run_success(fprime_test_api, seq) + + def test_check_timeout_never_with_persist_and_period(self, fprime_test_api): + """Test `timeout never` combined with persist and period clauses.""" + seq = """ +check_passed: bool = False +check True timeout never persist Fw.TimeIntervalValue(0, 0) period Fw.TimeIntervalValue(0, 100000): + check_passed = True +assert check_passed +""" + assert_run_success(fprime_test_api, seq) + + def test_check_timeout_never_clause_order(self, fprime_test_api): + """Test `timeout never` appearing after other clauses.""" + seq = """ +check_passed: bool = False +check True period Fw.TimeIntervalValue(0, 100000) timeout never: + check_passed = True +assert check_passed +""" + assert_run_success(fprime_test_api, seq) + + def test_check_timeout_never_multiline(self, fprime_test_api): + """Test `timeout never` spread across indented lines.""" + seq = """ +check_passed: bool = False +check True + timeout never + period Fw.TimeIntervalValue(0, 100000): + check_passed = True +assert check_passed +""" + assert_run_success(fprime_test_api, seq) + + def test_check_timeout_never_bodyless(self, fprime_test_api): + """Test a body-less check with `timeout never` waits for the condition.""" + seq = """ +counter: I64 = 0 +def counting_condition() -> bool: + counter = counter + 1 + return counter >= 3 + +check counting_condition() timeout never period Fw.TimeIntervalValue(0, 10000) +assert counter >= 3 +""" + assert_run_success(fprime_test_api, seq, timeout_s=10) + + def test_check_duplicate_timeout_never(self, fprime_test_api): + """Test that duplicate timeout clauses are an error even when one is `never`.""" + seq = """ +check True timeout never timeout Fw.TimeIntervalValue(1, 0): + pass +""" + assert_compile_failure(fprime_test_api, seq) + + +class TestCheckUnreachableTimeoutBody: + """`timeout never` together with a timeout body emits the + `unreachable-timeout-body` warning: the body can never run, but the check + still compiles (the warning is non-fatal).""" + + # `timeout never` can never time out, so this timeout body is dead code. + NEVER_WITH_TIMEOUT_BODY_SEQ = """\ +check True timeout never: + pass +timeout: + pass +""" + + def test_never_with_timeout_body_warns(self): + state, _, _ = compile_seq(self.NEVER_WITH_TIMEOUT_BODY_SEQ) + assert any( + w.type == WarningType.UNREACHABLE_TIMEOUT_BODY for w in state.warnings + ), f"expected an unreachable-timeout-body warning, got {state.warnings}" + + def test_never_with_timeout_body_still_compiles(self): + # The warning is non-fatal: compilation succeeds. + compile_seq(self.NEVER_WITH_TIMEOUT_BODY_SEQ) + + def test_never_without_timeout_body_does_not_warn(self): + state, _, _ = compile_seq("check True timeout never:\n pass\n") + assert not any( + w.type == WarningType.UNREACHABLE_TIMEOUT_BODY for w in state.warnings + ) + + def test_finite_timeout_with_timeout_body_does_not_warn(self): + # A real timeout with a timeout body is the normal, reachable case. + state, _, _ = compile_seq( + "check True timeout Fw.TimeIntervalValue(1, 0):\n" + " pass\n" + "timeout:\n" + " pass\n" + ) + assert not any( + w.type == WarningType.UNREACHABLE_TIMEOUT_BODY for w in state.warnings + ) + + class TestCheckNesting: def test_check_nested_in_function(self, fprime_test_api): @@ -332,7 +484,7 @@ def test_check_duplicate_timeout(self, fprime_test_api): def test_check_duplicate_persist(self, fprime_test_api): """Test that duplicate persist clauses cause a compile error.""" seq = """ -check True persist Fw.TimeIntervalValue(1, 0) persist Fw.TimeIntervalValue(2, 0): +check True timeout never persist Fw.TimeIntervalValue(1, 0) persist Fw.TimeIntervalValue(2, 0): pass """ assert_compile_failure(fprime_test_api, seq) @@ -340,7 +492,7 @@ def test_check_duplicate_persist(self, fprime_test_api): def test_check_duplicate_freq(self, fprime_test_api): """Test that duplicate period clauses cause a compile error.""" seq = """ -check True period Fw.TimeIntervalValue(1, 0) period Fw.TimeIntervalValue(2, 0): +check True timeout never period Fw.TimeIntervalValue(1, 0) period Fw.TimeIntervalValue(2, 0): pass """ assert_compile_failure(fprime_test_api, seq) @@ -429,21 +581,21 @@ def test_check_singleline_any_order_freq_first(self, fprime_test_api): """ assert_run_success(fprime_test_api, seq) - def test_check_singleline_only_persist(self, fprime_test_api): - """Test single-line check with only persist clause.""" + def test_check_singleline_never_and_persist(self, fprime_test_api): + """Test single-line check with `timeout never` and a persist clause.""" seq = """ check_passed: bool = False -check True persist Fw.TimeIntervalValue(0, 0): +check True timeout never persist Fw.TimeIntervalValue(0, 0): check_passed = True assert check_passed """ assert_run_success(fprime_test_api, seq) - def test_check_singleline_only_freq(self, fprime_test_api): - """Test single-line check with only period clause.""" + def test_check_singleline_never_and_period(self, fprime_test_api): + """Test single-line check with `timeout never` and a period clause.""" seq = """ check_passed: bool = False -check True period Fw.TimeIntervalValue(0, 100000): +check True timeout never period Fw.TimeIntervalValue(0, 100000): check_passed = True assert check_passed """ @@ -584,13 +736,13 @@ def check_needs_return() -> I64: """ assert_compile_failure(fprime_test_api, seq) - def test_check_no_timeout_clause_still_needs_return(self, fprime_test_api): - """Test that a check with no timeout clause still needs a trailing return + def test_check_timeout_never_still_needs_return(self, fprime_test_api): + """Test that a check with `timeout never` still needs a trailing return because the desugared if/else has an implicit else branch that doesn't return. """ seq = """ -def check_no_timeout() -> I64: - check True: +def check_never() -> I64: + check True timeout never: return 42 """ # The check desugars to: while True:...; if result: else: @@ -617,11 +769,11 @@ def test_bodyless_check_inline_with_timeout(self, fprime_test_api): """ assert_run_success(fprime_test_api, seq) - def test_bodyless_check_inline_no_clauses(self, fprime_test_api): - """Body-less check with no clauses uses defaults (True passes immediately).""" + def test_bodyless_check_inline_never(self, fprime_test_api): + """Body-less check with `timeout never` (True passes immediately).""" seq = """ done: bool = False -check True +check True timeout never done = True assert done """ @@ -637,11 +789,11 @@ def test_bodyless_check_inline_only_timeout(self, fprime_test_api): """ assert_run_success(fprime_test_api, seq) - def test_bodyless_check_inline_only_period(self, fprime_test_api): - """Body-less check with only a period clause.""" + def test_bodyless_check_inline_never_and_period(self, fprime_test_api): + """Body-less check with `timeout never` and a period clause.""" seq = """ done: bool = False -check True period Fw.TimeIntervalValue(0, 100000) +check True timeout never period Fw.TimeIntervalValue(0, 100000) done = True assert done """ diff --git a/test/fpy/test_integration.py b/test/fpy/test_integration.py index 2ff65bf..119a25f 100644 --- a/test/fpy/test_integration.py +++ b/test/fpy/test_integration.py @@ -140,7 +140,7 @@ def recurse(limit: U64): recurse(5) # prints "tick" 5 times -check CdhCore.cmdDisp.CommandsDispatched > 1 persist {seconds: 1}: +check CdhCore.cmdDisp.CommandsDispatched > 1 timeout never persist {seconds: 1}: log("more than 30 commands for 15 seconds!") check CdhCore.cmdDisp.CommandsDispatched > 1 timeout {seconds: 60} persist {seconds: 1}: log("more than 30 commands for 2 seconds!") @@ -148,7 +148,7 @@ def recurse(limit: U64): log("more than 30 commands for 2 seconds!") timeout: log("took more than 60 seconds :(") -check CdhCore.cmdDisp.CommandsDispatched > 1 period {seconds: 1}: # check every 1 second +check CdhCore.cmdDisp.CommandsDispatched > 1 timeout never period {seconds: 1}: # check every 1 second log("more than 30 commands!") check CdhCore.cmdDisp.CommandsDispatched > 1 timeout {seconds: 60}