-
Notifications
You must be signed in to change notification settings - Fork 1.7k
internal: remove Default
from OpQueue
#18195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
internal: remove Default
from OpQueue
#18195
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this idea, I think explicitly having a 'OpQueue has never done anything' state is much clearer.
The test failures are legitimate, fwiw. |
29c9834
to
0c58839
Compare
If there are no objections, I'll merge this after Monday's release. This change feels like it has a large blast radius. |
0ff9e51
to
b66d3ff
Compare
b66d3ff
to
fc6eb66
Compare
@bors r+ |
☀️ Test successful - checks-actions |
…eykril internal: switch remaining OpQueues to use named structs Building atop of #18195, I switched `GlobalState::fetch_build_data_queue` to use a struct instead of a tuple. (I didn't switch `fetch_proc_macros_queue` to not return a bool, as the return value is only used in one spot.)
…avidbarsky internal: switch remaining OpQueues to use named structs Building atop of #18195, I switched `GlobalState::fetch_build_data_queue` to use a struct instead of a tuple. (I didn't switch `fetch_proc_macros_queue` to not return a bool, as the return value is only used in one spot.)
…rpoq, r=davidbarsky internal: switch remaining OpQueues to use named structs Building atop of rust-lang/rust-analyzer#18195, I switched `GlobalState::fetch_build_data_queue` to use a struct instead of a tuple. (I didn't switch `fetch_proc_macros_queue` to not return a bool, as the return value is only used in one spot.)
@Wilfred and I were talking about
OpQueue
and we identified one symptom of its (relative) weirdness is thatlast_op_result
returns aT::default()
; notOption<&T>
, which means it's not possible to distinguish between "theOpQueue
hasn't run yet" and "the OpQueue ran, but didn't produce a result". This branch fixes that.