Skip to content

Rewrite query_ensure_result.#154075

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
nnethercote:rewrite-query_ensure_result
Mar 20, 2026
Merged

Rewrite query_ensure_result.#154075
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
nnethercote:rewrite-query_ensure_result

Conversation

@nnethercote
Copy link
Contributor

It currently uses chaining which is concise but hard to read. There are up to four implicit matches occurring after the call to execute_query_fn.

  • The first map on Option<Erased<Result<T, ErrorGuaranteed>>>.
  • The second map on Option<Result<T, ErrorGuaranteed>>.
  • The third map on Result<T, ErrorGuaranteed>.
  • The unwrap_or on Option<Result<(), ErrorGuaranteed>>.

This commit rewrites it to use at most two matches.

  • An explicit match on Option<Erased<Result<T, ErrorGuaranteed>>>.
  • An explicit match on Result<T, ErrorGuaranteed>.

This is easier to read. It's also more efficient, though the code isn't hot enough for that to matter.

r? @Zalathar

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 19, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 19, 2026

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

@nnethercote nnethercote force-pushed the rewrite-query_ensure_result branch from cf1ff55 to d7ba9ed Compare March 19, 2026 07:53
@rustbot
Copy link
Collaborator

rustbot commented Mar 19, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@nnethercote
Copy link
Contributor Author

Let's run perf just in case.

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Mar 19, 2026
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 19, 2026
QueryMode::Ensure { ensure_mode: EnsureMode::Ok },
) {
// We executed the query. Convert the successful result.
Some(res) => convert(res),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: I think value is better than res here (and above), as it’s consistent with the other cache lookups on this file, and we can’t treat the value as a Result without unerasing it first anyway.

@Zalathar
Copy link
Member

r=me after considering the res/value feedback, assuming perf is good.

It currently uses chaining which is concise but hard to read. There are
up to four implicit matches occurring after the call to
`execute_query_fn`.
- The first `map` on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- The second `map` on `Option<Result<T, ErrorGuaranteed>>`.
- The third `map` on `Result<T, ErrorGuaranteed>`.
- The `unwrap_or` on `Option<Result<(), ErrorGuaranteed>>`.

This commit rewrites it to use at most two matches.
- An explicit match on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- An explicit match on `Result<T, ErrorGuaranteed>`.

This is easier to read. It's also more efficient, though the code isn't
hot enough for that to matter.
@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 19, 2026

☀️ Try build successful (CI)
Build commit: 936d6e3 (936d6e3bcbc7a2764e21d35c7e1d90ab67157121, parent: fd0c901b00ee1e08a250039cdb90258603497e20)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (936d6e3): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -4.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.5% [-4.5%, -4.5%] 1
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 481.003s -> 479.608s (-0.29%)
Artifact size: 396.91 MiB -> 394.93 MiB (-0.50%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 19, 2026
@nnethercote nnethercote force-pushed the rewrite-query_ensure_result branch from d7ba9ed to 74776c4 Compare March 19, 2026 11:35
@nnethercote
Copy link
Contributor Author

@bors r=Zalathar rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 19, 2026

📌 Commit 74776c4 has been approved by Zalathar

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 19, 2026
…esult, r=Zalathar

Rewrite `query_ensure_result`.

It currently uses chaining which is concise but hard to read. There are up to four implicit matches occurring after the call to `execute_query_fn`.
- The first `map` on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- The second `map` on `Option<Result<T, ErrorGuaranteed>>`.
- The third `map` on `Result<T, ErrorGuaranteed>`.
- The `unwrap_or` on `Option<Result<(), ErrorGuaranteed>>`.

This commit rewrites it to use at most two matches.
- An explicit match on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- An explicit match on `Result<T, ErrorGuaranteed>`.

This is easier to read. It's also more efficient, though the code isn't hot enough for that to matter.

r? @Zalathar
rust-bors bot pushed a commit that referenced this pull request Mar 19, 2026
…uwer

Rollup of 9 pull requests

Successful merges:

 - #153556 (`impl` restriction lowering)
 - #153992 (bootstrap: Optionally print a backtrace if a command fails)
 - #154019 (two smaller feature cleanups)
 - #154059 (tests: Activate `must_not_suspend` test for `MutexGuard` dropped before `await`)
 - #154075 (Rewrite `query_ensure_result`.)
 - #154082 (Updates derive_where and removes workaround)
 - #154084 (Preserve braces around `self` in use tree pretty printing)
 - #154086 (Insert space after float literal ending with `.` in pretty printer)
 - #154087 (Fix whitespace after fragment specifiers in macro pretty printing)
rust-bors bot pushed a commit that referenced this pull request Mar 19, 2026
…uwer

Rollup of 9 pull requests

Successful merges:

 - #153556 (`impl` restriction lowering)
 - #153992 (bootstrap: Optionally print a backtrace if a command fails)
 - #154019 (two smaller feature cleanups)
 - #154059 (tests: Activate `must_not_suspend` test for `MutexGuard` dropped before `await`)
 - #154075 (Rewrite `query_ensure_result`.)
 - #154082 (Updates derive_where and removes workaround)
 - #154084 (Preserve braces around `self` in use tree pretty printing)
 - #154086 (Insert space after float literal ending with `.` in pretty printer)
 - #154087 (Fix whitespace after fragment specifiers in macro pretty printing)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 19, 2026
…esult, r=Zalathar

Rewrite `query_ensure_result`.

It currently uses chaining which is concise but hard to read. There are up to four implicit matches occurring after the call to `execute_query_fn`.
- The first `map` on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- The second `map` on `Option<Result<T, ErrorGuaranteed>>`.
- The third `map` on `Result<T, ErrorGuaranteed>`.
- The `unwrap_or` on `Option<Result<(), ErrorGuaranteed>>`.

This commit rewrites it to use at most two matches.
- An explicit match on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- An explicit match on `Result<T, ErrorGuaranteed>`.

This is easier to read. It's also more efficient, though the code isn't hot enough for that to matter.

r? @Zalathar
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 19, 2026
…esult, r=Zalathar

Rewrite `query_ensure_result`.

It currently uses chaining which is concise but hard to read. There are up to four implicit matches occurring after the call to `execute_query_fn`.
- The first `map` on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- The second `map` on `Option<Result<T, ErrorGuaranteed>>`.
- The third `map` on `Result<T, ErrorGuaranteed>`.
- The `unwrap_or` on `Option<Result<(), ErrorGuaranteed>>`.

This commit rewrites it to use at most two matches.
- An explicit match on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- An explicit match on `Result<T, ErrorGuaranteed>`.

This is easier to read. It's also more efficient, though the code isn't hot enough for that to matter.

r? @Zalathar
rust-bors bot pushed a commit that referenced this pull request Mar 20, 2026
…uwer

Rollup of 12 pull requests

Successful merges:

 - #152909 (sess: `-Zbranch-protection` is a target modifier)
 - #153556 (`impl` restriction lowering)
 - #154048 (Don't emit rustdoc `missing_doc_code_examples` lint on impl items)
 - #153992 (bootstrap: Optionally print a backtrace if a command fails)
 - #154019 (two smaller feature cleanups)
 - #154059 (tests: Activate `must_not_suspend` test for `MutexGuard` dropped before `await`)
 - #154075 (Rewrite `query_ensure_result`.)
 - #154082 (Updates derive_where and removes workaround)
 - #154084 (Preserve braces around `self` in use tree pretty printing)
 - #154086 (Insert space after float literal ending with `.` in pretty printer)
 - #154087 (Fix whitespace after fragment specifiers in macro pretty printing)
 - #154109 (tests: Add regression test for async closures involving HRTBs)
rust-bors bot pushed a commit that referenced this pull request Mar 20, 2026
Rollup of 15 pull requests

Successful merges:

 - #152909 (sess: `-Zbranch-protection` is a target modifier)
 - #153556 (`impl` restriction lowering)
 - #154048 (Don't emit rustdoc `missing_doc_code_examples` lint on impl items)
 - #150935 (Introduce #[diagnostic::on_move(message)])
 - #152973 (remove -Csoft-float)
 - #153862 (Rename `cycle_check` to `find_cycle`)
 - #153992 (bootstrap: Optionally print a backtrace if a command fails)
 - #154019 (two smaller feature cleanups)
 - #154059 (tests: Activate `must_not_suspend` test for `MutexGuard` dropped before `await`)
 - #154075 (Rewrite `query_ensure_result`.)
 - #154082 (Updates derive_where and removes workaround)
 - #154084 (Preserve braces around `self` in use tree pretty printing)
 - #154086 (Insert space after float literal ending with `.` in pretty printer)
 - #154087 (Fix whitespace after fragment specifiers in macro pretty printing)
 - #154109 (tests: Add regression test for async closures involving HRTBs)
@rust-bors rust-bors bot merged commit 3cabafe into rust-lang:main Mar 20, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants