Skip to content

Refactor ActiveJobGuard#153471

Open
nnethercote wants to merge 2 commits intorust-lang:mainfrom
nnethercote:complete_inner
Open

Refactor ActiveJobGuard#153471
nnethercote wants to merge 2 commits intorust-lang:mainfrom
nnethercote:complete_inner

Conversation

@nnethercote
Copy link
Contributor

A few small improvements.

r? @petrochenkov

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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 6, 2026
@nnethercote
Copy link
Contributor Author

cc @Zalathar @Zoxc @zetanumbers

@nnethercote nnethercote force-pushed the complete_inner branch 2 times, most recently from 61284b1 to 232409f Compare March 6, 2026 07:25
@zetanumbers
Copy link
Contributor

zetanumbers commented Mar 6, 2026

What is ActiveJobGuard? I've never seen this struct. Was it renamed? How was it previously named?

EDIT: Ahh, so it's the JobOwner (#152377)

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2026
@zetanumbers
Copy link
Contributor

Outside of those suggestions this code looks pretty clean.

@nnethercote
Copy link
Contributor Author

lol, I don't think I've ever had comments from this many people for a small refactoring PR :) Anyway, I think all comments have now been addressed.

@Zalathar
Copy link
Member

Zalathar commented Mar 7, 2026

I had intended to move the struct definition when I did the rename, but it got lost during a non-trivial rebase, so I’m happy to see it moved in this PR.

@rust-bors

This comment has been minimized.

`ActiveJobGuard::complete` and `ActiveJobGuard::drop` have very similar
code, though non-essential structural differences obscure this a little.

This commit factors out the common code into a new method
`ActiveJobGuard::complete_inner`. It also inlines and remove
`expect_job`, which ends up having a single call site.
@rustbot
Copy link
Collaborator

rustbot commented Mar 8, 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

I rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

6 participants