Repository navigation
fix(coordinator): box CachedResult::Cached to fix large_enum_variant on Windows - #3001
Merged
trunk-io[bot] merged 1 commit intoAug 11, 2026
Merged
Conversation
…on Windows `CachedResult::Cached` inlined a full `eyre::Result<ControlRequestReply>`, which is large, while the sibling `Pending` variant is just a `Vec`. Clippy's `large_enum_variant` heuristic flags this on some targets (Windows) but not others (Linux), so `cargo clippy --all -- -D warnings` — the documented cross-platform pre-commit/pre-push gate — passed on Linux CI yet failed for Windows contributors on identical code, toolchain, and lockfile. Box the `result` field so `Cached` is a single pointer, making the two variants comparable in size. The two `matches!` arms that destructured the field are rewritten with `.is_err()` / `.is_ok()` guards, since box deref-patterns are unstable. Fixes #2979 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSeehXFhKtMyFH67mSLBYt
Contributor
|
😎 Merged successfully - details. |
Collaborator
Author
|
No issues found. Boxing Automated review by Claude — fully automated, not vetted by a human. Treat accordingly. Generated by Claude Code Generated by Claude Code |
GuTS805
added a commit
to GuTS805/dora
that referenced
this pull request
Aug 5, 2026
… OS-independent test The CachedResult::Cached boxing fix duplicated the already-open PR dora-rs#3001, so it's dropped here to avoid merge conflicts (per 'don't fix unrelated warnings in PRs' convention). The regression test now asserts against a hardcoded Windows-style sample path instead of the live workspace_dir() output, since on Linux/macOS CI the real path never contains a backslash and the old assertion passed trivially without exercising the normalization logic.
phil-opp
marked this pull request as ready for review
August 9, 2026 11:33
trunk-io
Bot
deleted the
claude/clever-wright-q4btae-coordinator-large-enum
branch
August 11, 2026 14:02
trunk-io Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
…ates (#3040) * fix(cli): normalize workspace path to forward slashes for CMake templates workspace_dir() returned a raw Windows path (backslash-separated), which gets substituted verbatim into __DORA_PATH__ in the generated CMakeLists.txt for --lang c/cxx --internal-create-with-path-dependencies. CMake treats backslash as a string escape character, so cmake -B failed immediately with 'Invalid character escape' before any compiler logic ran. Normalize to forward slashes, which both CMake and Windows accept. Also box the large Cached variant of CachedResult in the coordinator (pre-existing clippy::large_enum_variant failure surfaced while running clippy locally, related to #2979). * fix: address review feedback - drop unrelated coordinator change, use OS-independent test The CachedResult::Cached boxing fix duplicated the already-open PR #3001, so it's dropped here to avoid merge conflicts (per 'don't fix unrelated warnings in PRs' convention). The regression test now asserts against a hardcoded Windows-style sample path instead of the live workspace_dir() output, since on Linux/macOS CI the real path never contains a backslash and the old assertion passed trivially without exercising the normalization logic.
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
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
Fixes #2979.
CachedResultinbinaries/coordinator/src/state.rshad one large variant and one small one:ControlRequestReplyis large, soCacheddwarfsPending. Clippy'slarge_enum_variantheuristic flags this on some targets (Windows) but not others (Linux) for identical code, toolchain (1.97.1), and lockfile. Because PR CI only runsclippyon ubuntu-latest,cargo clippy --all -- -D warnings— the documented cross-platform pre-commit/pre-push gate — passed in CI yet failed for Windows contributors running the exact command the docs tell them to.Fix
Box the
resultfield soCachedbecomes a single pointer, making the two variants comparable in size:The two
matches!arms that destructured the field (is_terminal_error,is_cached_ok) are rewritten with.is_err()/.is_ok()guards, since box deref-patterns are unstable. All construction sites nowBox::new(...)the result.send_result_tois unchanged —&Box<T>deref-coerces to&Tat the call sites.Verification
cargo clippy -p dora-coordinator --all-targets -- -D warnings— passes (exit 0).cargo test -p dora-coordinator --lib—110 passed; 0 failed.Note: because the lint only fires on Windows, the fix can't be observed to remove the diagnostic on this Linux runner, but it is a strict size reduction of the flagged variant and the behavior-preserving change is covered by the existing lib tests (including the reconstruction test that asserts on
CachedResult::Cached { result } if result.is_ok()).Generated by Claude Code