From 8c789bf7d5573b2775522db51910b4d1663f17df Mon Sep 17 00:00:00 2001 From: Daumantas Kavolis Date: Mon, 17 Jul 2023 09:09:19 +0300 Subject: [PATCH] After review --- lib/binding_rust/bindings.rs | 5 +++-- lib/binding_rust/lib.rs | 14 ++++++++++++-- lib/include/tree_sitter/api.h | 18 +++++++++++++++++- lib/src/tree_cursor.c | 26 ++++++++++++++++---------- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/lib/binding_rust/bindings.rs b/lib/binding_rust/bindings.rs index 5ad38721a..d0d6c5e86 100644 --- a/lib/binding_rust/bindings.rs +++ b/lib/binding_rust/bindings.rs @@ -480,14 +480,15 @@ extern "C" { pub fn ts_tree_cursor_goto_next_sibling(arg1: *mut TSTreeCursor) -> bool; } extern "C" { - #[doc = " Move the cursor to the previous sibling of its current node.\n\n This returns `true` if the cursor successfully moved, and returns `false` if\n there was no previous sibling node."] + #[doc = " Move the cursor to the previous sibling of its current node.\n\n This returns `true` if the cursor successfully moved, and returns `false` if\n there was no previous sibling node.\n\n Note, that this function may be slower than\n `ts_tree_cursor_goto_next_sibling` due to how node positions are stored. In\n the worst case, this will need to iterate through all the children upto the\n previous sibling node to recalculate its position."] pub fn ts_tree_cursor_goto_previous_sibling(arg1: *mut TSTreeCursor) -> bool; } extern "C" { - #[doc = " Move the cursor to the first/last child of its current node.\n\n This returns `true` if the cursor successfully moved, and returns `false`\n if there were no children."] + #[doc = " Move the cursor to the first child of its current node.\n\n This returns `true` if the cursor successfully moved, and returns `false`\n if there were no children."] pub fn ts_tree_cursor_goto_first_child(arg1: *mut TSTreeCursor) -> bool; } extern "C" { + #[doc = " Move the cursor to the last child of its current node.\n\n This returns `true` if the cursor successfully moved, and returns `false` if\n there were no children.\n\n Note that this function may be slower than `ts_tree_cursor_goto_first_child`\n because it needs to iterate through all the children to compute the child's\n position."] pub fn ts_tree_cursor_goto_last_child(arg1: *mut TSTreeCursor) -> bool; } extern "C" { diff --git a/lib/binding_rust/lib.rs b/lib/binding_rust/lib.rs index c371b8a1f..6dd9daece 100644 --- a/lib/binding_rust/lib.rs +++ b/lib/binding_rust/lib.rs @@ -1380,8 +1380,12 @@ impl<'a> TreeCursor<'a> { /// Move this cursor to the last child of its current node. /// - /// This returns `true` if the cursor successfully moved, and returns `false` - /// if there were no children. + /// This returns `true` if the cursor successfully moved, and returns + /// `false` if there were no children. + /// + /// Note that this function may be slower than + /// [`goto_first_child`](TreeCursor::goto_first_child) because it needs to + /// iterate through all the children to compute the child's position. #[doc(alias = "ts_tree_cursor_goto_last_child")] pub fn goto_last_child(&mut self) -> bool { return unsafe { ffi::ts_tree_cursor_goto_last_child(&mut self.0) }; @@ -1419,6 +1423,12 @@ impl<'a> TreeCursor<'a> { /// /// This returns `true` if the cursor successfully moved, and returns /// `false` if there was no previous sibling node. + /// + /// Note, that this function may be slower than + /// [`goto_next_sibling`](TreeCursor::goto_next_sibling) due to how node + /// positions are stored. In the worst case, this will need to iterate + /// through all the children upto the previous sibling node to recalculate + /// its position. #[doc(alias = "ts_tree_cursor_goto_previous_sibling")] pub fn goto_previous_sibling(&mut self) -> bool { return unsafe { ffi::ts_tree_cursor_goto_previous_sibling(&mut self.0) }; diff --git a/lib/include/tree_sitter/api.h b/lib/include/tree_sitter/api.h index 0fc9faf50..5ccf4610f 100644 --- a/lib/include/tree_sitter/api.h +++ b/lib/include/tree_sitter/api.h @@ -720,16 +720,32 @@ bool ts_tree_cursor_goto_next_sibling(TSTreeCursor *); * * This returns `true` if the cursor successfully moved, and returns `false` if * there was no previous sibling node. + * + * Note, that this function may be slower than + * `ts_tree_cursor_goto_next_sibling` due to how node positions are stored. In + * the worst case, this will need to iterate through all the children upto the + * previous sibling node to recalculate its position. */ bool ts_tree_cursor_goto_previous_sibling(TSTreeCursor *); /** - * Move the cursor to the first/last child of its current node. + * Move the cursor to the first child of its current node. * * This returns `true` if the cursor successfully moved, and returns `false` * if there were no children. */ bool ts_tree_cursor_goto_first_child(TSTreeCursor *); + +/** + * Move the cursor to the last child of its current node. + * + * This returns `true` if the cursor successfully moved, and returns `false` if + * there were no children. + * + * Note that this function may be slower than `ts_tree_cursor_goto_first_child` + * because it needs to iterate through all the children to compute the child's + * position. + */ bool ts_tree_cursor_goto_last_child(TSTreeCursor *); /** diff --git a/lib/src/tree_cursor.c b/lib/src/tree_cursor.c index 7b8209830..25eca4826 100644 --- a/lib/src/tree_cursor.c +++ b/lib/src/tree_cursor.c @@ -97,13 +97,19 @@ static inline bool ts_tree_cursor_child_iterator_next( return true; } -static inline Length length_sub_zero(Length a, Length b) { - // length_sub doesn't account for 0 row subtraction, i.e. only columns - // should be subtracted, but changing point_sub breaks other tests - Length result = length_sub(a, b); - if (b.extent.row == 0 && a.extent.row != 0) { - result.extent.column -= b.extent.column; +// Return a position that, when `b` is added to it, yields `a`. This +// can only be computed if `b` has zero rows. Otherwise, this function +// returns `LENGTH_UNDEFINED`, and the caller needs to recompute +// the position some other way. +static inline Length length_backtrack(Length a, Length b) { + if (length_is_undefined(a) || b.extent.row != 0) { + return LENGTH_UNDEFINED; } + + Length result; + result.bytes = a.bytes - b.bytes; + result.extent.row = a.extent.row; + result.extent.column = a.extent.column - b.extent.column; return result; } @@ -129,14 +135,14 @@ static inline bool ts_tree_cursor_child_iterator_previous( self->structural_child_index--; } - self->position = length_sub_zero(self->position, ts_subtree_padding(*child)); + self->position = length_backtrack(self->position, ts_subtree_padding(*child)); self->child_index--; // unsigned can underflow so compare it to child_count if (self->child_index < self->parent.ptr->child_count) { Subtree previous_child = ts_subtree_children(self->parent)[self->child_index]; Length size = ts_subtree_size(previous_child); - self->position = length_sub_zero(self->position, size); + self->position = length_backtrack(self->position, size); } return true; @@ -362,8 +368,8 @@ TreeCursorStep ts_tree_cursor_goto_previous_sibling_internal(TSTreeCursor *_self if (step == TreeCursorStepNone) return step; - // if row has not changed, column is still valid - if (array_back(&self->stack)->position.extent.row == position.extent.row) + // if length is already valid, there's no need to recompute it + if (!length_is_undefined(array_back(&self->stack)->position)) return step; // restore position from the parent node