Skip to content

Replace try_method! macro with an explicit predicate and inline matches#801

Open
demoray wants to merge 1 commit into
mainfrom
bcaswell/remove-try-method-macro
Open

Replace try_method! macro with an explicit predicate and inline matches#801
demoray wants to merge 1 commit into
mainfrom
bcaswell/remove-try-method-macro

Conversation

@demoray
Copy link
Copy Markdown
Collaborator

@demoray demoray commented May 18, 2026

try_method! in src/snapshot.rs had three exit paths embedded in
one expression: succeed-and-return-from-caller, fail-fast-and-return-
from-caller, or evaluate to a Box<Error> for aggregation. Two of the
three reached into the enclosing function's control flow, so reading

let crash = try_method!(self.create_source(&Source::DevCrash));

looked like assignment but might never bind. That's the kind of
hidden control flow macros should avoid.

Extract the disk-usage special case into a named predicate on Error,
and rewrite the three call sites in Snapshot::create() as explicit
match arms. Behavior is unchanged: success returns early, a
disk-usage rejection surfaces immediately, and any other failure is
boxed and aggregated into AllSourcesFailed { crash, kcore, devmem }.
The macro is deleted.

Wins: every return is visible at its call site; the predicate has
a name and is testable in isolation; no macros hiding control flow.

Verified with:

  • cargo fmt --check
  • cargo clippy --all-features --all-targets -- -D warnings
  • cargo test --all-features

succeed-and-return-from-caller, fail-fast-and-return-from-caller, or
evaluate to a Box<Error> for aggregation. Two of the three reached
into the enclosing function's control flow, so `let crash =

Extract the disk-usage special-case into a named predicate
`Error::is_disk_usage_exceeded()` and rewrite the three call sites as
explicit `match` arms. Behavior is unchanged: on success, return
`Ok(())`; on a disk-usage rejection, surface that error immediately;
otherwise box the error for aggregation into `AllSourcesFailed`.

The macro is deleted.

Verified with:

- cargo fmt --check
- cargo clippy --all-features --all-targets -- -D warnings
- cargo test --all-features
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant