Skip to content

Commit 0239563

Browse files
committed
refactor: whitelist of shadowed variables
1 parent fe72bb3 commit 0239563

File tree

4 files changed

+17
-16
lines changed

4 files changed

+17
-16
lines changed

crates/nu-cli/src/repl.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ pub fn evaluate_repl(
206206
)
207207
}));
208208
match iteration_panic_state {
209-
Ok((continue_loop, es, s, le)) => {
209+
Ok((continue_loop, mut es, s, le)) => {
210210
// We apply the changes from the updated stack back onto our previous stack
211211
let mut merged_stack = Stack::with_changes_from_child(previous_stack_arc, s);
212212

crates/nu-protocol/src/engine/engine_state.rs

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ impl EngineState {
270270
existing_overlay.decls.insert(item.0, item.1);
271271
}
272272
for item in delta_overlay.vars.into_iter() {
273-
existing_overlay.vars.insert(item.0, item.1);
273+
existing_overlay.insert_variable(item.0, item.1);
274274
}
275275
for item in delta_overlay.modules.into_iter() {
276276
existing_overlay.modules.insert(item.0, item.1);
@@ -373,24 +373,19 @@ impl EngineState {
373373

374374
/// Clean up unused variables from a Stack to prevent memory leaks.
375375
/// This removes variables that are no longer referenced by any overlay.
376-
pub fn cleanup_stack_variables(&self, stack: &mut Stack) {
376+
pub fn cleanup_stack_variables(&mut self, stack: &mut Stack) {
377377
use std::collections::HashSet;
378378

379-
// Collect all VarIds that are still referenced by overlays
380-
let referenced_vars: HashSet<_> = self
381-
.scope
382-
.overlays
383-
.iter()
384-
.flat_map(|(_, overlay)| overlay.vars.values())
385-
.copied()
386-
// Always preserve critical system variables
387-
.chain([NU_VARIABLE_ID, IN_VARIABLE_ID, ENV_VARIABLE_ID])
388-
.collect();
379+
let mut shadowed_vars = HashSet::new();
380+
for (_, frame) in self.scope.overlays.iter_mut() {
381+
shadowed_vars.extend(frame.shadowed_vars.to_owned());
382+
frame.shadowed_vars.clear();
383+
}
389384

390385
// Remove variables from stack that are no longer referenced
391386
stack
392387
.vars
393-
.retain(|(var_id, _)| referenced_vars.contains(var_id));
388+
.retain(|(var_id, _)| !shadowed_vars.contains(var_id));
394389
}
395390

396391
pub fn active_overlay_ids<'a, 'b>(

crates/nu-protocol/src/engine/overlay.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ pub struct OverlayFrame {
182182
pub predecls: HashMap<Vec<u8>, DeclId>, // temporary storage for predeclarations
183183
pub decls: HashMap<Vec<u8>, DeclId>,
184184
pub modules: HashMap<Vec<u8>, ModuleId>,
185+
pub shadowed_vars: Vec<VarId>,
185186
pub visibility: Visibility,
186187
pub origin: ModuleId, // The original module the overlay was created from
187188
pub prefixed: bool, // Whether the overlay has definitions prefixed with its name
@@ -194,6 +195,7 @@ impl OverlayFrame {
194195
predecls: HashMap::new(),
195196
decls: HashMap::new(),
196197
modules: HashMap::new(),
198+
shadowed_vars: Vec::new(),
197199
visibility: Visibility::new(),
198200
origin,
199201
prefixed,
@@ -209,7 +211,11 @@ impl OverlayFrame {
209211
}
210212

211213
pub fn insert_variable(&mut self, name: Vec<u8>, variable_id: VarId) -> Option<VarId> {
212-
self.vars.insert(name, variable_id)
214+
let res = self.vars.insert(name, variable_id);
215+
if let Some(old_id) = res {
216+
self.shadowed_vars.push(old_id);
217+
}
218+
res
213219
}
214220

215221
pub fn get_decl(&self, name: &[u8]) -> Option<DeclId> {

crates/nu-protocol/src/engine/state_working_set.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ impl<'a> StateWorkingSet<'a> {
628628
name.insert(0, b'$');
629629
}
630630

631-
self.last_overlay_mut().vars.insert(name, next_id);
631+
self.last_overlay_mut().insert_variable(name, next_id);
632632

633633
self.delta.vars.push(Variable::new(span, ty, mutable));
634634

0 commit comments

Comments
 (0)