Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2025-01-25 19:25:06 +00:00
Code Issues Projects Releases Packages Wiki Activity

Clean up some logic around handling the parser blocks

Browse source 
Fix a todo. Enforce reverse iteration order.
This commit is contained in:
Peter Ammon 2024-12-27 16:42:38 -08:00
parent 64cb86ac26
commit b97598fa6c
No known key found for this signature in database
5 changed files with 46 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/builtins/return.rs
View file

@ -50,7 +50,7 @@ pub fn r#return(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) ->
Err(e) => return e,
};
let has_function_block = parser.blocks().iter().any(|b| b.is_function_call());
let has_function_block = parser.blocks_iter_rev().any(|b| b.is_function_call());
// *nix does not support negative return values, but our `return` builtin happily accepts being
// called with negative literals (e.g. `return -1`).

2
src/builtins/shared.rs
View file

@ -873,7 +873,7 @@ fn builtin_break_continue(
// Paranoia: ensure we have a real loop.
// This is checked in the AST but we may be invoked dynamically, e.g. just via "eval break".
let mut has_loop = false;
for b in parser.blocks().iter().rev() {
for b in parser.blocks_iter_rev() {
if [BlockType::while_block, BlockType::for_block].contains(&b.typ()) {
has_loop = true;
break;

2
src/event.rs
View file

@ -264,7 +264,7 @@ impl Event {
/// Test if specified event is blocked.
fn is_blocked(&self, parser: &Parser) -> bool {
for block in parser.blocks().iter().rev() {
for block in parser.blocks_iter_rev() {
if block.event_blocks {
return true;
}

5
src/parse_execution.rs
View file

@ -198,10 +198,7 @@ impl<'a> ExecutionContext {
// Check for stack overflow in case of function calls (regular stack overflow) or string
// substitution blocks, which can be recursively called with eval (issue #9302).
let block_type = {
let blocks = ctx.parser().blocks();
blocks.get(associated_block).unwrap().typ()
};
let block_type = ctx.parser().block_with_id(associated_block).typ();
if (block_type == BlockType::top && ctx.parser().function_stack_is_overflowing())
|| (block_type == BlockType::subst && ctx.parser().is_eval_depth_exceeded())
{

81
src/parser.rs
View file

@ -356,7 +356,10 @@ pub enum ParserStatusVar {
count_,
}
pub type BlockId = usize;
/// A newtype for the block index.
/// This is the naive position in the block list.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct BlockId(usize);
/// Controls the behavior when fish itself receives a signal and there are
/// no blocks on the stack.
@ -447,9 +450,7 @@ impl Parser {
/// Return whether we are currently evaluating a function.
pub fn is_function(&self) -> bool {
self.blocks()
.iter()
.rev()
self.blocks_iter_rev()
// If a function sources a file, don't descend further.
.take_while(|b| b.typ() != BlockType::source)
.any(|b| b.is_function_call())
@ -457,9 +458,7 @@ impl Parser {
/// Return whether we are currently evaluating a command substitution.
pub fn is_command_substitution(&self) -> bool {
self.blocks()
.iter()
.rev()
self.blocks_iter_rev()
// If a function sources a file, don't descend further.
.take_while(|b| b.typ() != BlockType::source)
.any(|b| b.typ() == BlockType::subst)
@ -737,7 +736,7 @@ impl Parser {
/// This supports 'status is-block'.
pub fn is_block(&self) -> bool {
// Note historically this has descended into 'source', unlike 'is_function'.
self.blocks().iter().rev().any(|b| {
self.blocks_iter_rev().any(|b| {
![
BlockType::top,
BlockType::subst,
@ -749,37 +748,51 @@ impl Parser {
/// Return whether we have a breakpoint block.
pub fn is_breakpoint(&self) -> bool {
self.blocks()
.iter()
.rev()
self.blocks_iter_rev()
.any(|b| b.typ() == BlockType::breakpoint)
}
/// Return the list of blocks. The first block is at the top.
/// todo!("this RAII object should only be used for iterating over it (in reverse). Maybe enforce this")
pub fn blocks(&self) -> Ref<'_, Vec<Block>> {
self.block_list.borrow()
// Return an iterator over the blocks, in reverse order.
// That is, the first block is the innermost block.
pub fn blocks_iter_rev<'a>(&'a self) -> impl Iterator<Item = Ref<'a, Block>> {
let blocks = self.block_list.borrow();
let mut indices = (0..blocks.len()).rev();
std::iter::from_fn(move || {
let last = indices.next()?;
// note this clone is cheap
Some(Ref::map(Ref::clone(&blocks), |bl| &bl[last]))
})
}
// Return the block at a given index, where 0 is the innermost block.
pub fn block_at_index(&self, index: usize) -> Option<Ref<'_, Block>> {
let block_list = self.blocks();
if index >= block_list.len() {
let block_list = self.block_list.borrow();
let block_count = block_list.len();
if index >= block_count {
None
} else {
Some(Ref::map(block_list, |bl| &bl[bl.len() - 1 - index]))
Some(Ref::map(block_list, |bl| &bl[block_count - 1 - index]))
}
}
// Return the block at a given index, where 0 is the innermost block.
pub fn block_at_index_mut(&self, index: usize) -> Option<RefMut<'_, Block>> {
let block_list = self.block_list.borrow_mut();
if index >= block_list.len() {
let block_count = block_list.len();
if index >= block_count {
None
} else {
Some(RefMut::map(block_list, |bl| {
let len = bl.len();
&mut bl[len - 1 - index]
&mut bl[block_count - 1 - index]
}))
}
}
// Return the block with the given id, asserting it exists. Note ids are recycled.
pub fn block_with_id(&self, id: BlockId) -> Ref<'_, Block> {
Ref::map(self.block_list.borrow(), |bl| &bl[id.0])
}
pub fn blocks_size(&self) -> usize {
self.block_list.borrow().len()
}
@ -871,14 +884,14 @@ impl Parser {
let mut block_list = self.block_list.borrow_mut();
block_list.push(block);
block_list.len() - 1
BlockId(block_list.len() - 1)
}
/// Remove the outermost block, asserting it's the given one.
pub fn pop_block(&self, expected: BlockId) {
let block = {
let mut block_list = self.block_list.borrow_mut();
assert!(expected == block_list.len() - 1);
assert!(expected.0 == block_list.len() - 1);
block_list.pop().unwrap()
};
if block.wants_pop_env() {
@ -893,9 +906,7 @@ impl Parser {
// isn't one return the function name for the current level.
// Walk until we find a breakpoint, then take the next function.
return self
.blocks()
.iter()
.rev()
.blocks_iter_rev()
.skip_while(|b| b.typ() != BlockType::breakpoint)
.find_map(|b| match b.data() {
Some(BlockData::Function { name, .. }) => Some(name.clone()),
@ -903,9 +914,7 @@ impl Parser {
});
}
self.blocks()
.iter()
.rev()
self.blocks_iter_rev()
// Historical: If we want the topmost function, but we are really in a file sourced by a
// function, don't consider ourselves to be in a function.
.take_while(|b| !(level == 1 && b.typ() == BlockType::source))
@ -1052,9 +1061,7 @@ impl Parser {
/// reader_current_filename, e.g. if we are evaluating a function defined in a different file
/// than the one currently read.
pub fn current_filename(&self) -> Option<FilenameRef> {
self.blocks()
.iter()
.rev()
self.blocks_iter_rev()
.find_map(|b| match b.data() {
Some(BlockData::Function { name, .. }) => {
function::get_props(name).and_then(|props| props.definition_file.clone())
@ -1073,16 +1080,14 @@ impl Parser {
/// Return a string representing the current stack trace.
pub fn stack_trace(&self) -> WString {
self.blocks()
.iter()
.rev()
self.blocks_iter_rev()
// Stop at event handler. No reason to believe that any other code is relevant.
// It might make sense in the future to continue printing the stack trace of the code
// that invoked the event, if this is a programmatic event, but we can't currently
// detect that.
.take_while(|b| b.typ() != BlockType::event)
.fold(WString::new(), |mut trace, b| {
append_block_description_to_stack_trace(self, b, &mut trace);
append_block_description_to_stack_trace(self, &b, &mut trace);
trace
})
}
@ -1098,9 +1103,7 @@ impl Parser {
}
// Count the functions.
let depth = self
.blocks()
.iter()
.rev()
.blocks_iter_rev()
.filter(|b| b.is_function_call())
.count();
depth > FISH_MAX_STACK_DEPTH