From b020e7f97b43f05df46d6cc14f831c61c29506ca Mon Sep 17 00:00:00 2001 From: Dmitry Bushev Date: Mon, 17 Apr 2023 11:48:32 +0300 Subject: [PATCH] Fix stepping out of the function after renaming the project (#6285) close #6138 Rename method pointers in existing execution contexts after the project is renamed. --- app/gui/src/controller/graph/executed.rs | 16 -------------- app/gui/src/model/execution_context.rs | 3 +++ app/gui/src/model/execution_context/plain.rs | 21 ++++++++++++++++--- .../model/execution_context/synchronized.rs | 9 ++++++-- app/gui/src/model/project/synchronized.rs | 14 +++++++++++-- 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index 671f4ccc556..3635a812876 100644 --- a/app/gui/src/controller/graph/executed.rs +++ b/app/gui/src/controller/graph/executed.rs @@ -244,22 +244,6 @@ impl Handle { Ok(node_info) } - /// Enter node by given ID. - /// - /// This will cause pushing a new stack frame to the execution context and changing the graph - /// controller to point to a new definition. - /// - /// Fails if there's no information about target method pointer (e.g. because node value hasn't - /// been yet computed by the engine) or if method graph cannot be created (see - /// `graph_for_method` documentation). - pub async fn enter_node(&self, node: double_representation::node::Id) -> FallibleResult { - let computed_value = self.node_computed_value(node)?; - let method_pointer = computed_value.method_call.as_ref().ok_or(NoResolvedMethod(node))?; - let definition = method_pointer.clone(); - let local_call = LocalCall { call: node, definition }; - self.enter_method_pointer(&local_call).await - } - /// Leave the current node. Reverse of `enter_node`. /// /// Fails if this execution context is already at the stack's root or if the parent graph diff --git a/app/gui/src/model/execution_context.rs b/app/gui/src/model/execution_context.rs index 9d635678d90..7a7682c538c 100644 --- a/app/gui/src/model/execution_context.rs +++ b/app/gui/src/model/execution_context.rs @@ -500,6 +500,9 @@ pub trait API: Debug { /// Restart the program execution. #[allow(clippy::needless_lifetimes)] // Note: Needless lifetimes fn restart<'a>(&'a self) -> BoxFuture<'a, FallibleResult>; + + /// Adjust method pointers after the project rename action. + fn rename_method_pointers(&self, old_project_name: String, new_project_name: String); } // Note: Needless lifetimes diff --git a/app/gui/src/model/execution_context/plain.rs b/app/gui/src/model/execution_context/plain.rs index 8d888d77544..cf5123f2a3b 100644 --- a/app/gui/src/model/execution_context/plain.rs +++ b/app/gui/src/model/execution_context/plain.rs @@ -50,7 +50,7 @@ pub struct InvalidVisualizationId(VisualizationId); #[derive(Debug)] pub struct ExecutionContext { /// A name of definition which is a root call of this context. - pub entry_point: MethodPointer, + pub entry_point: RefCell, /// Local call stack. stack: RefCell>, /// Set of active visualizations. @@ -65,7 +65,8 @@ pub struct ExecutionContext { impl ExecutionContext { /// Create new execution context - pub fn new(entry_point: MethodPointer) -> Self { + pub fn new(method_pointer: MethodPointer) -> Self { + let entry_point = RefCell::new(method_pointer); let stack = default(); let visualizations = default(); let computed_value_info_registry = default(); @@ -166,7 +167,7 @@ impl model::execution_context::API for ExecutionContext { if let Some(top_frame) = self.stack.borrow().last() { top_frame.definition.clone() } else { - self.entry_point.clone() + self.entry_point.borrow().clone() } } @@ -258,6 +259,20 @@ impl model::execution_context::API for ExecutionContext { fn restart(&self) -> BoxFuture { futures::future::ready(Ok(())).boxed_local() } + + fn rename_method_pointers(&self, old_project_name: String, new_project_name: String) { + let update_method_pointer = |method_pointer: &mut MethodPointer| { + let module = method_pointer.module.replacen(&old_project_name, &new_project_name, 1); + let defined_on_type = + method_pointer.defined_on_type.replacen(&old_project_name, &new_project_name, 1); + let name = method_pointer.name.clone(); + MethodPointer { module, defined_on_type, name } + }; + self.entry_point.replace_with(update_method_pointer); + self.stack.borrow_mut().iter_mut().for_each(|local_call| { + local_call.definition = update_method_pointer(&mut local_call.definition) + }); + } } diff --git a/app/gui/src/model/execution_context/synchronized.rs b/app/gui/src/model/execution_context/synchronized.rs index 6912d0d7c6f..d20e79634a7 100644 --- a/app/gui/src/model/execution_context/synchronized.rs +++ b/app/gui/src/model/execution_context/synchronized.rs @@ -79,7 +79,7 @@ impl ExecutionContext { } fn push_root_frame(&self) -> impl Future { - let method_pointer = self.model.entry_point.clone(); + let method_pointer = self.model.entry_point.borrow().clone(); let this_argument_expression = default(); let positional_arguments_expressions = default(); @@ -304,6 +304,10 @@ impl model::execution_context::API for ExecutionContext { } .boxed_local() } + + fn rename_method_pointers(&self, old_project_name: String, new_project_name: String) { + self.model.rename_method_pointers(old_project_name, new_project_name); + } } impl Drop for ExecutionContext { @@ -433,7 +437,8 @@ pub mod test { let f = Fixture::new(); assert_eq!(f.data.context_id, f.context.id); let name_in_data = f.data.module_qualified_name(); - let name_in_ctx_model = QualifiedName::try_from(&f.context.model.entry_point.module); + let name_in_ctx_model = + QualifiedName::try_from(&f.context.model.entry_point.borrow().module); assert_eq!(name_in_data, name_in_ctx_model.unwrap()); assert_eq!(Vec::::new(), f.context.model.stack_items().collect_vec()); } diff --git a/app/gui/src/model/project/synchronized.rs b/app/gui/src/model/project/synchronized.rs index 74408a2d198..714df7e69aa 100644 --- a/app/gui/src/model/project/synchronized.rs +++ b/app/gui/src/model/project/synchronized.rs @@ -102,6 +102,14 @@ impl ExecutionContextsRegistry { pub fn insert(&self, context: Rc) { self.0.borrow_mut().insert(context.id(), context); } + + /// Adjust execution contexts after renaming the project. + pub fn rename_project(&self, old_name: impl Str, new_name: impl Str) { + self.0.borrow().iter().for_each(|(_, execution_context)| { + execution_context + .rename_method_pointers(old_name.as_ref().to_owned(), new_name.as_ref().to_owned()); + }); + } } @@ -695,12 +703,14 @@ impl model::project::API for Project { fn rename_project(&self, name: String) -> BoxFuture { async move { - let referent_name = name.as_str().try_into()?; + let old_name = self.properties.borrow_mut().name.project.clone_ref(); + let referent_name = name.to_im_string(); let project_manager = self.project_manager.as_ref().ok_or(ProjectManagerUnavailable)?; let project_id = self.properties.borrow().id; let project_name = ProjectName::new_unchecked(name); project_manager.rename_project(&project_id, &project_name).await?; - self.properties.borrow_mut().name.project = referent_name; + self.properties.borrow_mut().name.project = referent_name.clone_ref(); + self.execution_contexts.rename_project(old_name, referent_name); Ok(()) } .boxed_local()