fix: restore forge check/build/lint/test on latest March - #1
Merged
Merged
Conversation
- cmd_conduit.march: convert FileError to String via to_string() before concatenation (3 sites) — was a type error against latest March's stricter FileError handling. - test_conduit.march: qualify Failed(...) as Conduit.Failed to disambiguate from RemoteCall.ReplyResult's own Failed constructor. - cron_parser.march: rework list-field parsing (pparse_list_item and a new pparse_range_or_step_values) to return Result(List(Int), String) instead of Result(Field, String). The prior shape — a private helper returning a Result wrapping a multi-constructor ADT whose payload constructor wraps a List, destructured via Ok(Ctor(vs)) inside a self-recursive accumulator loop — segfaulted the compiled (march --compile) binary on any comma-separated cron field (e.g. "1,31 * * * *") while the interpreter evaluated the identical logic correctly. Confirmed via isolated repro to be a March native-codegen bug, not a logic bug; keeping pparse_list's whole recursive family on plain Result(List(Int), String) avoids the miscompile. forge check/build/lint --strict/test (167 tests, incl. property tests) all pass clean. Required a matching update to the March toolchain itself (module-resolution fixes) installed separately.
…ted)
forge.toml declared depot via { registry = "forge", version = "0.1.0" },
switched from a path dep in bcead90 ("prepare conduit 0.1.0 for forge
registry publish"). forge's RegistryDep resolution is an unbuilt
placeholder (forge/lib/cmd_deps.ml: "RegistryDep: placeholder (registry
not yet built)"), so that dependency silently resolved to nothing —
depot was completely absent from MARCH_LIB_PATH, and every depot-backed
module (Db, Pool, Connection, ParamText, ...) in postgres.march showed as
unknown. This has been the actual cause of CI failing since that commit;
none of it was related to the March version bump in this branch.
CI's workflow already clones depot to ../depot specifically for this,
so switching back to a path dep re-enables what CI was built for.
Verified against a from-scratch sandbox: fresh depot clone, march built
from upstream main (after merging march-language/march#106 and
march-language/depot#1, both required for this to typecheck) — forge
build and forge test --release both pass clean (167 tests, 0 failures).
march's resolver fix (march-language/march#106, merged) makes forge test actually typecheck every auto-discovered test file instead of silently dropping siblings the entry doesn't reference by name. That surfaced a real, previously-invisible gap here: the module imports Connection (needs IO.NetConnect) and calls random_bytes transitively via the Postgres storage backend (needs IO.Random) but declared neither.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FileError-to-Stringconversion incmd_conduit.march(3 sites) — needed explicitto_string(e)before concatenation.Failed(...)constructor intest_conduit.march(Conduit.FailedvsRemoteCall.ReplyResult's ownFailed).cron_parser.march's list-field parsing to avoid a March native-compiler codegen bug: a private helper returningResult(Field, String)(whereField's payload constructor wraps aList), destructured viaOk(Values(vs))inside a self-recursive accumulator loop, reliably segfaulted the compiled binary on any comma-separated cron field (e.g."1,31 * * * *") while the interpreter evaluated the identical logic correctly. Isolated to a ~40-line standalone repro and confirmed as a compiler bug, not a logic bug. Fixed by keeping the wholepparse_listrecursive family on plainResult(List(Int), String).Two multi-file module-resolution bugs in the March compiler itself (dedup order-dependence in
march check, and a missing filename fallback in the import resolver) were also found and fixed separately in the toolchain to unblock this — those aren't part of this diff since they live in themarchrepo, but are required forforge check/forge testto pass cleanly with this branch.Test plan
forge check— 0 errorsforge build— 0 errorsforge lint --strict— no issues foundforge test(incl. property tests) — 167 tests, 0 failures*,N,N-M,N-M/S,*/S, and comma-lists of each