Skip to content

Commit 8621cbe

Browse files
committed
Auto merge of #18195 - davidbarsky:davidbarsky/push-xkqnsyksmzqv, r=Veykril
internal: remove `Default` from OpQueue `@Wilfred` and I were talking about `OpQueue` and we identified one symptom of its (relative) weirdness is that `last_op_result` returns a `T::default()`; not `Option<&T>`, which means it's not possible to distinguish between "the `OpQueue` hasn't run yet" and "the OpQueue ran, but didn't produce a result". This branch fixes that.
2 parents 52e8eaa + d7440d7 commit 8621cbe

4 files changed

Lines changed: 40 additions & 24 deletions

File tree

‎src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ pub(crate) struct FetchWorkspaceRequest {
4646
pub(crate)force_crate_graph_reload:bool,
4747
}
4848

49+
pub(crate)structFetchWorkspaceResponse{
50+
pub(crate)workspaces:Vec<anyhow::Result<ProjectWorkspace>>,
51+
pub(crate)force_crate_graph_reload:bool,
52+
}
53+
4954
// Enforces drop order
5055
pub(crate)structHandle<H,C>{
5156
pub(crate)handle:H,
@@ -146,8 +151,7 @@ pub(crate) struct GlobalState {
146151
pub(crate)detached_files:FxHashSet<ManifestPath>,
147152

148153
// op queues
149-
pub(crate)fetch_workspaces_queue:
150-
OpQueue<FetchWorkspaceRequest,Option<(Vec<anyhow::Result<ProjectWorkspace>>,bool)>>,
154+
pub(crate)fetch_workspaces_queue:OpQueue<FetchWorkspaceRequest,FetchWorkspaceResponse>,
151155
pub(crate)fetch_build_data_queue:
152156
OpQueue<(),(Arc<Vec<ProjectWorkspace>>,Vec<anyhow::Result<WorkspaceBuildScripts>>)>,
153157
pub(crate)fetch_proc_macros_queue:OpQueue<Vec<ProcMacroPaths>,bool>,
@@ -502,7 +506,7 @@ impl GlobalState {
502506
mem_docs:self.mem_docs.clone(),
503507
semantic_tokens_cache:Arc::clone(&self.semantic_tokens_cache),
504508
proc_macros_loaded: !self.config.expand_proc_macros()
505-
|| *self.fetch_proc_macros_queue.last_op_result(),
509+
|| self.fetch_proc_macros_queue.last_op_result().copied().unwrap_or(false),
506510
flycheck:self.flycheck.clone(),
507511
}
508512
}

‎src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ use crate::{
2222
diagnostics::{fetch_native_diagnostics,DiagnosticsGeneration,NativeDiagnosticsFetchKind},
2323
discover::{DiscoverArgument,DiscoverCommand,DiscoverProjectMessage},
2424
flycheck::{self,FlycheckMessage},
25-
global_state::{file_id_to_url, url_to_file_id,FetchWorkspaceRequest,GlobalState},
25+
global_state::{
26+
file_id_to_url, url_to_file_id,FetchWorkspaceRequest,FetchWorkspaceResponse,GlobalState,
27+
},
2628
hack_recover_crate_name,
2729
handlers::dispatch::{NotificationDispatcher,RequestDispatcher},
2830
lsp::{
@@ -695,9 +697,9 @@ impl GlobalState {
695697
let(state, msg) = match progress {
696698
ProjectWorkspaceProgress::Begin => (Progress::Begin,None),
697699
ProjectWorkspaceProgress::Report(msg) => (Progress::Report,Some(msg)),
698-
ProjectWorkspaceProgress::End(workspaces,force_reload_crate_graph) => {
699-
self.fetch_workspaces_queue
700-
.op_completed(Some((workspaces, force_reload_crate_graph)));
700+
ProjectWorkspaceProgress::End(workspaces,force_crate_graph_reload) => {
701+
let resp = FetchWorkspaceResponse{ workspaces, force_crate_graph_reload };
702+
self.fetch_workspaces_queue.op_completed(resp);
701703
ifletErr(e) = self.fetch_workspace_error(){
702704
error!("FetchWorkspaceError: {e}");
703705
}

‎src/tools/rust-analyzer/crates/rust-analyzer/src/op_queue.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ pub(crate) type Cause = String;
2727
pub(crate)structOpQueue<Args = (),Output = ()>{
2828
op_requested:Option<(Cause,Args)>,
2929
op_in_progress:bool,
30-
last_op_result:Output,
30+
last_op_result:Option<Output>,
3131
}
3232

33-
impl<Args,Output:Default>DefaultforOpQueue<Args,Output>{
33+
impl<Args,Output>DefaultforOpQueue<Args,Output>{
3434
fndefault() -> Self{
35-
Self{op_requested:None,op_in_progress:false,last_op_result:Default::default()}
35+
Self{op_requested:None,op_in_progress:false,last_op_result:None}
3636
}
3737
}
3838

@@ -56,12 +56,12 @@ impl<Args, Output> OpQueue<Args, Output> {
5656
pub(crate)fnop_completed(&mutself,result:Output){
5757
assert!(self.op_in_progress);
5858
self.op_in_progress = false;
59-
self.last_op_result = result;
59+
self.last_op_result = Some(result);
6060
}
6161

6262
/// Get the result of the last operation.
63-
pub(crate)fnlast_op_result(&self) -> &Output{
64-
&self.last_op_result
63+
pub(crate)fnlast_op_result(&self) -> Option<&Output>{
64+
self.last_op_result.as_ref()
6565
}
6666

6767
// Is there an operation that has started, but hasn't yet finished?

‎src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use vfs::{AbsPath, AbsPathBuf, ChangeKind};
3333
usecrate::{
3434
config::{Config,FilesWatcher,LinkedProject},
3535
flycheck::{FlycheckConfig,FlycheckHandle},
36-
global_state::{FetchWorkspaceRequest,GlobalState},
36+
global_state::{FetchWorkspaceRequest,FetchWorkspaceResponse,GlobalState},
3737
lsp_ext,
3838
main_loop::{DiscoverProjectParam,Task},
3939
op_queue::Cause,
@@ -448,15 +448,15 @@ impl GlobalState {
448448
let _p = tracing::info_span!("GlobalState::switch_workspaces").entered();
449449
tracing::info!(%cause,"will switch workspaces");
450450

451-
letSome((workspaces,force_reload_crate_graph)) =
451+
letSome(FetchWorkspaceResponse{workspaces,force_crate_graph_reload }) =
452452
self.fetch_workspaces_queue.last_op_result()
453453
else{
454454
return;
455455
};
456456

457-
info!(%cause, ?force_reload_crate_graph);
457+
info!(%cause, ?force_crate_graph_reload);
458458
ifself.fetch_workspace_error().is_err() && !self.workspaces.is_empty(){
459-
if*force_reload_crate_graph{
459+
if*force_crate_graph_reload{
460460
self.recreate_crate_graph(cause);
461461
}
462462
// It only makes sense to switch to a partially broken workspace
@@ -474,8 +474,12 @@ impl GlobalState {
474474
.all(|(l, r)| l.eq_ignore_build_data(r));
475475

476476
if same_workspaces {
477-
let(workspaces, build_scripts) = self.fetch_build_data_queue.last_op_result();
478-
ifArc::ptr_eq(workspaces,&self.workspaces){
477+
let(workspaces, build_scripts) = matchself.fetch_build_data_queue.last_op_result(){
478+
Some((workspaces, build_scripts)) => (workspaces.clone(), build_scripts.as_slice()),
479+
None => (Default::default(),Default::default()),
480+
};
481+
482+
ifArc::ptr_eq(&workspaces,&self.workspaces){
479483
info!("set build scripts to workspaces");
480484

481485
let workspaces = workspaces
@@ -492,7 +496,7 @@ impl GlobalState {
492496
self.workspaces = Arc::new(workspaces);
493497
}else{
494498
info!("build scripts do not match the version of the active workspace");
495-
if*force_reload_crate_graph{
499+
if*force_crate_graph_reload{
496500
self.recreate_crate_graph(cause);
497501
}
498502

@@ -739,14 +743,16 @@ impl GlobalState {
739743
pub(super)fnfetch_workspace_error(&self) -> Result<(),String>{
740744
letmut buf = String::new();
741745

742-
letSome((last_op_result, _)) = self.fetch_workspaces_queue.last_op_result()else{
746+
letSome(FetchWorkspaceResponse{ workspaces, .. }) =
747+
self.fetch_workspaces_queue.last_op_result()
748+
else{
743749
returnOk(());
744750
};
745751

746-
iflast_op_result.is_empty() && self.config.discover_workspace_config().is_none(){
752+
ifworkspaces.is_empty() && self.config.discover_workspace_config().is_none(){
747753
stdx::format_to!(buf,"rust-analyzer failed to fetch workspace");
748754
}else{
749-
for ws inlast_op_result{
755+
for ws inworkspaces{
750756
ifletErr(err) = ws {
751757
stdx::format_to!(buf,"rust-analyzer failed to load workspace: {:#}\n", err);
752758
}
@@ -763,7 +769,11 @@ impl GlobalState {
763769
pub(super)fnfetch_build_data_error(&self) -> Result<(),String>{
764770
letmut buf = String::new();
765771

766-
for ws in&self.fetch_build_data_queue.last_op_result().1{
772+
letSome((_, ws)) = &self.fetch_build_data_queue.last_op_result()else{
773+
returnOk(());
774+
};
775+
776+
for ws in ws {
767777
match ws {
768778
Ok(data) => {
769779
ifletSome(stderr) = data.error(){

0 commit comments

Comments
 (0)