From bfd10e4c2b4d506512bf401817aaaeaf4534f090 Mon Sep 17 00:00:00 2001 From: Matt Fellenz Date: Sat, 20 Jan 2024 22:25:13 -0800 Subject: [PATCH 01/33] Use niching for Checkpoint --- src/green/builder.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/green/builder.rs b/src/green/builder.rs index 1dbc8bb0..03d66df5 100644 --- a/src/green/builder.rs +++ b/src/green/builder.rs @@ -1,3 +1,5 @@ +use std::num::NonZeroUsize; + use crate::{ cow_mut::CowMut, green::{node_cache::NodeCache, GreenElement, GreenNode, SyntaxKind}, @@ -6,7 +8,17 @@ use crate::{ /// A checkpoint for maybe wrapping a node. See `GreenNodeBuilder::checkpoint` for details. #[derive(Clone, Copy, Debug)] -pub struct Checkpoint(usize); +pub struct Checkpoint(NonZeroUsize); + +impl Checkpoint { + fn new(inner: usize) -> Self { + Self(NonZeroUsize::new(inner + 1).unwrap()) + } + + fn into_inner(self) -> usize { + self.0.get() - 1 + } +} /// A builder for a green tree. #[derive(Default, Debug)] @@ -82,14 +94,14 @@ impl GreenNodeBuilder<'_> { /// ``` #[inline] pub fn checkpoint(&self) -> Checkpoint { - Checkpoint(self.children.len()) + Checkpoint::new(self.children.len()) } /// Wrap the previous branch marked by `checkpoint` in a new branch and /// make it current. #[inline] pub fn start_node_at(&mut self, checkpoint: Checkpoint, kind: SyntaxKind) { - let Checkpoint(checkpoint) = checkpoint; + let checkpoint = checkpoint.into_inner(); assert!( checkpoint <= self.children.len(), "checkpoint no longer valid, was finish_node called early?" From 62bd09ba8e42fb6b386657678a4b2fdd86b4aab4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Wed, 4 Oct 2023 11:06:32 +0100 Subject: [PATCH 02/33] Add rowan::api::SyntaxNode::new_root_mut --- src/api.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/api.rs b/src/api.rs index 605fcc50..d6411d30 100644 --- a/src/api.rs +++ b/src/api.rs @@ -98,6 +98,9 @@ impl SyntaxNode { pub fn new_root(green: GreenNode) -> SyntaxNode { SyntaxNode::from(cursor::SyntaxNode::new_root(green)) } + pub fn new_root_mut(green: GreenNode) -> SyntaxNode { + SyntaxNode::from(cursor::SyntaxNode::new_root_mut(green)) + } /// Returns a green tree, equal to the green tree this node /// belongs two, except with this node substitute. The complexity /// of operation is proportional to the depth of the tree From 20f30010f5f5e58e0626b3e922c0083161c43435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 13 May 2024 09:58:15 +0100 Subject: [PATCH 03/33] Fix pointer comparison warning on newer rustc --- src/arc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arc.rs b/src/arc.rs index 34883129..36b7f24a 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -83,7 +83,7 @@ impl Arc { /// allocation #[inline] pub(crate) fn ptr_eq(this: &Self, other: &Self) -> bool { - this.ptr() == other.ptr() + std::ptr::addr_eq(this.ptr(), other.ptr()) } pub(crate) fn ptr(&self) -> *mut ArcInner { From 9f64e506a3f2c2751b82fd5ad8868e39d6bea6df Mon Sep 17 00:00:00 2001 From: Kevin Kuriakose Date: Sat, 18 Feb 2023 18:11:43 +0530 Subject: [PATCH 04/33] Prevent use of SyntaxNodePtr and AstPtr on mutable trees Both of these use source code locations to identify the node, which can get invalidated by syntax tree mutations. This commit adds assertions to prevent their use, and adds documentation to inform users of the issue Fixes #150 --- src/api.rs | 4 +++ src/ast.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++- src/cursor.rs | 4 +++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/api.rs b/src/api.rs index d6411d30..bf85fa0f 100644 --- a/src/api.rs +++ b/src/api.rs @@ -247,6 +247,10 @@ impl SyntaxNode { SyntaxNode::from(self.raw.clone_for_update()) } + pub fn is_mutable(&self) -> bool { + self.raw.is_mutable() + } + pub fn detach(&self) { self.raw.detach() } diff --git a/src/ast.rs b/src/ast.rs index 856a9f63..e142395b 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -55,6 +55,11 @@ pub trait AstNode { } /// A "pointer" to a [`SyntaxNode`], via location in the source code. +/// +/// ## Note +/// Since the location is source code dependent, this must not be used +/// with mutable syntax trees. Any changes made in such trees causes +/// the pointed node's source location to change, invalidating the pointer. #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct SyntaxNodePtr { kind: L::Kind, @@ -63,7 +68,10 @@ pub struct SyntaxNodePtr { impl SyntaxNodePtr { /// Returns a [`SyntaxNodePtr`] for the node. + /// + /// Panics if the provided node is mutable pub fn new(node: &SyntaxNode) -> Self { + assert!(!node.is_mutable(), "tree is mutable"); Self { kind: node.kind(), range: node.text_range() } } @@ -82,10 +90,13 @@ impl SyntaxNodePtr { /// Also returns `None` if `root` is not actually a root (i.e. it has a /// parent). /// + /// NOTE: If this function is called on a mutable tree, it will panic + /// /// The complexity is linear in the depth of the tree and logarithmic in /// tree width. As most trees are shallow, thinking about this as /// `O(log(N))` in the size of the tree is not too wrong! pub fn try_to_node(&self, root: &SyntaxNode) -> Option> { + assert!(!root.is_mutable(), "tree is mutable"); if root.parent().is_some() { return None; } @@ -113,13 +124,21 @@ impl SyntaxNodePtr { } /// Like [`SyntaxNodePtr`], but remembers the type of node. +/// +/// ## Note +/// As with [`SyntaxNodePtr`], this must not be used on mutable +/// syntax trees, since any mutation can cause the pointed node's +/// source location to change, invalidating the pointer pub struct AstPtr { raw: SyntaxNodePtr, } impl AstPtr { /// Returns an [`AstPtr`] for the node. + /// + /// Panics if the provided node is mutable pub fn new(node: &N) -> Self { + // The above mentioned panic is handled by SyntaxNodePtr Self { raw: SyntaxNodePtr::new(node.syntax()) } } @@ -129,8 +148,9 @@ impl AstPtr { } /// Given the root node containing the node `n` that `self` is a pointer to, - /// returns `n` if possible. See [`SyntaxNodePtr::try_to_node`]. + /// returns `n` if possible. Panics if `root` is mutable. See [`SyntaxNodePtr::try_to_node`]. pub fn try_to_node(&self, root: &SyntaxNode) -> Option { + // The above mentioned panic is handled by SyntaxNodePtr N::cast(self.raw.try_to_node(root)?) } @@ -215,3 +235,53 @@ pub mod support { parent.children_with_tokens().filter_map(|it| it.into_token()).find(|it| it.kind() == kind) } } + +#[cfg(test)] +mod tests { + use crate::{GreenNodeBuilder, Language, SyntaxKind, SyntaxNode}; + + use super::SyntaxNodePtr; + + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + struct TestLanguage; + impl Language for TestLanguage { + type Kind = SyntaxKind; + + fn kind_from_raw(raw: SyntaxKind) -> Self::Kind { + raw + } + + fn kind_to_raw(kind: Self::Kind) -> SyntaxKind { + kind + } + } + + fn build_immut_tree() -> SyntaxNode { + // Creates a single-node tree + let mut builder = GreenNodeBuilder::new(); + builder.start_node(SyntaxKind(0)); + builder.finish_node(); + + SyntaxNode::::new_root(builder.finish()) + } + + #[test] + #[should_panic = "tree is mutable"] + fn ensure_mut_panic_on_create() { + // Make a mutable version + let tree = build_immut_tree().clone_for_update(); + + SyntaxNodePtr::new(&tree); + } + + #[test] + #[should_panic = "tree is mutable"] + fn ensure_mut_panic_on_deref() { + let tree = build_immut_tree(); + let tree_mut = tree.clone_for_update(); + + // Create on immutable, convert on mutable + let syn_ptr = SyntaxNodePtr::new(&tree); + syn_ptr.to_node(&tree_mut); + } +} diff --git a/src/cursor.rs b/src/cursor.rs index 671be234..9060507e 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -544,6 +544,10 @@ impl SyntaxNode { SyntaxNode { ptr: NodeData::new(Some(parent), index, offset, green, mutable) } } + pub fn is_mutable(&self) -> bool { + self.data().mutable + } + pub fn clone_for_update(&self) -> SyntaxNode { assert!(!self.data().mutable); match self.parent() { From 54c9c96d472c46e92c474c0cb6b8988f638cc2b3 Mon Sep 17 00:00:00 2001 From: peamaeq Date: Wed, 4 May 2022 11:30:05 +0800 Subject: [PATCH 05/33] 0503 --- src/arc.rs | 4 +--- src/cursor.rs | 18 +++++++----------- src/green/node.rs | 6 ++---- src/green/token.rs | 4 +--- src/sll.rs | 10 +++++----- 5 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/arc.rs b/src/arc.rs index 36b7f24a..76420b85 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -257,12 +257,10 @@ impl Deref for HeaderSlice { type Target = HeaderSlice; fn deref(&self) -> &Self::Target { - unsafe { let len = self.length; let fake_slice: *const [T] = ptr::slice_from_raw_parts(self as *const _ as *const T, len); - &*(fake_slice as *const HeaderSlice) - } + unsafe { &*(fake_slice as *const HeaderSlice) } } } diff --git a/src/cursor.rs b/src/cursor.rs index 9060507e..b9e55371 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -431,9 +431,8 @@ impl NodeData { None => return, }; - unsafe { sll::adjust(self, self.index() + 1, Delta::Sub(1)); - let parent = parent_ptr.as_ref(); + let parent = unsafe { parent_ptr.as_ref() }; sll::unlink(&parent.first, self); // Add strong ref to green @@ -449,27 +448,25 @@ impl NodeData { match parent.green() { NodeOrToken::Node(green) => { let green = green.remove_child(self.index() as usize); - parent.respine(green) + unsafe { parent.respine(green) } } NodeOrToken::Token(_) => unreachable!(), } if parent.dec_rc() { - free(parent_ptr) + unsafe { free(parent_ptr) } } - } } fn attach_child(&self, index: usize, child: &NodeData) { assert!(self.mutable && child.mutable && child.parent().is_none()); assert!(self.rc.get() > 0 && child.rc.get() > 0); - unsafe { child.index.set(index as u32); child.parent.set(Some(self.into())); self.inc_rc(); if !self.first.get().is_null() { - sll::adjust(&*self.first.get(), index as u32, Delta::Add(1)); + sll::adjust(unsafe { &*self.first.get() }, index as u32, Delta::Add(1)); } match sll::link(&self.first, child) { @@ -483,16 +480,15 @@ impl NodeData { NodeOrToken::Node(green) => { // Child is root, so it ownes the green node. Steal it! let child_green = match &child.green { - Green::Node { ptr } => GreenNode::from_raw(ptr.get()).into(), - Green::Token { ptr } => GreenToken::from_raw(*ptr).into(), + Green::Node { ptr } => unsafe { GreenNode::from_raw(ptr.get()).into() }, + Green::Token { ptr } => unsafe { GreenToken::from_raw(*ptr).into() }, }; let green = green.insert_child(index, child_green); - self.respine(green); + unsafe { self.respine(green) }; } NodeOrToken::Token(_) => unreachable!(), } - } } unsafe fn respine(&self, mut new_green: GreenNode) { let mut node = self; diff --git a/src/green/node.rs b/src/green/node.rs index e94dc01c..7753bb92 100644 --- a/src/green/node.rs +++ b/src/green/node.rs @@ -56,11 +56,9 @@ impl ToOwned for GreenNodeData { #[inline] fn to_owned(&self) -> GreenNode { - unsafe { - let green = GreenNode::from_raw(ptr::NonNull::from(self)); + let green = unsafe { GreenNode::from_raw(ptr::NonNull::from(self)) }; let green = ManuallyDrop::new(green); GreenNode::clone(&green) - } } } @@ -194,8 +192,8 @@ impl ops::Deref for GreenNode { #[inline] fn deref(&self) -> &GreenNodeData { + let repr: &Repr = &self.ptr; unsafe { - let repr: &Repr = &self.ptr; let repr: &ReprThin = &*(repr as *const Repr as *const ReprThin); mem::transmute::<&ReprThin, &GreenNodeData>(repr) } diff --git a/src/green/token.rs b/src/green/token.rs index 1a4548a4..e5ed8925 100644 --- a/src/green/token.rs +++ b/src/green/token.rs @@ -44,11 +44,9 @@ impl ToOwned for GreenTokenData { #[inline] fn to_owned(&self) -> GreenToken { - unsafe { - let green = GreenToken::from_raw(ptr::NonNull::from(self)); + let green = unsafe { GreenToken::from_raw(ptr::NonNull::from(self)) }; let green = ManuallyDrop::new(green); GreenToken::clone(&green) - } } } diff --git a/src/sll.rs b/src/sll.rs index 87d2f1f3..0dd17b87 100644 --- a/src/sll.rs +++ b/src/sll.rs @@ -84,12 +84,12 @@ pub(crate) fn unlink(head: &Cell<*const E>, elem: &E) { #[cold] pub(crate) fn link<'a, E: Elem>(head: &'a Cell<*const E>, elem: &E) -> AddToSllResult<'a, E> { + let old_head = head.get(); + // Case 1: empty head, replace it. + if old_head.is_null() { + return AddToSllResult::EmptyHead(head); + } unsafe { - let old_head = head.get(); - // Case 1: empty head, replace it. - if old_head.is_null() { - return AddToSllResult::EmptyHead(head); - } // Case 2: we are smaller than the head, replace it. if elem.key() < (*old_head).key() { From c082936255a0735ad751803ed5b7a0e8c3e5961e Mon Sep 17 00:00:00 2001 From: peamaeq Date: Thu, 16 Jun 2022 20:31:02 +0800 Subject: [PATCH 06/33] '0503' --- src/arc.rs | 7 ++-- src/cursor.rs | 86 +++++++++++++++++++++++----------------------- src/green/node.rs | 6 ++-- src/green/token.rs | 6 ++-- src/sll.rs | 1 - 5 files changed, 52 insertions(+), 54 deletions(-) diff --git a/src/arc.rs b/src/arc.rs index 76420b85..7adf45e7 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -257,10 +257,9 @@ impl Deref for HeaderSlice { type Target = HeaderSlice; fn deref(&self) -> &Self::Target { - let len = self.length; - let fake_slice: *const [T] = - ptr::slice_from_raw_parts(self as *const _ as *const T, len); - unsafe { &*(fake_slice as *const HeaderSlice) } + let len = self.length; + let fake_slice: *const [T] = ptr::slice_from_raw_parts(self as *const _ as *const T, len); + unsafe { &*(fake_slice as *const HeaderSlice) } } } diff --git a/src/cursor.rs b/src/cursor.rs index b9e55371..3b849582 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -431,64 +431,64 @@ impl NodeData { None => return, }; - sll::adjust(self, self.index() + 1, Delta::Sub(1)); - let parent = unsafe { parent_ptr.as_ref() }; - sll::unlink(&parent.first, self); - - // Add strong ref to green - match self.green().to_owned() { - NodeOrToken::Node(it) => { - GreenNode::into_raw(it); - } - NodeOrToken::Token(it) => { - GreenToken::into_raw(it); - } + sll::adjust(self, self.index() + 1, Delta::Sub(1)); + let parent = unsafe { parent_ptr.as_ref() }; + sll::unlink(&parent.first, self); + + // Add strong ref to green + match self.green().to_owned() { + NodeOrToken::Node(it) => { + GreenNode::into_raw(it); } - - match parent.green() { - NodeOrToken::Node(green) => { - let green = green.remove_child(self.index() as usize); - unsafe { parent.respine(green) } - } - NodeOrToken::Token(_) => unreachable!(), + NodeOrToken::Token(it) => { + GreenToken::into_raw(it); } + } - if parent.dec_rc() { - unsafe { free(parent_ptr) } + match parent.green() { + NodeOrToken::Node(green) => { + let green = green.remove_child(self.index() as usize); + unsafe { parent.respine(green) } } + NodeOrToken::Token(_) => unreachable!(), + } + + if parent.dec_rc() { + unsafe { free(parent_ptr) } + } } fn attach_child(&self, index: usize, child: &NodeData) { assert!(self.mutable && child.mutable && child.parent().is_none()); assert!(self.rc.get() > 0 && child.rc.get() > 0); - child.index.set(index as u32); - child.parent.set(Some(self.into())); - self.inc_rc(); + child.index.set(index as u32); + child.parent.set(Some(self.into())); + self.inc_rc(); - if !self.first.get().is_null() { - sll::adjust(unsafe { &*self.first.get() }, index as u32, Delta::Add(1)); - } + if !self.first.get().is_null() { + sll::adjust(unsafe { &*self.first.get() }, index as u32, Delta::Add(1)); + } - match sll::link(&self.first, child) { - sll::AddToSllResult::AlreadyInSll(_) => { - panic!("Child already in sorted linked list") - } - it => it.add_to_sll(child), + match sll::link(&self.first, child) { + sll::AddToSllResult::AlreadyInSll(_) => { + panic!("Child already in sorted linked list") } + it => it.add_to_sll(child), + } - match self.green() { - NodeOrToken::Node(green) => { - // Child is root, so it ownes the green node. Steal it! - let child_green = match &child.green { - Green::Node { ptr } => unsafe { GreenNode::from_raw(ptr.get()).into() }, - Green::Token { ptr } => unsafe { GreenToken::from_raw(*ptr).into() }, - }; + match self.green() { + NodeOrToken::Node(green) => { + // Child is root, so it ownes the green node. Steal it! + let child_green = match &child.green { + Green::Node { ptr } => unsafe { GreenNode::from_raw(ptr.get()).into() }, + Green::Token { ptr } => unsafe { GreenToken::from_raw(*ptr).into() }, + }; - let green = green.insert_child(index, child_green); - unsafe { self.respine(green) }; - } - NodeOrToken::Token(_) => unreachable!(), + let green = green.insert_child(index, child_green); + unsafe { self.respine(green) }; } + NodeOrToken::Token(_) => unreachable!(), + } } unsafe fn respine(&self, mut new_green: GreenNode) { let mut node = self; diff --git a/src/green/node.rs b/src/green/node.rs index 7753bb92..69b1cf76 100644 --- a/src/green/node.rs +++ b/src/green/node.rs @@ -56,9 +56,9 @@ impl ToOwned for GreenNodeData { #[inline] fn to_owned(&self) -> GreenNode { - let green = unsafe { GreenNode::from_raw(ptr::NonNull::from(self)) }; - let green = ManuallyDrop::new(green); - GreenNode::clone(&green) + let green = unsafe { GreenNode::from_raw(ptr::NonNull::from(self)) }; + let green = ManuallyDrop::new(green); + GreenNode::clone(&green) } } diff --git a/src/green/token.rs b/src/green/token.rs index e5ed8925..81e298c7 100644 --- a/src/green/token.rs +++ b/src/green/token.rs @@ -44,9 +44,9 @@ impl ToOwned for GreenTokenData { #[inline] fn to_owned(&self) -> GreenToken { - let green = unsafe { GreenToken::from_raw(ptr::NonNull::from(self)) }; - let green = ManuallyDrop::new(green); - GreenToken::clone(&green) + let green = unsafe { GreenToken::from_raw(ptr::NonNull::from(self)) }; + let green = ManuallyDrop::new(green); + GreenToken::clone(&green) } } diff --git a/src/sll.rs b/src/sll.rs index 0dd17b87..4a298390 100644 --- a/src/sll.rs +++ b/src/sll.rs @@ -90,7 +90,6 @@ pub(crate) fn link<'a, E: Elem>(head: &'a Cell<*const E>, elem: &E) -> AddToSllR return AddToSllResult::EmptyHead(head); } unsafe { - // Case 2: we are smaller than the head, replace it. if elem.key() < (*old_head).key() { return AddToSllResult::SmallerThanHead(head); From ba6b8e4cb42fcb7bc3fd24a9f5a6299620525c6e Mon Sep 17 00:00:00 2001 From: Alona Enraght-Moony Date: Fri, 31 May 2024 16:19:09 +0100 Subject: [PATCH 07/33] Remove `memoffset` dependency. `std::mem::offset` has been availible on stable since 1.77.0. `memoffset` uses this when compiled against a recent enough rustc. Moving to the implementation in `std` lets us drop both `memoffset` and `autocfg` from our dependencies. --- Cargo.toml | 3 +-- src/arc.rs | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5c5bba7f..879c77d6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ repository = "https://github.com/rust-analyzer/rowan" license = "MIT OR Apache-2.0" description = "Library for generic lossless syntax trees" edition = "2021" - +rust-version = "1.77.0" exclude = [".github/", "bors.toml", "rustfmt.toml"] [workspace] @@ -18,7 +18,6 @@ hashbrown = { version = "0.14.3", features = [ "inline-more", ], default-features = false } text-size = "1.1.0" -memoffset = "0.9" countme = "3.0.0" serde = { version = "1.0.89", optional = true, default-features = false } diff --git a/src/arc.rs b/src/arc.rs index 7adf45e7..cb6758e6 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -4,7 +4,7 @@ use std::{ cmp::Ordering, hash::{Hash, Hasher}, marker::PhantomData, - mem::{self, ManuallyDrop}, + mem::{self, offset_of, ManuallyDrop}, ops::Deref, ptr, sync::atomic::{ @@ -13,8 +13,6 @@ use std::{ }, }; -use memoffset::offset_of; - /// A soft limit on the amount of references that may be made to an `Arc`. /// /// Going above this limit will abort your program (although not From e2d2e93e16c5104b136d0bc738a0d48346922200 Mon Sep 17 00:00:00 2001 From: Alona Enraght-Moony Date: Mon, 3 Jun 2024 15:38:43 +0100 Subject: [PATCH 08/33] v0.15.16 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 879c77d6..fcee9656 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rowan" -version = "0.15.15" +version = "0.15.16" authors = ["Aleksey Kladov "] repository = "https://github.com/rust-analyzer/rowan" license = "MIT OR Apache-2.0" From a87bcd2494d62c110ba9bbe4752948a4c9d2e0a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Fri, 16 Aug 2024 16:16:20 +0100 Subject: [PATCH 09/33] Fix typo in docstring: two => to Also, rephrase slightly. --- src/api.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/api.rs b/src/api.rs index bf85fa0f..03b2cc23 100644 --- a/src/api.rs +++ b/src/api.rs @@ -101,9 +101,10 @@ impl SyntaxNode { pub fn new_root_mut(green: GreenNode) -> SyntaxNode { SyntaxNode::from(cursor::SyntaxNode::new_root_mut(green)) } + /// Returns a green tree, equal to the green tree this node - /// belongs two, except with this node substitute. The complexity - /// of operation is proportional to the depth of the tree + /// belongs to, except with this node substituted. The complexity + /// of the operation is proportional to the depth of the tree. pub fn replace_with(&self, replacement: GreenNode) -> GreenNode { self.raw.replace_with(replacement) } @@ -263,8 +264,8 @@ impl SyntaxNode { impl SyntaxToken { /// Returns a green tree, equal to the green tree this token - /// belongs two, except with this token substitute. The complexity - /// of operation is proportional to the depth of the tree + /// belongs to, except with this token substituted. The complexity + /// of the operation is proportional to the depth of the tree. pub fn replace_with(&self, new_token: GreenToken) -> GreenNode { self.raw.replace_with(new_token) } From b0a922bc99c673174f3f8f155e561c7e65f8e114 Mon Sep 17 00:00:00 2001 From: Michael van Straten Date: Fri, 21 Jun 2024 16:33:31 +0200 Subject: [PATCH 10/33] Allow passing an iterable to `splice_children` --- src/api.rs | 8 ++++++-- src/cursor.rs | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/api.rs b/src/api.rs index 03b2cc23..c1d66983 100644 --- a/src/api.rs +++ b/src/api.rs @@ -256,8 +256,12 @@ impl SyntaxNode { self.raw.detach() } - pub fn splice_children(&self, to_delete: Range, to_insert: Vec>) { - let to_insert = to_insert.into_iter().map(cursor::SyntaxElement::from).collect::>(); + pub fn splice_children>>( + &self, + to_delete: Range, + to_insert: I, + ) { + let to_insert = to_insert.into_iter().map(cursor::SyntaxElement::from); self.raw.splice_children(to_delete, to_insert) } } diff --git a/src/cursor.rs b/src/cursor.rs index 3b849582..0fa2eb5b 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -806,7 +806,11 @@ impl SyntaxNode { }) } - pub fn splice_children(&self, to_delete: Range, to_insert: Vec) { + pub fn splice_children>( + &self, + to_delete: Range, + to_insert: I, + ) { assert!(self.data().mutable, "immutable tree: {}", self); for (i, child) in self.children_with_tokens().enumerate() { if to_delete.contains(&i) { From f45924cdca24bac7c617d1ac3981f457795c5889 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Sat, 28 Sep 2024 07:11:54 +0800 Subject: [PATCH 11/33] Fix typos --- src/api.rs | 4 ++-- src/cursor.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/api.rs b/src/api.rs index 03b2cc23..ccfb0a58 100644 --- a/src/api.rs +++ b/src/api.rs @@ -214,7 +214,7 @@ impl SyntaxNode { } /// Find a token in the subtree corresponding to this node, which covers the offset. - /// Precondition: offset must be withing node's range. + /// Precondition: offset must be within node's range. pub fn token_at_offset(&self, offset: TextSize) -> TokenAtOffset> { self.raw.token_at_offset(offset).map(SyntaxToken::from) } @@ -222,7 +222,7 @@ impl SyntaxNode { /// Return the deepest node or token in the current subtree that fully /// contains the range. If the range is empty and is contained in two leaf /// nodes, either one can be returned. Precondition: range must be contained - /// withing the current node + /// within the current node pub fn covering_element(&self, range: TextRange) -> SyntaxElement { NodeOrToken::from(self.raw.covering_element(range)) } diff --git a/src/cursor.rs b/src/cursor.rs index 3b849582..f7faf27a 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -478,7 +478,7 @@ impl NodeData { match self.green() { NodeOrToken::Node(green) => { - // Child is root, so it ownes the green node. Steal it! + // Child is root, so it owns the green node. Steal it! let child_green = match &child.green { Green::Node { ptr } => unsafe { GreenNode::from_raw(ptr.get()).into() }, Green::Token { ptr } => unsafe { GreenToken::from_raw(*ptr).into() }, From 2f4371a21123dd62dcfb9ec2ee127612747a807c Mon Sep 17 00:00:00 2001 From: Martin Huschenbett Date: Mon, 7 Oct 2024 07:55:49 +0200 Subject: [PATCH 12/33] Remove needless .into() --- examples/math.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/math.rs b/examples/math.rs index 930c591b..e3c35056 100644 --- a/examples/math.rs +++ b/examples/math.rs @@ -111,7 +111,7 @@ impl> Parser { } fn print(indent: usize, element: SyntaxElement) { - let kind: SyntaxKind = element.kind().into(); + let kind: SyntaxKind = element.kind(); print!("{:indent$}", "", indent = indent); match element { NodeOrToken::Node(node) => { From f06a2c903caa270e977a73ef5b9091f99e8fbe6c Mon Sep 17 00:00:00 2001 From: Theodore Luo Wang Date: Sun, 3 Oct 2021 15:03:23 -0400 Subject: [PATCH 13/33] Reduce allocations when iterating over Syntax{Node,Element} children This allows users of the API to apply a filter on the SyntaxKind before materializing concrete SyntaxNode/Token objects, which require a memory allocation for the NodeData. For slint, this removes ~400k allocations when parsing a largish project, about 50% of all rowan allocations (850k down to 450k). --- src/api.rs | 93 +++++++++++++++++++++++ src/cursor.rs | 202 +++++++++++++++++++++++++++++++++++++++++++++++--- src/lib.rs | 3 +- 3 files changed, 288 insertions(+), 10 deletions(-) diff --git a/src/api.rs b/src/api.rs index ccfb0a58..72ef0a96 100644 --- a/src/api.rs +++ b/src/api.rs @@ -148,6 +148,13 @@ impl SyntaxNode { pub fn first_child(&self) -> Option> { self.raw.first_child().map(Self::from) } + + pub fn first_child_by_kind(&self, matcher: &impl Fn(L::Kind) -> bool) -> Option> { + self.raw + .first_child_by_kind(&|raw_kind| matcher(L::kind_from_raw(raw_kind))) + .map(Self::from) + } + pub fn last_child(&self) -> Option> { self.raw.last_child().map(Self::from) } @@ -155,6 +162,16 @@ impl SyntaxNode { pub fn first_child_or_token(&self) -> Option> { self.raw.first_child_or_token().map(NodeOrToken::from) } + + pub fn first_child_or_token_by_kind( + &self, + matcher: &impl Fn(L::Kind) -> bool, + ) -> Option> { + self.raw + .first_child_or_token_by_kind(&|raw_kind| matcher(L::kind_from_raw(raw_kind))) + .map(NodeOrToken::from) + } + pub fn last_child_or_token(&self) -> Option> { self.raw.last_child_or_token().map(NodeOrToken::from) } @@ -162,6 +179,16 @@ impl SyntaxNode { pub fn next_sibling(&self) -> Option> { self.raw.next_sibling().map(Self::from) } + + pub fn next_sibling_by_kind( + &self, + matcher: &impl Fn(L::Kind) -> bool, + ) -> Option> { + self.raw + .next_sibling_by_kind(&|raw_kind| matcher(L::kind_from_raw(raw_kind))) + .map(Self::from) + } + pub fn prev_sibling(&self) -> Option> { self.raw.prev_sibling().map(Self::from) } @@ -169,6 +196,16 @@ impl SyntaxNode { pub fn next_sibling_or_token(&self) -> Option> { self.raw.next_sibling_or_token().map(NodeOrToken::from) } + + pub fn next_sibling_or_token_by_kind( + &self, + matcher: &impl Fn(L::Kind) -> bool, + ) -> Option> { + self.raw + .next_sibling_or_token_by_kind(&|raw_kind| matcher(L::kind_from_raw(raw_kind))) + .map(NodeOrToken::from) + } + pub fn prev_sibling_or_token(&self) -> Option> { self.raw.prev_sibling_or_token().map(NodeOrToken::from) } @@ -403,6 +440,34 @@ impl Iterator for SyntaxNodeChildren { } } +impl SyntaxNodeChildren { + pub fn by_kind<'a>( + self, + matcher: &'a dyn Fn(SyntaxKind) -> bool, + ) -> SyntaxNodeChildrenByKind<'a, L> { + SyntaxNodeChildrenByKind { raw: self.raw.by_kind(matcher), _p: PhantomData } + } +} + +#[derive(Clone)] +pub struct SyntaxNodeChildrenByKind<'a, L: Language> { + raw: cursor::SyntaxNodeChildrenByKind<&'a dyn Fn(SyntaxKind) -> bool>, + _p: PhantomData, +} + +impl<'a, L: Language> Iterator for SyntaxNodeChildrenByKind<'a, L> { + type Item = SyntaxNode; + fn next(&mut self) -> Option { + self.raw.next().map(SyntaxNode::from) + } +} + +impl<'a, L: Language> fmt::Debug for SyntaxNodeChildrenByKind<'a, L> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SyntaxNodeChildrenByKind").finish() + } +} + #[derive(Debug, Clone)] pub struct SyntaxElementChildren { raw: cursor::SyntaxElementChildren, @@ -416,6 +481,34 @@ impl Iterator for SyntaxElementChildren { } } +impl SyntaxElementChildren { + pub fn by_kind<'a>( + self, + matcher: &'a dyn Fn(SyntaxKind) -> bool, + ) -> SyntaxElementChildrenByKind<'a, L> { + SyntaxElementChildrenByKind { raw: self.raw.by_kind(matcher), _p: PhantomData } + } +} + +#[derive(Clone)] +pub struct SyntaxElementChildrenByKind<'a, L: Language> { + raw: cursor::SyntaxElementChildrenByKind<&'a dyn Fn(SyntaxKind) -> bool>, + _p: PhantomData, +} + +impl<'a, L: Language> Iterator for SyntaxElementChildrenByKind<'a, L> { + type Item = SyntaxElement; + fn next(&mut self) -> Option { + self.raw.next().map(NodeOrToken::from) + } +} + +impl<'a, L: Language> fmt::Debug for SyntaxElementChildrenByKind<'a, L> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SyntaxElementChildrenByKind").finish() + } +} + pub struct Preorder { raw: cursor::Preorder, _p: PhantomData, diff --git a/src/cursor.rs b/src/cursor.rs index f7faf27a..4681c214 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -376,11 +376,26 @@ impl NodeData { } fn next_sibling(&self) -> Option { - let mut siblings = self.green_siblings().enumerate(); + let siblings = self.green_siblings().enumerate(); + let index = self.index() as usize; + + siblings.skip(index + 1).find_map(|(index, child)| { + child.as_ref().into_node().and_then(|green| { + let parent = self.parent_node()?; + let offset = parent.offset() + child.rel_offset(); + Some(SyntaxNode::new_child(green, parent, index as u32, offset)) + }) + }) + } + + fn next_sibling_by_kind(&self, matcher: &impl Fn(SyntaxKind) -> bool) -> Option { + let siblings = self.green_siblings().enumerate(); let index = self.index() as usize; - siblings.nth(index); - siblings.find_map(|(index, child)| { + siblings.skip(index + 1).find_map(|(index, child)| { + if !matcher(child.as_ref().kind()) { + return None; + } child.as_ref().into_node().and_then(|green| { let parent = self.parent_node()?; let offset = parent.offset() + child.rel_offset(); @@ -388,12 +403,12 @@ impl NodeData { }) }) } + fn prev_sibling(&self) -> Option { - let mut rev_siblings = self.green_siblings().enumerate().rev(); - let index = rev_siblings.len().checked_sub(self.index() as usize + 1)?; + let rev_siblings = self.green_siblings().enumerate().rev(); + let index = rev_siblings.len().checked_sub(self.index() as usize)?; - rev_siblings.nth(index); - rev_siblings.find_map(|(index, child)| { + rev_siblings.skip(index + 1).find_map(|(index, child)| { child.as_ref().into_node().and_then(|green| { let parent = self.parent_node()?; let offset = parent.offset() + child.rel_offset(); @@ -412,6 +427,24 @@ impl NodeData { Some(SyntaxElement::new(child.as_ref(), parent, index as u32, offset)) }) } + + fn next_sibling_or_token_by_kind( + &self, + matcher: &impl Fn(SyntaxKind) -> bool, + ) -> Option { + let siblings = self.green_siblings().enumerate(); + let index = self.index() as usize; + + siblings.skip(index + 1).find_map(|(index, child)| { + if !matcher(child.as_ref().kind()) { + return None; + } + let parent = self.parent_node()?; + let offset = parent.offset() + child.rel_offset(); + Some(SyntaxElement::new(child.as_ref(), parent, index as u32, offset)) + }) + } + fn prev_sibling_or_token(&self) -> Option { let mut siblings = self.green_siblings().enumerate(); let index = self.index().checked_sub(1)? as usize; @@ -647,6 +680,23 @@ impl SyntaxNode { }) }) } + + pub fn first_child_by_kind(&self, matcher: &impl Fn(SyntaxKind) -> bool) -> Option { + self.green_ref().children().raw.enumerate().find_map(|(index, child)| { + if !matcher(child.as_ref().kind()) { + return None; + } + child.as_ref().into_node().map(|green| { + SyntaxNode::new_child( + green, + self.clone(), + index as u32, + self.offset() + child.rel_offset(), + ) + }) + }) + } + pub fn last_child(&self) -> Option { self.green_ref().children().raw.enumerate().rev().find_map(|(index, child)| { child.as_ref().into_node().map(|green| { @@ -665,6 +715,24 @@ impl SyntaxNode { SyntaxElement::new(child.as_ref(), self.clone(), 0, self.offset() + child.rel_offset()) }) } + + pub fn first_child_or_token_by_kind( + &self, + matcher: &impl Fn(SyntaxKind) -> bool, + ) -> Option { + self.green_ref().children().raw.enumerate().find_map(|(index, child)| { + if !matcher(child.as_ref().kind()) { + return None; + } + Some(SyntaxElement::new( + child.as_ref(), + self.clone(), + index as u32, + self.offset() + child.rel_offset(), + )) + }) + } + pub fn last_child_or_token(&self) -> Option { self.green_ref().children().raw.enumerate().next_back().map(|(index, child)| { SyntaxElement::new( @@ -679,6 +747,14 @@ impl SyntaxNode { pub fn next_sibling(&self) -> Option { self.data().next_sibling() } + + pub fn next_sibling_by_kind( + &self, + matcher: &impl Fn(SyntaxKind) -> bool, + ) -> Option { + self.data().next_sibling_by_kind(matcher) + } + pub fn prev_sibling(&self) -> Option { self.data().prev_sibling() } @@ -686,6 +762,14 @@ impl SyntaxNode { pub fn next_sibling_or_token(&self) -> Option { self.data().next_sibling_or_token() } + + pub fn next_sibling_or_token_by_kind( + &self, + matcher: &impl Fn(SyntaxKind) -> bool, + ) -> Option { + self.data().next_sibling_or_token_by_kind(matcher) + } + pub fn prev_sibling_or_token(&self) -> Option { self.data().prev_sibling_or_token() } @@ -910,6 +994,14 @@ impl SyntaxToken { pub fn next_sibling_or_token(&self) -> Option { self.data().next_sibling_or_token() } + + pub fn next_sibling_or_token_by_kind( + &self, + matcher: &impl Fn(SyntaxKind) -> bool, + ) -> Option { + self.data().next_sibling_or_token_by_kind(matcher) + } + pub fn prev_sibling_or_token(&self) -> Option { self.data().prev_sibling_or_token() } @@ -1028,6 +1120,17 @@ impl SyntaxElement { NodeOrToken::Token(it) => it.next_sibling_or_token(), } } + + pub fn next_sibling_or_token_by_kind( + &self, + matcher: &impl Fn(SyntaxKind) -> bool, + ) -> Option { + match self { + NodeOrToken::Node(it) => it.next_sibling_or_token_by_kind(matcher), + NodeOrToken::Token(it) => it.next_sibling_or_token_by_kind(matcher), + } + } + pub fn prev_sibling_or_token(&self) -> Option { match self { NodeOrToken::Node(it) => it.prev_sibling_or_token(), @@ -1133,18 +1236,41 @@ impl From for SyntaxElement { #[derive(Clone, Debug)] pub struct SyntaxNodeChildren { + parent: SyntaxNode, next: Option, + next_initialized: bool, } impl SyntaxNodeChildren { fn new(parent: SyntaxNode) -> SyntaxNodeChildren { - SyntaxNodeChildren { next: parent.first_child() } + SyntaxNodeChildren { parent, next: None, next_initialized: false } + } + + pub fn by_kind bool>(self, matcher: F) -> SyntaxNodeChildrenByKind { + if !self.next_initialized { + SyntaxNodeChildrenByKind { next: self.parent.first_child_by_kind(&matcher), matcher } + } else { + SyntaxNodeChildrenByKind { + next: self.next.and_then(|node| { + if matcher(node.kind()) { + Some(node) + } else { + node.next_sibling_by_kind(&matcher) + } + }), + matcher, + } + } } } impl Iterator for SyntaxNodeChildren { type Item = SyntaxNode; fn next(&mut self) -> Option { + if !self.next_initialized { + self.next = self.parent.first_child(); + self.next_initialized = true; + } self.next.take().map(|next| { self.next = next.next_sibling(); next @@ -1152,20 +1278,62 @@ impl Iterator for SyntaxNodeChildren { } } +#[derive(Clone, Debug)] +pub struct SyntaxNodeChildrenByKind bool> { + next: Option, + matcher: F, +} + +impl bool> Iterator for SyntaxNodeChildrenByKind { + type Item = SyntaxNode; + fn next(&mut self) -> Option { + self.next.take().map(|next| { + self.next = next.next_sibling_by_kind(&self.matcher); + next + }) + } +} + #[derive(Clone, Debug)] pub struct SyntaxElementChildren { + parent: SyntaxNode, next: Option, + next_initialized: bool, } impl SyntaxElementChildren { fn new(parent: SyntaxNode) -> SyntaxElementChildren { - SyntaxElementChildren { next: parent.first_child_or_token() } + SyntaxElementChildren { parent, next: None, next_initialized: false } + } + + pub fn by_kind bool>(self, matcher: F) -> SyntaxElementChildrenByKind { + if !self.next_initialized { + SyntaxElementChildrenByKind { + next: self.parent.first_child_or_token_by_kind(&matcher), + matcher, + } + } else { + SyntaxElementChildrenByKind { + next: self.next.and_then(|node| { + if matcher(node.kind()) { + Some(node) + } else { + node.next_sibling_or_token_by_kind(&matcher) + } + }), + matcher, + } + } } } impl Iterator for SyntaxElementChildren { type Item = SyntaxElement; fn next(&mut self) -> Option { + if !self.next_initialized { + self.next = self.parent.first_child_or_token(); + self.next_initialized = true; + } self.next.take().map(|next| { self.next = next.next_sibling_or_token(); next @@ -1173,6 +1341,22 @@ impl Iterator for SyntaxElementChildren { } } +#[derive(Clone, Debug)] +pub struct SyntaxElementChildrenByKind bool> { + next: Option, + matcher: F, +} + +impl bool> Iterator for SyntaxElementChildrenByKind { + type Item = SyntaxElement; + fn next(&mut self) -> Option { + self.next.take().map(|next| { + self.next = next.next_sibling_or_token_by_kind(&self.matcher); + next + }) + } +} + pub struct Preorder { start: SyntaxNode, next: Option>, diff --git a/src/lib.rs b/src/lib.rs index bb8f30d0..066d8c52 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,7 +30,8 @@ pub use text_size::{TextLen, TextRange, TextSize}; pub use crate::{ api::{ - Language, SyntaxElement, SyntaxElementChildren, SyntaxNode, SyntaxNodeChildren, SyntaxToken, + Language, SyntaxElement, SyntaxElementChildren, SyntaxElementChildrenByKind, SyntaxNode, + SyntaxNodeChildren, SyntaxNodeChildrenByKind, SyntaxToken, }, green::{ Checkpoint, Children, GreenNode, GreenNodeBuilder, GreenNodeData, GreenToken, From 0b1a6ec04ae9aa60413c28ddee0c344415129260 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Sun, 27 Oct 2024 19:39:28 +0100 Subject: [PATCH 14/33] Optimize iteration over sibling nodes by reusing NodeData if possible When possible, reuse the allocated NodeData instead of allocating a new one for each iteration. This can be done as long as the refcount is 1 - we can then just rewire the values in NodeData to point to the new item. This removes ~220k allocations when compiling a largish slint file, about half of all rowan allocations that happen during iteration, i.e. we go from 450k down to 230k. --- src/cursor.rs | 134 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 126 insertions(+), 8 deletions(-) diff --git a/src/cursor.rs b/src/cursor.rs index 4681c214..aa66be05 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -597,6 +597,20 @@ impl SyntaxNode { unsafe { self.ptr.as_ref() } } + #[inline] + fn can_take_ptr(&self) -> bool { + self.data().rc.get() == 1 && !self.data().mutable + } + + #[inline] + fn take_ptr(self) -> ptr::NonNull { + assert!(self.can_take_ptr()); + let ret = self.ptr; + // don't change the refcount when self gets dropped + std::mem::forget(self); + ret + } + pub fn replace_with(&self, replacement: GreenNode) -> GreenNode { assert_eq!(self.kind(), replacement.kind()); match &self.parent() { @@ -744,6 +758,43 @@ impl SyntaxNode { }) } + // if possible (i.e. unshared), consume self and advance it to point to the next sibling + // this way, we can reuse the previously allocated buffer + pub fn to_next_sibling(self) -> Option { + if !self.can_take_ptr() { + // cannot mutate in-place + return self.next_sibling(); + } + + let mut ptr = self.take_ptr(); + let data = unsafe { ptr.as_mut() }; + assert!(data.rc.get() == 1); + + let parent = data.parent_node()?; + let parent_offset = parent.offset(); + let siblings = parent.green_ref().children().raw.enumerate(); + let index = data.index() as usize; + + siblings + .skip(index + 1) + .find_map(|(index, child)| { + child + .as_ref() + .into_node() + .and_then(|green| Some((green, index as u32, child.rel_offset()))) + }) + .and_then(|(green, index, rel_offset)| { + data.index.set(index); + data.offset = parent_offset + rel_offset; + data.green = Green::Node { ptr: Cell::new(green.into()) }; + Some(SyntaxNode { ptr }) + }) + .or_else(|| { + unsafe { free(ptr) }; + None + }) + } + pub fn next_sibling(&self) -> Option { self.data().next_sibling() } @@ -937,6 +988,20 @@ impl SyntaxToken { unsafe { self.ptr.as_ref() } } + #[inline] + fn can_take_ptr(&self) -> bool { + self.data().rc.get() == 1 && !self.data().mutable + } + + #[inline] + fn take_ptr(self) -> ptr::NonNull { + assert!(self.can_take_ptr()); + let ret = self.ptr; + // don't change the refcount when self gets dropped + std::mem::forget(self); + ret + } + pub fn replace_with(&self, replacement: GreenToken) -> GreenNode { assert_eq!(self.kind(), replacement.kind()); let parent = self.parent().unwrap(); @@ -1121,6 +1186,59 @@ impl SyntaxElement { } } + fn can_take_ptr(&self) -> bool { + match self { + NodeOrToken::Node(it) => it.can_take_ptr(), + NodeOrToken::Token(it) => it.can_take_ptr(), + } + } + + fn take_ptr(self) -> ptr::NonNull { + match self { + NodeOrToken::Node(it) => it.take_ptr(), + NodeOrToken::Token(it) => it.take_ptr(), + } + } + + // if possible (i.e. unshared), consume self and advance it to point to the next sibling + // this way, we can reuse the previously allocated buffer + pub fn to_next_sibling_or_token(self) -> Option { + if !self.can_take_ptr() { + // cannot mutate in-place + return self.next_sibling_or_token(); + } + + let mut ptr = self.take_ptr(); + let data = unsafe { ptr.as_mut() }; + + let parent = data.parent_node()?; + let parent_offset = parent.offset(); + let siblings = parent.green_ref().children().raw.enumerate(); + let index = data.index() as usize; + + siblings + .skip(index + 1) + .find_map(|(index, green)| { + data.index.set(index as u32); + data.offset = parent_offset + green.rel_offset(); + + match green.as_ref() { + NodeOrToken::Node(node) => { + data.green = Green::Node { ptr: Cell::new(node.into()) }; + Some(SyntaxElement::Node(SyntaxNode { ptr })) + } + NodeOrToken::Token(token) => { + data.green = Green::Token { ptr: token.into() }; + Some(SyntaxElement::Token(SyntaxToken { ptr })) + } + } + }) + .or_else(|| { + unsafe { free(ptr) }; + None + }) + } + pub fn next_sibling_or_token_by_kind( &self, matcher: &impl Fn(SyntaxKind) -> bool, @@ -1270,11 +1388,11 @@ impl Iterator for SyntaxNodeChildren { if !self.next_initialized { self.next = self.parent.first_child(); self.next_initialized = true; + } else { + self.next = self.next.take().and_then(|next| next.to_next_sibling()); } - self.next.take().map(|next| { - self.next = next.next_sibling(); - next - }) + + self.next.clone() } } @@ -1333,11 +1451,11 @@ impl Iterator for SyntaxElementChildren { if !self.next_initialized { self.next = self.parent.first_child_or_token(); self.next_initialized = true; + } else { + self.next = self.next.take().and_then(|next| next.to_next_sibling_or_token()); } - self.next.take().map(|next| { - self.next = next.next_sibling_or_token(); - next - }) + + self.next.clone() } } From ab5463e2749330be6846886c21e98c83caca8598 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Sun, 27 Oct 2024 22:06:08 +0100 Subject: [PATCH 15/33] Use iterators for by_kind instead of introducing separate structs This makes this API actually useful from the outside - there is no lifetime problem with the matcher callback, and we can remap the raw Kind to the Language::Kind on the fly. --- src/api.rs | 56 +++++++++--------------------------------------------- src/lib.rs | 3 +-- 2 files changed, 10 insertions(+), 49 deletions(-) diff --git a/src/api.rs b/src/api.rs index 72ef0a96..cd86d9d8 100644 --- a/src/api.rs +++ b/src/api.rs @@ -441,30 +441,11 @@ impl Iterator for SyntaxNodeChildren { } impl SyntaxNodeChildren { - pub fn by_kind<'a>( - self, - matcher: &'a dyn Fn(SyntaxKind) -> bool, - ) -> SyntaxNodeChildrenByKind<'a, L> { - SyntaxNodeChildrenByKind { raw: self.raw.by_kind(matcher), _p: PhantomData } - } -} - -#[derive(Clone)] -pub struct SyntaxNodeChildrenByKind<'a, L: Language> { - raw: cursor::SyntaxNodeChildrenByKind<&'a dyn Fn(SyntaxKind) -> bool>, - _p: PhantomData, -} - -impl<'a, L: Language> Iterator for SyntaxNodeChildrenByKind<'a, L> { - type Item = SyntaxNode; - fn next(&mut self) -> Option { - self.raw.next().map(SyntaxNode::from) - } -} - -impl<'a, L: Language> fmt::Debug for SyntaxNodeChildrenByKind<'a, L> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("SyntaxNodeChildrenByKind").finish() + pub fn by_kind(self, matcher: impl Fn(L::Kind) -> bool) -> impl Iterator> { + self.raw + .by_kind(move |raw_kind| matcher(L::kind_from_raw(raw_kind))) + .into_iter() + .map(SyntaxNode::from) } } @@ -482,30 +463,11 @@ impl Iterator for SyntaxElementChildren { } impl SyntaxElementChildren { - pub fn by_kind<'a>( + pub fn by_kind( self, - matcher: &'a dyn Fn(SyntaxKind) -> bool, - ) -> SyntaxElementChildrenByKind<'a, L> { - SyntaxElementChildrenByKind { raw: self.raw.by_kind(matcher), _p: PhantomData } - } -} - -#[derive(Clone)] -pub struct SyntaxElementChildrenByKind<'a, L: Language> { - raw: cursor::SyntaxElementChildrenByKind<&'a dyn Fn(SyntaxKind) -> bool>, - _p: PhantomData, -} - -impl<'a, L: Language> Iterator for SyntaxElementChildrenByKind<'a, L> { - type Item = SyntaxElement; - fn next(&mut self) -> Option { - self.raw.next().map(NodeOrToken::from) - } -} - -impl<'a, L: Language> fmt::Debug for SyntaxElementChildrenByKind<'a, L> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("SyntaxElementChildrenByKind").finish() + matcher: impl Fn(L::Kind) -> bool, + ) -> impl Iterator> { + self.raw.by_kind(move |raw_kind| matcher(L::kind_from_raw(raw_kind))).map(NodeOrToken::from) } } diff --git a/src/lib.rs b/src/lib.rs index 066d8c52..bb8f30d0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,8 +30,7 @@ pub use text_size::{TextLen, TextRange, TextSize}; pub use crate::{ api::{ - Language, SyntaxElement, SyntaxElementChildren, SyntaxElementChildrenByKind, SyntaxNode, - SyntaxNodeChildren, SyntaxNodeChildrenByKind, SyntaxToken, + Language, SyntaxElement, SyntaxElementChildren, SyntaxNode, SyntaxNodeChildren, SyntaxToken, }, green::{ Checkpoint, Children, GreenNode, GreenNodeBuilder, GreenNodeData, GreenToken, From 32949733eeaa62ff91b08dfbe51e55fa0c737834 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Thu, 31 Oct 2024 11:01:59 +0100 Subject: [PATCH 16/33] Decrement refcount before calling free in to_next_sibling Fixes an assertion in debug builds which I accidentally introduced when attending the review comments for [1] in [2] - instead of only removing the increment, I also removed the decrement which was wrong - `std::mem::forget` only allows us to remove the increment, but the decrement is still needed before free since we are in a place of code that is by definition only run when the rc value is set to 1. See also `can_take_ptr`. I did not spot this earlier since I ran the integration test on a release build, where the assertion was disabled. It's sad that the rowan repo itself doesn't have any big test coverage in this repo itself, but rather relies on external repos for testing purposes... [1]: https://github.com/rust-analyzer/rowan/pull/171#discussion_r1818509336 [2]: https://github.com/rust-analyzer/rowan/compare/60a632ad984ab451e32058169193511154c675a9..ab5463e2749330be6846886c21e98c83caca8598 Fixes: https://github.com/rust-analyzer/rowan/issues/172 --- src/cursor.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cursor.rs b/src/cursor.rs index aa66be05..a6d84c68 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -790,6 +790,7 @@ impl SyntaxNode { Some(SyntaxNode { ptr }) }) .or_else(|| { + data.dec_rc(); unsafe { free(ptr) }; None }) @@ -1234,6 +1235,7 @@ impl SyntaxElement { } }) .or_else(|| { + data.dec_rc(); unsafe { free(ptr) }; None }) From dc1eb13afcad213632fe5f43dc636bc83a34f43a Mon Sep 17 00:00:00 2001 From: Michael van Straten Date: Sun, 18 Aug 2024 19:30:58 +0200 Subject: [PATCH 17/33] Bump crate version number --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index fcee9656..34c84bbc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rowan" -version = "0.15.16" +version = "0.16.0" authors = ["Aleksey Kladov "] repository = "https://github.com/rust-analyzer/rowan" license = "MIT OR Apache-2.0" From 84b1c8fb43c83b51a80217423b1e32c0730b348b Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Fri, 22 Nov 2024 15:54:48 +0100 Subject: [PATCH 18/33] Fix prev_sibling indexing off-by-one The patch f06a2c903caa270e977a73ef5b9091f99e8fbe6c changed the code to use skip instead of nth, which lead to an off-by-one bug that was uncovered by unit tests in ludtwig, see [1]. [1]: https://github.com/MalteJanz/ludtwig/pull/122 Fixes: https://github.com/rust-analyzer/rowan/issues/175 --- Cargo.toml | 2 +- src/cursor.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 34c84bbc..e2bc8c41 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rowan" -version = "0.16.0" +version = "0.16.1" authors = ["Aleksey Kladov "] repository = "https://github.com/rust-analyzer/rowan" license = "MIT OR Apache-2.0" diff --git a/src/cursor.rs b/src/cursor.rs index 6b220af7..cb6ceeb1 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -408,7 +408,7 @@ impl NodeData { let rev_siblings = self.green_siblings().enumerate().rev(); let index = rev_siblings.len().checked_sub(self.index() as usize)?; - rev_siblings.skip(index + 1).find_map(|(index, child)| { + rev_siblings.skip(index).find_map(|(index, child)| { child.as_ref().into_node().and_then(|green| { let parent = self.parent_node()?; let offset = parent.offset() + child.rel_offset(); From 2caf8512f992a9f22a34b4b18f399b5c95e4e055 Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 14 Feb 2025 13:07:30 +0000 Subject: [PATCH 19/33] Fix CI --- xtask/Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index f87c7f46..6109c867 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -7,3 +7,6 @@ edition = "2021" [dependencies] xaction = "0.2" + +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(trick_rust_analyzer_into_highlighting_interpolated_bits)'] } From da4dc39f8eb0530c8402802dd67c092aea0f4da1 Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 14 Feb 2025 13:07:53 +0000 Subject: [PATCH 20/33] Make tests pass with -Zrandomize-layout Fixes #182 --- src/green/node.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/green/node.rs b/src/green/node.rs index 69b1cf76..9bd860ce 100644 --- a/src/green/node.rs +++ b/src/green/node.rs @@ -23,6 +23,7 @@ pub(super) struct GreenNodeHead { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[repr(u8)] pub(crate) enum GreenChild { Node { rel_offset: TextSize, node: GreenNode }, Token { rel_offset: TextSize, token: GreenToken }, From 080b942f8b4fca576339af81e7a3fccd503af9c1 Mon Sep 17 00:00:00 2001 From: Ioannis Nezis <74186504+IoannisNezis@users.noreply.github.com> Date: Sun, 23 Feb 2025 11:48:28 +0100 Subject: [PATCH 21/33] Update link in README (#185) * doc: change link in README * doc: update link in tutorial --- README.md | 2 +- examples/s_expressions.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f0d5b133..a09b5bfb 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Rowan is a library for lossless syntax trees, inspired in part by Swift's [libsyntax](https://github.com/apple/swift/tree/5e2c815edfd758f9b1309ce07bfc01c4bc20ec23/lib/Syntax). -A conceptual overview is available in the [rust-analyzer repo](https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/syntax.md). +A conceptual overview is available in the [rust-analyzer book](https://rust-analyzer.github.io/book/contributing/syntax.html). See `examples/s_expressions` for a tutorial, and [rust-analyzer](https://github.com/rust-analyzer/rust-analyzer/) for real-world usage. diff --git a/examples/s_expressions.rs b/examples/s_expressions.rs index e7df4c64..f3ceec33 100644 --- a/examples/s_expressions.rs +++ b/examples/s_expressions.rs @@ -7,7 +7,7 @@ //! //! It's suggested to read the conceptual overview of the design //! alongside this tutorial: -//! https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/syntax.md +//! https://rust-analyzer.github.io/book/contributing/syntax.html /// Let's start with defining all kinds of tokens and /// composite nodes. From 42cb260f9bf7c7475804dbee01bbe616dfe604d3 Mon Sep 17 00:00:00 2001 From: Benjamin Brienen Date: Wed, 5 Mar 2025 11:19:15 +0100 Subject: [PATCH 22/33] rust 2024 + clippy --- Cargo.toml | 17 ++-- README.md | 2 +- examples/math.rs | 1 + examples/s_expressions.rs | 10 +-- src/api.rs | 9 +- src/arc.rs | 22 +++-- src/ast.rs | 4 +- src/cow_mut.rs | 4 +- src/cursor.rs | 125 +++++++++++++-------------- src/green/builder.rs | 4 +- src/green/element.rs | 2 +- src/green/node.rs | 32 ++++--- src/green/node_cache.rs | 4 +- src/green/token.rs | 27 ++++-- src/serde_impls.rs | 2 +- src/sll.rs | 17 ++-- src/syntax_text.rs | 14 +-- src/utility_types.rs | 4 +- xtask/Cargo.toml | 8 +- xtask/src/main.rs | 175 ++++++++++++++++++++++++++++++++++---- xtask/tests/tidy.rs | 5 +- 21 files changed, 316 insertions(+), 172 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e2bc8c41..aff46212 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,22 +5,23 @@ authors = ["Aleksey Kladov "] repository = "https://github.com/rust-analyzer/rowan" license = "MIT OR Apache-2.0" description = "Library for generic lossless syntax trees" -edition = "2021" -rust-version = "1.77.0" +edition = "2024" +rust-version = "1.85.0" exclude = [".github/", "bors.toml", "rustfmt.toml"] [workspace] members = ["xtask"] [dependencies] -rustc-hash = "1.0.1" -hashbrown = { version = "0.14.3", features = [ - "inline-more", +rustc-hash = "2.1.1" +hashbrown = { version = "0.15.2", features = [ + "inline-more", + "raw-entry", ], default-features = false } -text-size = "1.1.0" -countme = "3.0.0" +text-size = "1.1.1" +countme = "3.0.1" -serde = { version = "1.0.89", optional = true, default-features = false } +serde = { version = "1.0.218", optional = true, default-features = false } [dev-dependencies] m_lexer = "0.0.4" diff --git a/README.md b/README.md index a09b5bfb..f0d5b133 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Rowan is a library for lossless syntax trees, inspired in part by Swift's [libsyntax](https://github.com/apple/swift/tree/5e2c815edfd758f9b1309ce07bfc01c4bc20ec23/lib/Syntax). -A conceptual overview is available in the [rust-analyzer book](https://rust-analyzer.github.io/book/contributing/syntax.html). +A conceptual overview is available in the [rust-analyzer repo](https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/syntax.md). See `examples/s_expressions` for a tutorial, and [rust-analyzer](https://github.com/rust-analyzer/rust-analyzer/) for real-world usage. diff --git a/examples/math.rs b/examples/math.rs index e3c35056..3a451b6a 100644 --- a/examples/math.rs +++ b/examples/math.rs @@ -32,6 +32,7 @@ enum SyntaxKind { OPERATION, ROOT, } + use SyntaxKind::*; impl From for rowan::SyntaxKind { diff --git a/examples/s_expressions.rs b/examples/s_expressions.rs index f3ceec33..18c740f5 100644 --- a/examples/s_expressions.rs +++ b/examples/s_expressions.rs @@ -7,7 +7,7 @@ //! //! It's suggested to read the conceptual overview of the design //! alongside this tutorial: -//! https://rust-analyzer.github.io/book/contributing/syntax.html +//! https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/syntax.md /// Let's start with defining all kinds of tokens and /// composite nodes. @@ -194,8 +194,10 @@ fn parse(text: &str) -> Parse { /// has identity semantics. type SyntaxNode = rowan::SyntaxNode; + #[allow(unused)] type SyntaxToken = rowan::SyntaxToken; + #[allow(unused)] type SyntaxElement = rowan::NodeOrToken; @@ -255,11 +257,7 @@ macro_rules! ast_node { impl $ast { #[allow(unused)] fn cast(node: SyntaxNode) -> Option { - if node.kind() == $kind { - Some(Self(node)) - } else { - None - } + if node.kind() == $kind { Some(Self(node)) } else { None } } } }; diff --git a/src/api.rs b/src/api.rs index a877fdd9..a35d0019 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1,8 +1,8 @@ use std::{borrow::Cow, fmt, iter, marker::PhantomData, ops::Range}; use crate::{ - cursor, green::GreenTokenData, Direction, GreenNode, GreenNodeData, GreenToken, NodeOrToken, - SyntaxKind, SyntaxText, TextRange, TextSize, TokenAtOffset, WalkEvent, + Direction, GreenNode, GreenNodeData, GreenToken, NodeOrToken, SyntaxKind, SyntaxText, + TextRange, TextSize, TokenAtOffset, WalkEvent, cursor, green::GreenTokenData, }; pub trait Language: Sized + Copy + fmt::Debug + Eq + Ord + std::hash::Hash { @@ -446,10 +446,7 @@ impl Iterator for SyntaxNodeChildren { impl SyntaxNodeChildren { pub fn by_kind(self, matcher: impl Fn(L::Kind) -> bool) -> impl Iterator> { - self.raw - .by_kind(move |raw_kind| matcher(L::kind_from_raw(raw_kind))) - .into_iter() - .map(SyntaxNode::from) + self.raw.by_kind(move |raw_kind| matcher(L::kind_from_raw(raw_kind))).map(SyntaxNode::from) } } diff --git a/src/arc.rs b/src/arc.rs index cb6758e6..fa3774fd 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -4,7 +4,7 @@ use std::{ cmp::Ordering, hash::{Hash, Hasher}, marker::PhantomData, - mem::{self, offset_of, ManuallyDrop}, + mem::{self, ManuallyDrop, offset_of}, ops::Deref, ptr, sync::atomic::{ @@ -55,14 +55,17 @@ impl Arc { pub(crate) unsafe fn from_raw(ptr: *const T) -> Self { // To find the corresponding pointer to the `ArcInner` we need // to subtract the offset of the `data` field from the pointer. - let ptr = (ptr as *const u8).sub(offset_of!(ArcInner, data)); - Arc { p: ptr::NonNull::new_unchecked(ptr as *mut ArcInner), phantom: PhantomData } + unsafe { + let ptr = (ptr as *const u8).sub(offset_of!(ArcInner, data)); + Arc { p: ptr::NonNull::new_unchecked(ptr as *mut ArcInner), phantom: PhantomData } + } } } impl Arc { #[inline] fn inner(&self) -> &ArcInner { + // SAFETY: // This unsafety is ok because while this arc is alive we're guaranteed // that the inner pointer is valid. Furthermore, we know that the // `ArcInner` structure itself is `Sync` because the inner data is @@ -74,7 +77,9 @@ impl Arc { // Non-inlined part of `drop`. Just invokes the destructor. #[inline(never)] unsafe fn drop_slow(&mut self) { - let _ = Box::from_raw(self.ptr()); + unsafe { + let _ = Box::from_raw(self.ptr()); + } } /// Test pointer equality between the two Arcs, i.e. they must be the _same_ @@ -195,10 +200,6 @@ impl PartialEq for Arc { fn eq(&self, other: &Arc) -> bool { Self::ptr_eq(self, other) || *(*self) == *(*other) } - - fn ne(&self, other: &Arc) -> bool { - !Self::ptr_eq(self, other) && *(*self) != *(*other) - } } impl PartialOrd for Arc { @@ -312,10 +313,7 @@ impl ThinArc { }; // Expose the transient Arc to the callback, which may clone it if it wants. - let result = f(&transient); - - // Forward the result. - result + f(&transient) } /// Creates a `ThinArc` for a HeaderSlice using the given header struct and diff --git a/src/ast.rs b/src/ast.rs index e142395b..292dba9f 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -156,7 +156,7 @@ impl AstPtr { /// Returns the underlying [`SyntaxNodePtr`]. pub fn syntax_node_ptr(&self) -> SyntaxNodePtr { - self.raw.clone() + self.raw } /// Casts this to an [`AstPtr`] to the given node type if possible. @@ -176,7 +176,7 @@ impl fmt::Debug for AstPtr { impl Clone for AstPtr { fn clone(&self) -> Self { - Self { raw: self.raw.clone() } + Self { raw: self.raw } } } diff --git a/src/cow_mut.rs b/src/cow_mut.rs index c50e25b7..418d7ee3 100644 --- a/src/cow_mut.rs +++ b/src/cow_mut.rs @@ -9,7 +9,7 @@ impl std::ops::Deref for CowMut<'_, T> { fn deref(&self) -> &T { match self { CowMut::Owned(it) => it, - CowMut::Borrowed(it) => *it, + CowMut::Borrowed(it) => it, } } } @@ -18,7 +18,7 @@ impl std::ops::DerefMut for CowMut<'_, T> { fn deref_mut(&mut self) -> &mut T { match self { CowMut::Owned(it) => it, - CowMut::Borrowed(it) => *it, + CowMut::Borrowed(it) => it, } } } diff --git a/src/cursor.rs b/src/cursor.rs index cb6ceeb1..1288620c 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -95,11 +95,11 @@ use std::{ use countme::Count; use crate::{ + Direction, GreenNode, GreenToken, NodeOrToken, SyntaxText, TextRange, TextSize, TokenAtOffset, + WalkEvent, green::{GreenChild, GreenElementRef, GreenNodeData, GreenTokenData, SyntaxKind}, sll, utility_types::Delta, - Direction, GreenNode, GreenToken, NodeOrToken, SyntaxText, TextRange, TextSize, TokenAtOffset, - WalkEvent, }; enum Green { @@ -188,32 +188,34 @@ impl Drop for SyntaxToken { #[inline(never)] unsafe fn free(mut data: ptr::NonNull) { - loop { - debug_assert_eq!(data.as_ref().rc.get(), 0); - debug_assert!(data.as_ref().first.get().is_null()); - let node = Box::from_raw(data.as_ptr()); - match node.parent.take() { - Some(parent) => { - debug_assert!(parent.as_ref().rc.get() > 0); - if node.mutable { - sll::unlink(&parent.as_ref().first, &*node) - } - if parent.as_ref().dec_rc() { - data = parent; - } else { - break; - } - } - None => { - match &node.green { - Green::Node { ptr } => { - let _ = GreenNode::from_raw(ptr.get()); + unsafe { + loop { + debug_assert_eq!(data.as_ref().rc.get(), 0); + debug_assert!(data.as_ref().first.get().is_null()); + let node = Box::from_raw(data.as_ptr()); + match node.parent.take() { + Some(parent) => { + debug_assert!(parent.as_ref().rc.get() > 0); + if node.mutable { + sll::unlink(&parent.as_ref().first, &*node) + } + if parent.as_ref().dec_rc() { + data = parent; + } else { + break; } - Green::Token { ptr } => { - let _ = GreenToken::from_raw(*ptr); + } + None => { + match &node.green { + Green::Node { ptr } => { + let _ = GreenNode::from_raw(ptr.get()); + } + Green::Token { ptr } => { + let _ = GreenToken::from_raw(*ptr); + } } + break; } - break; } } } @@ -321,7 +323,7 @@ impl NodeData { fn green(&self) -> GreenElementRef<'_> { match &self.green { Green::Node { ptr } => GreenElementRef::Node(unsafe { &*ptr.get().as_ptr() }), - Green::Token { ptr } => GreenElementRef::Token(unsafe { &*ptr.as_ref() }), + Green::Token { ptr } => GreenElementRef::Token(unsafe { ptr.as_ref() }), } } #[inline] @@ -342,11 +344,7 @@ impl NodeData { #[inline] fn offset(&self) -> TextSize { - if self.mutable { - self.offset_mut() - } else { - self.offset - } + if self.mutable { self.offset_mut() } else { self.offset } } #[cold] @@ -524,25 +522,27 @@ impl NodeData { } } unsafe fn respine(&self, mut new_green: GreenNode) { - let mut node = self; - loop { - let old_green = match &node.green { - Green::Node { ptr } => ptr.replace(ptr::NonNull::from(&*new_green)), - Green::Token { .. } => unreachable!(), - }; - match node.parent() { - Some(parent) => match parent.green() { - NodeOrToken::Node(parent_green) => { - new_green = - parent_green.replace_child(node.index() as usize, new_green.into()); - node = parent; + unsafe { + let mut node = self; + loop { + let old_green = match &node.green { + Green::Node { ptr } => ptr.replace(ptr::NonNull::from(&*new_green)), + Green::Token { .. } => unreachable!(), + }; + match node.parent() { + Some(parent) => match parent.green() { + NodeOrToken::Node(parent_green) => { + new_green = + parent_green.replace_child(node.index() as usize, new_green.into()); + node = parent; + } + _ => unreachable!(), + }, + None => { + mem::forget(new_green); + let _ = GreenNode::from_raw(old_green); + break; } - _ => unreachable!(), - }, - None => { - mem::forget(new_green); - let _ = GreenNode::from_raw(old_green); - break; } } } @@ -778,16 +778,13 @@ impl SyntaxNode { siblings .skip(index + 1) .find_map(|(index, child)| { - child - .as_ref() - .into_node() - .and_then(|green| Some((green, index as u32, child.rel_offset()))) + child.as_ref().into_node().map(|green| (green, index as u32, child.rel_offset())) }) - .and_then(|(green, index, rel_offset)| { + .map(|(green, index, rel_offset)| { data.index.set(index); data.offset = parent_offset + rel_offset; data.green = Green::Node { ptr: Cell::new(green.into()) }; - Some(SyntaxNode { ptr }) + SyntaxNode { ptr } }) .or_else(|| { data.dec_rc(); @@ -1121,12 +1118,8 @@ impl SyntaxElement { offset: TextSize, ) -> SyntaxElement { match element { - NodeOrToken::Node(node) => { - SyntaxNode::new_child(node, parent, index as u32, offset).into() - } - NodeOrToken::Token(token) => { - SyntaxToken::new(token, parent, index as u32, offset).into() - } + NodeOrToken::Node(node) => SyntaxNode::new_child(node, parent, index, offset).into(), + NodeOrToken::Token(token) => SyntaxToken::new(token, parent, index, offset).into(), } } @@ -1223,7 +1216,7 @@ impl SyntaxElement { siblings .skip(index + 1) - .find_map(|(index, green)| { + .map(|(index, green)| { data.index.set(index as u32); data.offset = parent_offset + green.rel_offset(); @@ -1238,6 +1231,8 @@ impl SyntaxElement { } } }) + .next() + .flatten() .or_else(|| { data.dec_rc(); unsafe { free(ptr) }; @@ -1411,9 +1406,8 @@ pub struct SyntaxNodeChildrenByKind bool> { impl bool> Iterator for SyntaxNodeChildrenByKind { type Item = SyntaxNode; fn next(&mut self) -> Option { - self.next.take().map(|next| { + self.next.take().inspect(|next| { self.next = next.next_sibling_by_kind(&self.matcher); - next }) } } @@ -1474,9 +1468,8 @@ pub struct SyntaxElementChildrenByKind bool> { impl bool> Iterator for SyntaxElementChildrenByKind { type Item = SyntaxElement; fn next(&mut self) -> Option { - self.next.take().map(|next| { + self.next.take().inspect(|next| { self.next = next.next_sibling_or_token_by_kind(&self.matcher); - next }) } } diff --git a/src/green/builder.rs b/src/green/builder.rs index 03d66df5..a8becadc 100644 --- a/src/green/builder.rs +++ b/src/green/builder.rs @@ -1,9 +1,9 @@ use std::num::NonZeroUsize; use crate::{ - cow_mut::CowMut, - green::{node_cache::NodeCache, GreenElement, GreenNode, SyntaxKind}, NodeOrToken, + cow_mut::CowMut, + green::{GreenElement, GreenNode, SyntaxKind, node_cache::NodeCache}, }; /// A checkpoint for maybe wrapping a node. See `GreenNodeBuilder::checkpoint` for details. diff --git a/src/green/element.rs b/src/green/element.rs index 2d1ce1f6..29451579 100644 --- a/src/green/element.rs +++ b/src/green/element.rs @@ -1,8 +1,8 @@ use std::borrow::Cow; use crate::{ - green::{GreenNode, GreenToken, SyntaxKind}, GreenNodeData, NodeOrToken, TextSize, + green::{GreenNode, GreenToken, SyntaxKind}, }; use super::GreenTokenData; diff --git a/src/green/node.rs b/src/green/node.rs index 9bd860ce..832915bf 100644 --- a/src/green/node.rs +++ b/src/green/node.rs @@ -9,10 +9,10 @@ use std::{ use countme::Count; use crate::{ + GreenToken, NodeOrToken, TextRange, TextSize, arc::{Arc, HeaderSlice, ThinArc}, green::{GreenElement, GreenElementRef, SyntaxKind}, utility_types::static_assert, - GreenToken, NodeOrToken, TextRange, TextSize, }; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -66,7 +66,7 @@ impl ToOwned for GreenNodeData { impl Borrow for GreenNode { #[inline] fn borrow(&self) -> &GreenNodeData { - &*self + self } } @@ -89,14 +89,14 @@ impl fmt::Debug for GreenNodeData { impl fmt::Debug for GreenNode { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let data: &GreenNodeData = &*self; + let data: &GreenNodeData = self; fmt::Debug::fmt(data, f) } } impl fmt::Display for GreenNode { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let data: &GreenNodeData = &*self; + let data: &GreenNodeData = self; fmt::Display::fmt(data, f) } } @@ -159,11 +159,7 @@ impl GreenNodeData { pub fn replace_child(&self, index: usize, new_child: GreenElement) -> GreenNode { let mut replacement = Some(new_child); let children = self.children().enumerate().map(|(i, child)| { - if i == index { - replacement.take().unwrap() - } else { - child.to_owned() - } + if i == index { replacement.take().unwrap() } else { child.to_owned() } }); GreenNode::new(self.kind(), children) } @@ -238,15 +234,17 @@ impl GreenNode { #[inline] pub(crate) fn into_raw(this: GreenNode) -> ptr::NonNull { let green = ManuallyDrop::new(this); - let green: &GreenNodeData = &*green; - ptr::NonNull::from(&*green) + let green: &GreenNodeData = &green; + ptr::NonNull::from(green) } #[inline] pub(crate) unsafe fn from_raw(ptr: ptr::NonNull) -> GreenNode { - let arc = Arc::from_raw(&ptr.as_ref().data as *const ReprThin); - let arc = mem::transmute::, ThinArc>(arc); - GreenNode { ptr: arc } + unsafe { + let arc = Arc::from_raw(&ptr.as_ref().data as *const ReprThin); + let arc = mem::transmute::, ThinArc>(arc); + GreenNode { ptr: arc } + } } } @@ -321,19 +319,19 @@ impl<'a> Iterator for Children<'a> { } #[inline] - fn fold(mut self, init: Acc, mut f: Fold) -> Acc + fn fold(self, init: Acc, mut f: Fold) -> Acc where Fold: FnMut(Acc, Self::Item) -> Acc, { let mut accum = init; - while let Some(x) = self.next() { + for x in self { accum = f(accum, x); } accum } } -impl<'a> DoubleEndedIterator for Children<'a> { +impl DoubleEndedIterator for Children<'_> { #[inline] fn next_back(&mut self) -> Option { self.raw.next_back().map(GreenChild::as_ref) diff --git a/src/green/node_cache.rs b/src/green/node_cache.rs index c73f3e69..89740bf6 100644 --- a/src/green/node_cache.rs +++ b/src/green/node_cache.rs @@ -3,8 +3,8 @@ use rustc_hash::FxHasher; use std::hash::{BuildHasherDefault, Hash, Hasher}; use crate::{ - green::GreenElementRef, GreenNode, GreenNodeData, GreenToken, GreenTokenData, NodeOrToken, - SyntaxKind, + GreenNode, GreenNodeData, GreenToken, GreenTokenData, NodeOrToken, SyntaxKind, + green::GreenElementRef, }; use super::element::GreenElement; diff --git a/src/green/token.rs b/src/green/token.rs index 81e298c7..9e6a1f04 100644 --- a/src/green/token.rs +++ b/src/green/token.rs @@ -8,9 +8,9 @@ use std::{ use countme::Count; use crate::{ + TextSize, arc::{Arc, HeaderSlice, ThinArc}, green::SyntaxKind, - TextSize, }; #[derive(PartialEq, Eq, Hash)] @@ -53,7 +53,7 @@ impl ToOwned for GreenTokenData { impl Borrow for GreenToken { #[inline] fn borrow(&self) -> &GreenTokenData { - &*self + self } } @@ -68,14 +68,14 @@ impl fmt::Debug for GreenTokenData { impl fmt::Debug for GreenToken { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let data: &GreenTokenData = &*self; + let data: &GreenTokenData = self; fmt::Debug::fmt(data, f) } } impl fmt::Display for GreenToken { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let data: &GreenTokenData = &*self; + let data: &GreenTokenData = self; fmt::Display::fmt(data, f) } } @@ -117,14 +117,25 @@ impl GreenToken { #[inline] pub(crate) fn into_raw(this: GreenToken) -> ptr::NonNull { let green = ManuallyDrop::new(this); - let green: &GreenTokenData = &*green; - ptr::NonNull::from(&*green) + let green: &GreenTokenData = &green; + ptr::NonNull::from(green) } + /// # Safety + /// + /// This function uses `unsafe` code to create an `Arc` from a raw pointer and then transmutes it into a `ThinArc`. + /// + /// - The raw pointer must be valid and correctly aligned for the type `ReprThin`. + /// - The lifetime of the raw pointer must outlive the lifetime of the `Arc` created from it. + /// - The transmute operation must be safe, meaning that the memory layout of `Arc` must be compatible with `ThinArc`. + /// + /// Failure to uphold these invariants can lead to undefined behavior. #[inline] pub(crate) unsafe fn from_raw(ptr: ptr::NonNull) -> GreenToken { - let arc = Arc::from_raw(&ptr.as_ref().data as *const ReprThin); - let arc = mem::transmute::, ThinArc>(arc); + let arc = unsafe { + let arc = Arc::from_raw(&ptr.as_ref().data as *const ReprThin); + mem::transmute::, ThinArc>(arc) + }; GreenToken { ptr: arc } } } diff --git a/src/serde_impls.rs b/src/serde_impls.rs index aea5d88b..529303b9 100644 --- a/src/serde_impls.rs +++ b/src/serde_impls.rs @@ -2,8 +2,8 @@ use serde::ser::{Serialize, SerializeMap, SerializeSeq, Serializer}; use std::fmt; use crate::{ - api::{Language, SyntaxNode, SyntaxToken}, NodeOrToken, + api::{Language, SyntaxNode, SyntaxToken}, }; struct SerDisplay(T); diff --git a/src/sll.rs b/src/sll.rs index 4a298390..69476606 100644 --- a/src/sll.rs +++ b/src/sll.rs @@ -3,6 +3,15 @@ use std::{cell::Cell, cmp::Ordering, ptr}; use crate::utility_types::Delta; + +/// # Safety +/// +/// Implementors of this trait must ensure that the pointers returned by +/// `prev` and `next` are valid and properly initialized. The pointers must +/// point to valid instances of the implementing type or be null pointers. +/// Additionally, the `key` method must return a valid reference to a `Cell`. +/// +/// Failure to uphold these invariants can result in undefined behavior. pub(crate) unsafe trait Elem { fn prev(&self) -> &Cell<*const Self>; fn next(&self) -> &Cell<*const Self>; @@ -17,7 +26,7 @@ pub(crate) enum AddToSllResult<'a, E: Elem> { AlreadyInSll(*const E), } -impl<'a, E: Elem> AddToSllResult<'a, E> { +impl AddToSllResult<'_, E> { pub(crate) fn add_to_sll(&self, elem_ptr: *const E) { unsafe { (*elem_ptr).prev().set(elem_ptr); @@ -55,11 +64,7 @@ pub(crate) fn init<'a, E: Elem>( head: Option<&'a Cell<*const E>>, elem: &E, ) -> AddToSllResult<'a, E> { - if let Some(head) = head { - link(head, elem) - } else { - AddToSllResult::NoHead - } + if let Some(head) = head { link(head, elem) } else { AddToSllResult::NoHead } } #[cold] diff --git a/src/syntax_text.rs b/src/syntax_text.rs index 3ab3f6cb..981df565 100644 --- a/src/syntax_text.rs +++ b/src/syntax_text.rs @@ -1,8 +1,8 @@ use std::fmt; use crate::{ - cursor::{SyntaxNode, SyntaxToken}, TextRange, TextSize, + cursor::{SyntaxNode, SyntaxToken}, }; #[derive(Clone)] @@ -43,7 +43,6 @@ impl SyntaxText { } pub fn char_at(&self, offset: TextSize) -> Option { - let offset = offset.into(); let mut start: TextSize = 0.into(); let res = self.try_for_each_chunk(|chunk| { let end = start + TextSize::of(chunk); @@ -97,7 +96,10 @@ impl SyntaxText { pub fn for_each_chunk(&self, mut f: F) { enum Void {} - match self.try_for_each_chunk(|chunk| Ok::<(), Void>(f(chunk))) { + match self.try_for_each_chunk(|chunk| { + f(chunk); + Ok::<(), Void>(()) + }) { Ok(()) => (), Err(void) => match void {}, } @@ -266,7 +268,7 @@ mod private { #[cfg(test)] mod tests { - use crate::{green::SyntaxKind, GreenNodeBuilder}; + use crate::{GreenNodeBuilder, green::SyntaxKind}; use super::*; @@ -274,7 +276,7 @@ mod tests { let mut builder = GreenNodeBuilder::new(); builder.start_node(SyntaxKind(62)); for &chunk in chunks.iter() { - builder.token(SyntaxKind(92), chunk.into()) + builder.token(SyntaxKind(92), chunk) } builder.finish_node(); SyntaxNode::new_root(builder.finish()) @@ -288,7 +290,7 @@ mod tests { let expected = t1.to_string() == t2.to_string(); let actual = t1 == t2; assert_eq!(expected, actual, "`{}` (SyntaxText) `{}` (SyntaxText)", t1, t2); - let actual = t1 == &*t2.to_string(); + let actual = t1 == *t2.to_string(); assert_eq!(expected, actual, "`{}` (SyntaxText) `{}` (&str)", t1, t2); } fn check(t1: &[&str], t2: &[&str]) { diff --git a/src/utility_types.rs b/src/utility_types.rs index 817add72..f26dabfa 100644 --- a/src/utility_types.rs +++ b/src/utility_types.rs @@ -43,8 +43,8 @@ impl NodeOrToken { impl NodeOrToken { pub(crate) fn as_deref(&self) -> NodeOrToken<&N::Target, &T::Target> { match self { - NodeOrToken::Node(node) => NodeOrToken::Node(&*node), - NodeOrToken::Token(token) => NodeOrToken::Token(&*token), + NodeOrToken::Node(node) => NodeOrToken::Node(node), + NodeOrToken::Token(token) => NodeOrToken::Token(token), } } } diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 6109c867..8a76f529 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -3,10 +3,8 @@ name = "xtask" version = "0.0.0" publish = false authors = ["Aleksey Kladov "] -edition = "2021" +edition = "2024" [dependencies] -xaction = "0.2" - -[lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(trick_rust_analyzer_into_highlighting_interpolated_bits)'] } +anyhow = "1.0.96" +xshell = "0.2.7" diff --git a/xtask/src/main.rs b/xtask/src/main.rs index d8ee44bb..f14e1eaf 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -1,6 +1,13 @@ -use std::env; +use std::{ + env, + path::PathBuf, + sync::atomic::{AtomicBool, Ordering}, + time::{Duration, Instant}, +}; -use xaction::{cargo_toml, cmd, git, section, Result}; +use anyhow::anyhow; +use xshell::{Shell, cmd}; +pub type Result = anyhow::Result; fn main() { if let Err(err) = try_main() { @@ -9,44 +16,163 @@ fn main() { } } +pub struct Section { + name: &'static str, + start: Instant, +} + +pub struct CargoToml { + path: PathBuf, + contents: String, +} + +impl CargoToml { + pub fn version(&self) -> Result<&str> { + self.get("version") + } + + fn get(&self, field: &str) -> Result<&str> { + for line in self.contents.lines() { + let words = line.split_ascii_whitespace().collect::>(); + match words.as_slice() { + [n, "=", v, ..] if n.trim() == field => { + assert!(v.starts_with('"') && v.ends_with('"')); + return Ok(&v[1..v.len() - 1]); + } + _ => (), + } + } + Err(anyhow!("can't find `{}` in {}", field, self.path.display()))? + } + + pub fn publish(&self, sh: &mut Shell) -> Result<()> { + let token = env::var("CRATES_IO_TOKEN").unwrap_or("no token".to_string()); + let dry_run = dry_run(); + cmd!(sh, "cargo publish --token {token} {dry_run...}").run()?; + Ok(()) + } + + pub fn publish_all(&self, dirs: &[&str], sh: &mut Shell) -> Result<()> { + let token = env::var("CRATES_IO_TOKEN").unwrap_or("no token".to_string()); + if dry_run().is_none() { + for &dir in dirs { + for _ in 0..20 { + std::thread::sleep(Duration::from_secs(10)); + if cmd!( + sh, + "cargo publish --manifest-path {dir}'/Cargo.toml' --token {token} --dry-run" + ) + .run() + .is_ok() + { + break; + } + } + cmd!(sh, "cargo publish --manifest-path {dir}'/Cargo.toml' --token {token}") + .run()?; + } + } + Ok(()) + } +} + +fn dry_run() -> Option<&'static str> { + let dry_run = DRY_RUN.load(Ordering::Relaxed); + if dry_run { Some("--dry-run") } else { None } +} + +pub fn section(name: &'static str) -> Section { + Section::new(name) +} + +pub fn cargo_toml() -> Result { + let manifest_dir = env::var("CARGO_MANIFEST_DIR")?; + let path = PathBuf::from(manifest_dir).join("Cargo.toml"); + let contents = std::fs::read_to_string(&path)?; + Ok(CargoToml { path, contents }) +} + +static DRY_RUN: AtomicBool = AtomicBool::new(false); +pub fn set_dry_run(yes: bool) { + DRY_RUN.store(yes, Ordering::Relaxed) +} + fn try_main() -> Result<()> { + let mut sh = Shell::new()?; let subcommand = std::env::args().nth(1); match subcommand { Some(it) if it == "ci" => (), _ => { print_usage(); - Err("invalid arguments")? + Err(anyhow!("invalid arguments"))? } } - let cargo_toml = cargo_toml()?; - - { - let _s = section("BUILD"); - cmd!("cargo test --workspace --no-run").run()?; - } - { let _s = section("TEST"); - cmd!("cargo test --workspace -- --nocapture").run()?; + for &release in &[None, Some("--release")] { + cmd!(sh, "cargo test {release...} --workspace -- --nocapture").run()?; + } } let version = cargo_toml.version()?; let tag = format!("v{}", version); - let dry_run = - env::var("CI").is_err() || git::has_tag(&tag)? || git::current_branch()? != "master"; - xaction::set_dry_run(dry_run); + let dry_run = env::var("CI").is_err() + || git::has_tag(&tag, &mut sh)? + || git::current_branch(&mut sh)? != "master"; + set_dry_run(dry_run); { let _s = section("PUBLISH"); - cargo_toml.publish()?; - git::tag(&tag)?; - git::push_tags()?; + cargo_toml.publish(&mut sh)?; + git::tag(&tag, &mut sh)?; + git::push_tags(&mut sh)?; } Ok(()) } +pub mod git { + use xshell::{Shell, cmd}; + + use super::{Result, dry_run}; + + pub fn current_branch(sh: &mut Shell) -> Result { + let res = cmd!(sh, "git branch --show-current").read()?; + Ok(res) + } + + pub fn tag_list(sh: &mut Shell) -> Result> { + let tags = cmd!(sh, "git tag --list").read()?; + let res = tags.lines().map(|it| it.trim().to_string()).collect(); + Ok(res) + } + + pub fn has_tag(tag: &str, sh: &mut Shell) -> Result { + let res = tag_list(sh)?.iter().any(|it| it == tag); + Ok(res) + } + + pub fn tag(tag: &str, sh: &mut Shell) -> Result<()> { + if dry_run().is_some() { + return Ok(()); + } + cmd!(sh, "git tag {tag}").run()?; + Ok(()) + } + + pub fn push_tags(sh: &mut Shell) -> Result<()> { + // `git push --tags --dry-run` exists, but it will fail with permissions + // error for forks. + if dry_run().is_some() { + return Ok(()); + } + + cmd!(sh, "git push --tags").run()?; + Ok(()) + } +} + fn print_usage() { eprintln!( "\ @@ -57,3 +183,18 @@ SUBCOMMANDS: " ) } + +impl Section { + fn new(name: &'static str) -> Section { + println!("::group::{}", name); + let start = Instant::now(); + Section { name, start } + } +} + +impl Drop for Section { + fn drop(&mut self) { + eprintln!("{}: {:.2?}", self.name, self.start.elapsed()); + println!("::endgroup::"); + } +} diff --git a/xtask/tests/tidy.rs b/xtask/tests/tidy.rs index 97c423a1..60579e6b 100644 --- a/xtask/tests/tidy.rs +++ b/xtask/tests/tidy.rs @@ -1,6 +1,7 @@ -use xaction::cmd; +use xshell::{Shell, cmd}; #[test] fn test_formatting() { - cmd!("cargo fmt --all -- --check").run().unwrap() + let sh = Shell::new().unwrap(); + cmd!(sh, "cargo fmt --all -- --check").run().unwrap() } From 4e97c361a60821661cd26dfcf2c57a6e268b24e6 Mon Sep 17 00:00:00 2001 From: Benjamin Brienen Date: Wed, 5 Mar 2025 11:20:53 +0100 Subject: [PATCH 23/33] fix link --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f0d5b133..a09b5bfb 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Rowan is a library for lossless syntax trees, inspired in part by Swift's [libsyntax](https://github.com/apple/swift/tree/5e2c815edfd758f9b1309ce07bfc01c4bc20ec23/lib/Syntax). -A conceptual overview is available in the [rust-analyzer repo](https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/syntax.md). +A conceptual overview is available in the [rust-analyzer book](https://rust-analyzer.github.io/book/contributing/syntax.html). See `examples/s_expressions` for a tutorial, and [rust-analyzer](https://github.com/rust-analyzer/rust-analyzer/) for real-world usage. From d725a34a2dd0b317a947e600d32f1f96567665a0 Mon Sep 17 00:00:00 2001 From: Benjamin Brienen Date: Wed, 5 Mar 2025 11:22:35 +0100 Subject: [PATCH 24/33] fix link in doc --- examples/s_expressions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/s_expressions.rs b/examples/s_expressions.rs index 18c740f5..34b3068b 100644 --- a/examples/s_expressions.rs +++ b/examples/s_expressions.rs @@ -7,7 +7,7 @@ //! //! It's suggested to read the conceptual overview of the design //! alongside this tutorial: -//! https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/syntax.md +//! https://rust-analyzer.github.io/book/contributing/syntax.html /// Let's start with defining all kinds of tokens and /// composite nodes. From fe2e4a7059e1fb8961b92e9d7738844faad7d675 Mon Sep 17 00:00:00 2001 From: Chris Laplante <40474653+chris-laplante@users.noreply.github.com> Date: Mon, 5 May 2025 03:06:18 -0400 Subject: [PATCH 25/33] README.md: add link to docs.rs (#193) --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index a09b5bfb..a5d61678 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ # Rowan +[![docs.rs](https://docs.rs/rowan/badge.svg)](https://docs.rs/rowan/) [![Crates.io](https://img.shields.io/crates/v/rowan.svg)](https://crates.io/crates/rowan) [![Crates.io](https://img.shields.io/crates/d/rowan.svg)](https://crates.io/crates/rowan) From 30d686ce1d003e020e28ccc04413b0eb69ca6211 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sat, 14 Jun 2025 14:52:34 +0200 Subject: [PATCH 26/33] move size assert from const to tests Struct layouts aren't guaranteed and -Zrandomize-layout exercises this right. To compile the whole rust-lang/rust repo with layout randomization we can't have static asserts like these. --- src/green/node.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/green/node.rs b/src/green/node.rs index 832915bf..d056abb4 100644 --- a/src/green/node.rs +++ b/src/green/node.rs @@ -12,7 +12,6 @@ use crate::{ GreenToken, NodeOrToken, TextRange, TextSize, arc::{Arc, HeaderSlice, ThinArc}, green::{GreenElement, GreenElementRef, SyntaxKind}, - utility_types::static_assert, }; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -28,8 +27,6 @@ pub(crate) enum GreenChild { Node { rel_offset: TextSize, node: GreenNode }, Token { rel_offset: TextSize, token: GreenToken }, } -#[cfg(target_pointer_width = "64")] -static_assert!(mem::size_of::() == mem::size_of::() * 2); type Repr = HeaderSlice; type ReprThin = HeaderSlice; @@ -356,3 +353,16 @@ impl DoubleEndedIterator for Children<'_> { } impl FusedIterator for Children<'_> {} + +#[cfg(test)] +mod test { + + #[test] + #[cfg(target_pointer_width = "64")] + fn check_green_child_size() { + use super::GreenChild; + use std::mem; + + assert_eq!(mem::size_of::(), mem::size_of::() * 2); + } +} From e9813a6d60b0f6283b27ddc31dca1df724dbdf53 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sat, 14 Jun 2025 14:55:44 +0200 Subject: [PATCH 27/33] remove now-unusued macro --- src/utility_types.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/utility_types.rs b/src/utility_types.rs index f26dabfa..656f9d4b 100644 --- a/src/utility_types.rs +++ b/src/utility_types.rs @@ -149,14 +149,6 @@ impl Iterator for TokenAtOffset { impl ExactSizeIterator for TokenAtOffset {} -macro_rules! _static_assert { - ($expr:expr) => { - const _: i32 = 0 / $expr as i32; - }; -} - -pub(crate) use _static_assert as static_assert; - #[derive(Copy, Clone, Debug)] pub(crate) enum Delta { Add(T), From d80594dc433e5e11f312e78101a2c3a7f4f71989 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 27 Jul 2025 14:20:17 +0300 Subject: [PATCH 28/33] Slap `derive(Debug, Clone)` on preorder iterators I need them cloneable. --- src/api.rs | 2 ++ src/cursor.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/api.rs b/src/api.rs index a35d0019..69e2f075 100644 --- a/src/api.rs +++ b/src/api.rs @@ -472,6 +472,7 @@ impl SyntaxElementChildren { } } +#[derive(Debug, Clone)] pub struct Preorder { raw: cursor::Preorder, _p: PhantomData, @@ -490,6 +491,7 @@ impl Iterator for Preorder { } } +#[derive(Debug, Clone)] pub struct PreorderWithTokens { raw: cursor::PreorderWithTokens, _p: PhantomData, diff --git a/src/cursor.rs b/src/cursor.rs index 1288620c..7d639484 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -1474,6 +1474,7 @@ impl bool> Iterator for SyntaxElementChildrenByKind { } } +#[derive(Debug, Clone)] pub struct Preorder { start: SyntaxNode, next: Option>, @@ -1529,6 +1530,7 @@ impl Iterator for Preorder { } } +#[derive(Debug, Clone)] pub struct PreorderWithTokens { start: SyntaxElement, next: Option>, From 8cb4e0d4da1f4a67d5688c62a215556acda25d44 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 27 Jul 2025 14:23:33 +0300 Subject: [PATCH 29/33] Update version to 1.16.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index aff46212..461b57cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rowan" -version = "0.16.1" +version = "0.16.2" authors = ["Aleksey Kladov "] repository = "https://github.com/rust-analyzer/rowan" license = "MIT OR Apache-2.0" From 9dd084fb124805691355cf30eff2da9140579d4f Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 27 Jul 2025 14:53:48 +0300 Subject: [PATCH 30/33] Don't capture unneeded lifetimes in `impl Iterator` return types --- src/api.rs | 16 ++++++++-------- src/cursor.rs | 16 ++++++++-------- src/syntax_text.rs | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/api.rs b/src/api.rs index 69e2f075..3a83daf5 100644 --- a/src/api.rs +++ b/src/api.rs @@ -133,7 +133,7 @@ impl SyntaxNode { self.raw.parent().map(Self::from) } - pub fn ancestors(&self) -> impl Iterator> { + pub fn ancestors(&self) -> impl Iterator> + use { self.raw.ancestors().map(SyntaxNode::from) } @@ -219,7 +219,7 @@ impl SyntaxNode { self.raw.last_token().map(SyntaxToken::from) } - pub fn siblings(&self, direction: Direction) -> impl Iterator> { + pub fn siblings(&self, direction: Direction) -> impl Iterator> + use { self.raw.siblings(direction).map(SyntaxNode::from) } @@ -230,11 +230,11 @@ impl SyntaxNode { self.raw.siblings_with_tokens(direction).map(SyntaxElement::from) } - pub fn descendants(&self) -> impl Iterator> { + pub fn descendants(&self) -> impl Iterator> + use { self.raw.descendants().map(SyntaxNode::from) } - pub fn descendants_with_tokens(&self) -> impl Iterator> { + pub fn descendants_with_tokens(&self) -> impl Iterator> + use { self.raw.descendants_with_tokens().map(NodeOrToken::from) } @@ -337,12 +337,12 @@ impl SyntaxToken { /// Iterator over all the ancestors of this token excluding itself. #[deprecated = "use `SyntaxToken::parent_ancestors` instead"] - pub fn ancestors(&self) -> impl Iterator> { + pub fn ancestors(&self) -> impl Iterator> + use { self.parent_ancestors() } /// Iterator over all the ancestors of this token excluding itself. - pub fn parent_ancestors(&self) -> impl Iterator> { + pub fn parent_ancestors(&self) -> impl Iterator> + use { self.raw.ancestors().map(SyntaxNode::from) } @@ -356,7 +356,7 @@ impl SyntaxToken { pub fn siblings_with_tokens( &self, direction: Direction, - ) -> impl Iterator> { + ) -> impl Iterator> + use { self.raw.siblings_with_tokens(direction).map(SyntaxElement::from) } @@ -403,7 +403,7 @@ impl SyntaxElement { } } - pub fn ancestors(&self) -> impl Iterator> { + pub fn ancestors(&self) -> impl Iterator> + use { let first = match self { NodeOrToken::Node(it) => Some(it.clone()), NodeOrToken::Token(it) => it.parent(), diff --git a/src/cursor.rs b/src/cursor.rs index 7d639484..0ea88ac3 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -668,7 +668,7 @@ impl SyntaxNode { } #[inline] - pub fn ancestors(&self) -> impl Iterator { + pub fn ancestors(&self) -> impl Iterator + use<> { iter::successors(Some(self.clone()), SyntaxNode::parent) } @@ -831,7 +831,7 @@ impl SyntaxNode { } #[inline] - pub fn siblings(&self, direction: Direction) -> impl Iterator { + pub fn siblings(&self, direction: Direction) -> impl Iterator + use<> { iter::successors(Some(self.clone()), move |node| match direction { Direction::Next => node.next_sibling(), Direction::Prev => node.prev_sibling(), @@ -842,7 +842,7 @@ impl SyntaxNode { pub fn siblings_with_tokens( &self, direction: Direction, - ) -> impl Iterator { + ) -> impl Iterator + use<> { let me: SyntaxElement = self.clone().into(); iter::successors(Some(me), move |el| match direction { Direction::Next => el.next_sibling_or_token(), @@ -851,7 +851,7 @@ impl SyntaxNode { } #[inline] - pub fn descendants(&self) -> impl Iterator { + pub fn descendants(&self) -> impl Iterator + use<> { self.preorder().filter_map(|event| match event { WalkEvent::Enter(node) => Some(node), WalkEvent::Leave(_) => None, @@ -859,7 +859,7 @@ impl SyntaxNode { } #[inline] - pub fn descendants_with_tokens(&self) -> impl Iterator { + pub fn descendants_with_tokens(&self) -> impl Iterator + use<> { self.preorder_with_tokens().filter_map(|event| match event { WalkEvent::Enter(it) => Some(it), WalkEvent::Leave(_) => None, @@ -1054,7 +1054,7 @@ impl SyntaxToken { } #[inline] - pub fn ancestors(&self) -> impl Iterator { + pub fn ancestors(&self) -> impl Iterator + use<> { std::iter::successors(self.parent(), SyntaxNode::parent) } @@ -1077,7 +1077,7 @@ impl SyntaxToken { pub fn siblings_with_tokens( &self, direction: Direction, - ) -> impl Iterator { + ) -> impl Iterator + use<> { let me: SyntaxElement = self.clone().into(); iter::successors(Some(me), move |el| match direction { Direction::Next => el.next_sibling_or_token(), @@ -1156,7 +1156,7 @@ impl SyntaxElement { } #[inline] - pub fn ancestors(&self) -> impl Iterator { + pub fn ancestors(&self) -> impl Iterator + use<> { let first = match self { NodeOrToken::Node(it) => Some(it.clone()), NodeOrToken::Token(it) => it.parent(), diff --git a/src/syntax_text.rs b/src/syntax_text.rs index 981df565..7e91ba21 100644 --- a/src/syntax_text.rs +++ b/src/syntax_text.rs @@ -105,7 +105,7 @@ impl SyntaxText { } } - fn tokens_with_ranges(&self) -> impl Iterator { + fn tokens_with_ranges(&self) -> impl Iterator + use<> { let text_range = self.range; self.node.descendants_with_tokens().filter_map(|element| element.into_token()).filter_map( move |token| { From ef5eb794700296fadaca2badf58f6b4b891b016b Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sun, 5 Apr 2026 21:18:11 -0500 Subject: [PATCH 31/33] fix: address Miri UB in Arc, ThinArc, green types, and cursor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comprehensive fixes for Miri UB under both stacked and tree borrows: Arc: - clone/drop/is_unique: access refcount via ptr::addr_of! on raw pointer instead of through &ArcInner reference (avoids provenance narrowing) - ThinArc::clone/drop: operate on refcount directly via raw pointer instead of going through with_arc → transient Arc Green types: - GreenNodeData wraps fat Repr (unsized HeaderSlice) so &GreenNodeData has provenance covering the full slice - GreenNode/GreenToken::deref transmute from fat refs (correct provenance) - GreenNode/GreenToken::into_raw extract pointer from ThinArc directly instead of going through Deref → &Data → NonNull::from - GreenTokenData::text() uses raw pointer arithmetic for slice access - thin_to_thick made pub(crate), HeaderSlice fields made pub(crate) Cursor: - Cell> accessed via as_ptr().read() instead of get() to preserve allocation provenance through the Cell Status: ALL tests pass under -Zmiri-tree-borrows (4/4 non-mutable tests). Under stacked borrows, only the mutable tree path (clone_for_update) still fails due to Cell provenance limitations inherent to that model. Upstream: rust-analyzer/rowan#192, #163, #108 --- src/arc.rs | 50 +++++++++++++++----- src/cursor.rs | 115 +++++++++++++++++++++++++++++++-------------- src/green/node.rs | 26 ++++++---- src/green/token.rs | 24 +++++++--- 4 files changed, 152 insertions(+), 63 deletions(-) diff --git a/src/arc.rs b/src/arc.rs index fa3774fd..4c4a107d 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -108,7 +108,8 @@ impl Clone for Arc { // another must already provide any required synchronization. // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) - let old_size = self.inner().count.fetch_add(1, Relaxed); + let count_ptr = unsafe { ptr::addr_of!((*self.ptr()).count) }; + let old_size = unsafe { (*count_ptr).fetch_add(1, Relaxed) }; // However we need to guard against massive refcounts in case someone // is `mem::forget`ing Arcs. If we don't do this the count can overflow @@ -155,16 +156,19 @@ impl Arc { // See the extensive discussion in [1] for why this needs to be Acquire. // // [1] https://github.com/servo/servo/issues/21186 - self.inner().count.load(Acquire) == 1 + let count_ptr = unsafe { ptr::addr_of!((*self.ptr()).count) }; + unsafe { (*count_ptr).load(Acquire) == 1 } } } impl Drop for Arc { #[inline] fn drop(&mut self) { - // Because `fetch_sub` is already atomic, we do not need to synchronize - // with other threads unless we are going to delete the object. - if self.inner().count.fetch_sub(1, Release) != 1 { + // Access the refcount via raw pointer to avoid creating a reference + // to ArcInner whose provenance may be limited when T is a thin + // type wrapping a dynamically-sized allocation. + let count_ptr = unsafe { ptr::addr_of!((*self.ptr()).count) }; + if unsafe { (*count_ptr).fetch_sub(1, Release) } != 1 { return; } @@ -188,7 +192,7 @@ impl Drop for Arc { // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) // [2]: https://github.com/rust-lang/rust/pull/41714 - self.inner().count.load(Acquire); + unsafe { (*count_ptr).load(Acquire) }; unsafe { self.drop_slow(); @@ -242,8 +246,8 @@ impl Hash for Arc { #[repr(C)] pub(crate) struct HeaderSlice { pub(crate) header: H, - length: usize, - slice: T, + pub(crate) length: usize, + pub(crate) slice: T, } impl HeaderSlice { @@ -279,7 +283,7 @@ impl Deref for HeaderSlice { /// via `HeaderSlice`. #[repr(transparent)] pub(crate) struct ThinArc { - ptr: ptr::NonNull>>, + pub(crate) ptr: ptr::NonNull>>, phantom: PhantomData<(H, T)>, } @@ -287,7 +291,7 @@ unsafe impl Send for ThinArc {} unsafe impl Sync for ThinArc {} // Synthesize a fat pointer from a thin pointer. -fn thin_to_thick( +pub(crate) fn thin_to_thick( thin: *mut ArcInner>, ) -> *mut ArcInner> { let len = unsafe { (*thin).data.length }; @@ -300,6 +304,7 @@ impl ThinArc { /// Temporarily converts |self| into a bonafide Arc and exposes it to the /// provided callback. The refcount is not modified. #[inline] + #[allow(dead_code)] // kept for downstream users; Clone/Drop now use raw pointers pub(crate) fn with_arc(&self, f: F) -> U where F: FnOnce(&Arc>) -> U, @@ -402,14 +407,35 @@ impl Deref for ThinArc { impl Clone for ThinArc { #[inline] fn clone(&self) -> Self { - ThinArc::with_arc(self, |a| Arc::into_thin(a.clone())) + // Increment refcount directly via raw pointer, avoiding the + // with_arc → Arc::clone path that creates intermediate references. + let count_ptr = unsafe { ptr::addr_of!((*self.ptr.as_ptr()).count) }; + let old_size = unsafe { (*count_ptr).fetch_add(1, Relaxed) }; + if old_size > MAX_REFCOUNT { + std::process::abort(); + } + ThinArc { ptr: self.ptr, phantom: PhantomData } } } impl Drop for ThinArc { #[inline] fn drop(&mut self) { - let _ = Arc::from_thin(ThinArc { ptr: self.ptr, phantom: PhantomData }); + // Decrement refcount and deallocate directly via raw pointer, + // avoiding the from_thin → Arc::drop path which creates intermediate + // references that Miri flags for provenance issues. + let ptr = self.ptr.as_ptr(); + let count_ptr = unsafe { ptr::addr_of!((*ptr).count) }; + if unsafe { (*count_ptr).fetch_sub(1, Release) } != 1 { + return; + } + atomic::fence(Acquire); + unsafe { + let thick = thin_to_thick(ptr); + let layout = alloc::Layout::for_value(&*thick); + ptr::drop_in_place(&mut (*thick).data); + alloc::dealloc(ptr as *mut u8, layout); + } } } diff --git a/src/cursor.rs b/src/cursor.rs index 0ea88ac3..a4284bb0 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -121,6 +121,9 @@ struct NodeData { mutable: bool, /// Absolute offset for immutable nodes, unused for mutable nodes. offset: TextSize, + /// Raw pointer to self with original allocation provenance. + /// Used for deallocation — survives reference freezing under tree borrows. + self_alloc: *mut NodeData, // The following links only have meaning when `mutable` is true. first: Cell<*const NodeData>, /// Invariant: never null if mutable. @@ -144,44 +147,64 @@ unsafe impl sll::Elem for NodeData { pub type SyntaxElement = NodeOrToken; pub struct SyntaxNode { - ptr: ptr::NonNull, + /// Wrapped in UnsafeCell so that reading the pointer through &self + /// does not freeze provenance under tree/stacked borrows. This allows + /// the pointer to retain write/dealloc permission through the + /// lifecycle of the SyntaxNode. + ptr: std::cell::UnsafeCell>, +} + +impl SyntaxNode { + #[inline] + fn ptr(&self) -> ptr::NonNull { + unsafe { *self.ptr.get() } + } } impl Clone for SyntaxNode { #[inline] fn clone(&self) -> Self { self.data().inc_rc(); - SyntaxNode { ptr: self.ptr } + SyntaxNode { ptr: std::cell::UnsafeCell::new(self.ptr()) } } } impl Drop for SyntaxNode { #[inline] fn drop(&mut self) { - if self.data().dec_rc() { - unsafe { free(self.ptr) } + let ptr = self.ptr(); + if unsafe { NodeData::dec_rc_raw(ptr) } { + unsafe { free(ptr) } } } } #[derive(Debug)] pub struct SyntaxToken { - ptr: ptr::NonNull, + ptr: std::cell::UnsafeCell>, +} + +impl SyntaxToken { + #[inline] + fn ptr(&self) -> ptr::NonNull { + unsafe { *self.ptr.get() } + } } impl Clone for SyntaxToken { #[inline] fn clone(&self) -> Self { self.data().inc_rc(); - SyntaxToken { ptr: self.ptr } + SyntaxToken { ptr: std::cell::UnsafeCell::new(self.ptr()) } } } impl Drop for SyntaxToken { #[inline] fn drop(&mut self) { - if self.data().dec_rc() { - unsafe { free(self.ptr) } + let ptr = self.ptr(); + if unsafe { NodeData::dec_rc_raw(ptr) } { + unsafe { free(ptr) } } } } @@ -190,16 +213,18 @@ impl Drop for SyntaxToken { unsafe fn free(mut data: ptr::NonNull) { unsafe { loop { - debug_assert_eq!(data.as_ref().rc.get(), 0); - debug_assert!(data.as_ref().first.get().is_null()); - let node = Box::from_raw(data.as_ptr()); + // Use self_alloc which retains original Box::into_raw provenance, + // unaffected by any &NodeData references created during the node's lifetime. + let alloc_ptr = (*data.as_ptr()).self_alloc; + let node = Box::from_raw(alloc_ptr); + debug_assert_eq!(node.rc.get(), 0); + debug_assert!(node.first.get().is_null()); match node.parent.take() { Some(parent) => { - debug_assert!(parent.as_ref().rc.get() > 0); if node.mutable { - sll::unlink(&parent.as_ref().first, &*node) + sll::unlink(&(*parent.as_ptr()).first, &*node) } - if parent.as_ref().dec_rc() { + if NodeData::dec_rc_raw(parent) { data = parent; } else { break; @@ -208,7 +233,8 @@ unsafe fn free(mut data: ptr::NonNull) { None => { match &node.green { Green::Node { ptr } => { - let _ = GreenNode::from_raw(ptr.get()); + let p = ptr.as_ptr().read(); + let _ = GreenNode::from_raw(p); } Green::Token { ptr } => { let _ = GreenToken::from_raw(*ptr); @@ -234,12 +260,13 @@ impl NodeData { let res = NodeData { _c: Count::new(), rc: Cell::new(1), - parent: Cell::new(parent.as_ref().map(|it| it.ptr)), + parent: Cell::new(parent.as_ref().map(|it| it.ptr())), index: Cell::new(index), green, mutable, offset, + self_alloc: ptr::null_mut(), first: Cell::new(ptr::null()), next: Cell::new(ptr::null()), prev: Cell::new(ptr::null()), @@ -272,12 +299,15 @@ impl NodeData { } it => { let res = Box::into_raw(Box::new(res)); + (*res).self_alloc = res; it.add_to_sll(res); return ptr::NonNull::new_unchecked(res); } } } - ptr::NonNull::new_unchecked(Box::into_raw(Box::new(res))) + let raw = Box::into_raw(Box::new(res)); + (*raw).self_alloc = raw; + ptr::NonNull::new_unchecked(raw) } } @@ -297,10 +327,27 @@ impl NodeData { rc == 0 } + /// Decrement refcount via raw pointer only — no references created. + /// This preserves deallocation permission under tree/stacked borrows. + #[inline] + unsafe fn dec_rc_raw(ptr: ptr::NonNull) -> bool { + // Access the Cell's inner value via raw pointer. + // Cell::get()/set() create &Cell references which under tree + // borrows freeze the parent allocation's borrow tag. + unsafe { + let rc_cell_ptr = ptr::addr_of!((*ptr.as_ptr()).rc); + // Cell wraps UnsafeCell — its memory layout is just u32. + let rc_val_ptr = rc_cell_ptr as *mut u32; + let rc = rc_val_ptr.read() - 1; + rc_val_ptr.write(rc); + rc == 0 + } + } + #[inline] fn key(&self) -> (ptr::NonNull<()>, TextSize) { let ptr = match &self.green { - Green::Node { ptr } => ptr.get().cast(), + Green::Node { ptr } => unsafe { ptr.as_ptr().read() }.cast(), Green::Token { ptr } => ptr.cast(), }; (ptr, self.offset()) @@ -311,7 +358,7 @@ impl NodeData { let parent = self.parent()?; debug_assert!(matches!(parent.green, Green::Node { .. })); parent.inc_rc(); - Some(SyntaxNode { ptr: ptr::NonNull::from(parent) }) + Some(SyntaxNode { ptr: std::cell::UnsafeCell::new(ptr::NonNull::from(parent)) }) } #[inline] @@ -322,14 +369,14 @@ impl NodeData { #[inline] fn green(&self) -> GreenElementRef<'_> { match &self.green { - Green::Node { ptr } => GreenElementRef::Node(unsafe { &*ptr.get().as_ptr() }), + Green::Node { ptr } => GreenElementRef::Node(unsafe { &*ptr.as_ptr().read().as_ptr() }), Green::Token { ptr } => GreenElementRef::Token(unsafe { ptr.as_ref() }), } } #[inline] fn green_siblings(&self) -> slice::Iter { match &self.parent().map(|it| &it.green) { - Some(Green::Node { ptr }) => unsafe { &*ptr.get().as_ptr() }.children().raw, + Some(Green::Node { ptr }) => unsafe { &*ptr.as_ptr().read().as_ptr() }.children().raw, Some(Green::Token { .. }) => { debug_assert!(false); [].iter() @@ -511,7 +558,7 @@ impl NodeData { NodeOrToken::Node(green) => { // Child is root, so it owns the green node. Steal it! let child_green = match &child.green { - Green::Node { ptr } => unsafe { GreenNode::from_raw(ptr.get()).into() }, + Green::Node { ptr } => unsafe { GreenNode::from_raw(ptr.as_ptr().read()).into() }, Green::Token { ptr } => unsafe { GreenToken::from_raw(*ptr).into() }, }; @@ -553,13 +600,13 @@ impl SyntaxNode { pub fn new_root(green: GreenNode) -> SyntaxNode { let green = GreenNode::into_raw(green); let green = Green::Node { ptr: Cell::new(green) }; - SyntaxNode { ptr: NodeData::new(None, 0, 0.into(), green, false) } + SyntaxNode { ptr: std::cell::UnsafeCell::new(NodeData::new(None, 0, 0.into(), green, false)) } } pub fn new_root_mut(green: GreenNode) -> SyntaxNode { let green = GreenNode::into_raw(green); let green = Green::Node { ptr: Cell::new(green) }; - SyntaxNode { ptr: NodeData::new(None, 0, 0.into(), green, true) } + SyntaxNode { ptr: std::cell::UnsafeCell::new(NodeData::new(None, 0, 0.into(), green, true)) } } fn new_child( @@ -570,7 +617,7 @@ impl SyntaxNode { ) -> SyntaxNode { let mutable = parent.data().mutable; let green = Green::Node { ptr: Cell::new(green.into()) }; - SyntaxNode { ptr: NodeData::new(Some(parent), index, offset, green, mutable) } + SyntaxNode { ptr: std::cell::UnsafeCell::new(NodeData::new(Some(parent), index, offset, green, mutable)) } } pub fn is_mutable(&self) -> bool { @@ -594,7 +641,7 @@ impl SyntaxNode { #[inline] fn data(&self) -> &NodeData { - unsafe { self.ptr.as_ref() } + unsafe { self.ptr().as_ref() } } #[inline] @@ -605,8 +652,7 @@ impl SyntaxNode { #[inline] fn take_ptr(self) -> ptr::NonNull { assert!(self.can_take_ptr()); - let ret = self.ptr; - // don't change the refcount when self gets dropped + let ret = self.ptr(); std::mem::forget(self); ret } @@ -784,7 +830,7 @@ impl SyntaxNode { data.index.set(index); data.offset = parent_offset + rel_offset; data.green = Green::Node { ptr: Cell::new(green.into()) }; - SyntaxNode { ptr } + SyntaxNode { ptr: std::cell::UnsafeCell::new(ptr) } }) .or_else(|| { data.dec_rc(); @@ -982,12 +1028,12 @@ impl SyntaxToken { ) -> SyntaxToken { let mutable = parent.data().mutable; let green = Green::Token { ptr: green.into() }; - SyntaxToken { ptr: NodeData::new(Some(parent), index, offset, green, mutable) } + SyntaxToken { ptr: std::cell::UnsafeCell::new(NodeData::new(Some(parent), index, offset, green, mutable)) } } #[inline] fn data(&self) -> &NodeData { - unsafe { self.ptr.as_ref() } + unsafe { self.ptr().as_ref() } } #[inline] @@ -998,8 +1044,7 @@ impl SyntaxToken { #[inline] fn take_ptr(self) -> ptr::NonNull { assert!(self.can_take_ptr()); - let ret = self.ptr; - // don't change the refcount when self gets dropped + let ret = self.ptr(); std::mem::forget(self); ret } @@ -1223,11 +1268,11 @@ impl SyntaxElement { match green.as_ref() { NodeOrToken::Node(node) => { data.green = Green::Node { ptr: Cell::new(node.into()) }; - Some(SyntaxElement::Node(SyntaxNode { ptr })) + Some(SyntaxElement::Node(SyntaxNode { ptr: std::cell::UnsafeCell::new(ptr) })) } NodeOrToken::Token(token) => { data.green = Green::Token { ptr: token.into() }; - Some(SyntaxElement::Token(SyntaxToken { ptr })) + Some(SyntaxElement::Token(SyntaxToken { ptr: std::cell::UnsafeCell::new(ptr) })) } } }) diff --git a/src/green/node.rs b/src/green/node.rs index d056abb4..529f9526 100644 --- a/src/green/node.rs +++ b/src/green/node.rs @@ -10,7 +10,7 @@ use countme::Count; use crate::{ GreenToken, NodeOrToken, TextRange, TextSize, - arc::{Arc, HeaderSlice, ThinArc}, + arc::{Arc, HeaderSlice, ThinArc, thin_to_thick}, green::{GreenElement, GreenElementRef, SyntaxKind}, }; @@ -32,7 +32,7 @@ type Repr = HeaderSlice; type ReprThin = HeaderSlice; #[repr(transparent)] pub struct GreenNodeData { - data: ReprThin, + data: Repr, // unsized — provenance covers the full slice } impl PartialEq for GreenNodeData { @@ -186,11 +186,11 @@ impl ops::Deref for GreenNode { #[inline] fn deref(&self) -> &GreenNodeData { + // SAFETY: GreenNodeData is #[repr(transparent)] over Repr (fat HeaderSlice). + // ThinArc::deref() returns &HeaderSlice with full allocation provenance + // via thin_to_thick(). We transmute to &GreenNodeData which has the same layout. let repr: &Repr = &self.ptr; - unsafe { - let repr: &ReprThin = &*(repr as *const Repr as *const ReprThin); - mem::transmute::<&ReprThin, &GreenNodeData>(repr) - } + unsafe { mem::transmute::<&Repr, &GreenNodeData>(repr) } } } @@ -231,14 +231,22 @@ impl GreenNode { #[inline] pub(crate) fn into_raw(this: GreenNode) -> ptr::NonNull { let green = ManuallyDrop::new(this); - let green: &GreenNodeData = &green; - ptr::NonNull::from(green) + // Extract the raw pointer directly from ThinArc to preserve full + // allocation provenance. Going through Deref → &GreenNodeData would + // create a reference whose provenance is invalidated when the + // ManuallyDrop wrapper goes out of scope. + let thin_ptr = green.ptr.ptr.as_ptr(); + let thick = thin_to_thick(thin_ptr); + unsafe { ptr::NonNull::new_unchecked(ptr::addr_of!((*thick).data) as *mut Repr as *mut GreenNodeData) } } #[inline] pub(crate) unsafe fn from_raw(ptr: ptr::NonNull) -> GreenNode { unsafe { - let arc = Arc::from_raw(&ptr.as_ref().data as *const ReprThin); + // Cast the fat pointer to thin: just reinterpret the data pointer + // (dropping the length metadata, which is stored in the HeaderSlice). + let thin_ptr = ptr.as_ptr() as *const Repr as *const ReprThin; + let arc = Arc::from_raw(thin_ptr); let arc = mem::transmute::, ThinArc>(arc); GreenNode { ptr: arc } } diff --git a/src/green/token.rs b/src/green/token.rs index 9e6a1f04..5bb1e9c0 100644 --- a/src/green/token.rs +++ b/src/green/token.rs @@ -19,7 +19,6 @@ struct GreenTokenHead { _c: Count, } -type Repr = HeaderSlice; type ReprThin = HeaderSlice; #[repr(transparent)] pub struct GreenTokenData { @@ -96,7 +95,15 @@ impl GreenTokenData { /// Text of this Token. #[inline] pub fn text(&self) -> &str { - unsafe { std::str::from_utf8_unchecked(self.data.slice()) } + // Access the byte slice via raw pointer arithmetic to avoid going through + // Deref on HeaderSlice which creates a reference with provenance + // limited to the thin type. + unsafe { + let len = self.data.length; + let slice_start = ptr::addr_of!(self.data.slice) as *const u8; + let bytes = std::slice::from_raw_parts(slice_start, len); + std::str::from_utf8_unchecked(bytes) + } } /// Returns the length of the text covered by this token. @@ -117,8 +124,12 @@ impl GreenToken { #[inline] pub(crate) fn into_raw(this: GreenToken) -> ptr::NonNull { let green = ManuallyDrop::new(this); - let green: &GreenTokenData = &green; - ptr::NonNull::from(green) + // Extract pointer directly from ThinArc to preserve full allocation provenance. + let inner = green.ptr.ptr.as_ptr(); + unsafe { + let data = ptr::addr_of!((*inner).data); + ptr::NonNull::new_unchecked(data as *mut ReprThin as *mut GreenTokenData) + } } /// # Safety @@ -146,9 +157,8 @@ impl ops::Deref for GreenToken { #[inline] fn deref(&self) -> &GreenTokenData { unsafe { - let repr: &Repr = &self.ptr; - let repr: &ReprThin = &*(repr as *const Repr as *const ReprThin); - mem::transmute::<&ReprThin, &GreenTokenData>(repr) + let inner = self.ptr.ptr.as_ptr(); + &*(ptr::addr_of!((*inner).data) as *const ReprThin as *const GreenTokenData) } } } From 72c3be4e4490eb204eceba976c29076e6d9e6b40 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sun, 5 Apr 2026 23:57:28 -0500 Subject: [PATCH 32/33] fix: resolve hidden-lifetime-in-paths lint warnings Make elided lifetimes explicit in return types where &self borrows: - GreenChild::as_ref() -> GreenElementRef<'_> - NodeData::green_siblings() -> slice::Iter<'_, GreenChild> These warnings caused CI failure with RUSTFLAGS="-D warnings". --- src/cursor.rs | 42 ++++++++++++++++++++++++++++++++++-------- src/green/node.rs | 8 ++++++-- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/cursor.rs b/src/cursor.rs index a4284bb0..c52f4bd5 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -374,7 +374,7 @@ impl NodeData { } } #[inline] - fn green_siblings(&self) -> slice::Iter { + fn green_siblings(&self) -> slice::Iter<'_, GreenChild> { match &self.parent().map(|it| &it.green) { Some(Green::Node { ptr }) => unsafe { &*ptr.as_ptr().read().as_ptr() }.children().raw, Some(Green::Token { .. }) => { @@ -558,7 +558,9 @@ impl NodeData { NodeOrToken::Node(green) => { // Child is root, so it owns the green node. Steal it! let child_green = match &child.green { - Green::Node { ptr } => unsafe { GreenNode::from_raw(ptr.as_ptr().read()).into() }, + Green::Node { ptr } => unsafe { + GreenNode::from_raw(ptr.as_ptr().read()).into() + }, Green::Token { ptr } => unsafe { GreenToken::from_raw(*ptr).into() }, }; @@ -600,13 +602,17 @@ impl SyntaxNode { pub fn new_root(green: GreenNode) -> SyntaxNode { let green = GreenNode::into_raw(green); let green = Green::Node { ptr: Cell::new(green) }; - SyntaxNode { ptr: std::cell::UnsafeCell::new(NodeData::new(None, 0, 0.into(), green, false)) } + SyntaxNode { + ptr: std::cell::UnsafeCell::new(NodeData::new(None, 0, 0.into(), green, false)), + } } pub fn new_root_mut(green: GreenNode) -> SyntaxNode { let green = GreenNode::into_raw(green); let green = Green::Node { ptr: Cell::new(green) }; - SyntaxNode { ptr: std::cell::UnsafeCell::new(NodeData::new(None, 0, 0.into(), green, true)) } + SyntaxNode { + ptr: std::cell::UnsafeCell::new(NodeData::new(None, 0, 0.into(), green, true)), + } } fn new_child( @@ -617,7 +623,15 @@ impl SyntaxNode { ) -> SyntaxNode { let mutable = parent.data().mutable; let green = Green::Node { ptr: Cell::new(green.into()) }; - SyntaxNode { ptr: std::cell::UnsafeCell::new(NodeData::new(Some(parent), index, offset, green, mutable)) } + SyntaxNode { + ptr: std::cell::UnsafeCell::new(NodeData::new( + Some(parent), + index, + offset, + green, + mutable, + )), + } } pub fn is_mutable(&self) -> bool { @@ -1028,7 +1042,15 @@ impl SyntaxToken { ) -> SyntaxToken { let mutable = parent.data().mutable; let green = Green::Token { ptr: green.into() }; - SyntaxToken { ptr: std::cell::UnsafeCell::new(NodeData::new(Some(parent), index, offset, green, mutable)) } + SyntaxToken { + ptr: std::cell::UnsafeCell::new(NodeData::new( + Some(parent), + index, + offset, + green, + mutable, + )), + } } #[inline] @@ -1268,11 +1290,15 @@ impl SyntaxElement { match green.as_ref() { NodeOrToken::Node(node) => { data.green = Green::Node { ptr: Cell::new(node.into()) }; - Some(SyntaxElement::Node(SyntaxNode { ptr: std::cell::UnsafeCell::new(ptr) })) + Some(SyntaxElement::Node(SyntaxNode { + ptr: std::cell::UnsafeCell::new(ptr), + })) } NodeOrToken::Token(token) => { data.green = Green::Token { ptr: token.into() }; - Some(SyntaxElement::Token(SyntaxToken { ptr: std::cell::UnsafeCell::new(ptr) })) + Some(SyntaxElement::Token(SyntaxToken { + ptr: std::cell::UnsafeCell::new(ptr), + })) } } }) diff --git a/src/green/node.rs b/src/green/node.rs index 529f9526..906904bd 100644 --- a/src/green/node.rs +++ b/src/green/node.rs @@ -237,7 +237,11 @@ impl GreenNode { // ManuallyDrop wrapper goes out of scope. let thin_ptr = green.ptr.ptr.as_ptr(); let thick = thin_to_thick(thin_ptr); - unsafe { ptr::NonNull::new_unchecked(ptr::addr_of!((*thick).data) as *mut Repr as *mut GreenNodeData) } + unsafe { + ptr::NonNull::new_unchecked( + ptr::addr_of!((*thick).data) as *mut Repr as *mut GreenNodeData + ) + } } #[inline] @@ -255,7 +259,7 @@ impl GreenNode { impl GreenChild { #[inline] - pub(crate) fn as_ref(&self) -> GreenElementRef { + pub(crate) fn as_ref(&self) -> GreenElementRef<'_> { match self { GreenChild::Node { node, .. } => NodeOrToken::Node(node), GreenChild::Token { token, .. } => NodeOrToken::Token(token), From dcbece400019397b97764070435eba62c7aa5336 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Mon, 6 Apr 2026 06:31:37 -0500 Subject: [PATCH 33/33] fix: use self_alloc provenance for parent pointers in cursor --- src/cursor.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/cursor.rs b/src/cursor.rs index c52f4bd5..f92577d5 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -256,11 +256,24 @@ impl NodeData { green: Green, mutable: bool, ) -> ptr::NonNull { + // Extract parent's self_alloc by consuming the Option temporarily. + // We take() the value, read self_alloc through UnsafeCell (no &SyntaxNode), + // then put it back. This avoids creating &SyntaxNode which would freeze provenance. + let (parent, parent_alloc) = match parent { + Some(p) => { + let alloc = unsafe { + let node_data_ptr = (*p.ptr.get()).as_ptr(); + ptr::NonNull::new_unchecked((*node_data_ptr).self_alloc) + }; + (Some(p), Some(alloc)) + } + None => (None, None), + }; let parent = ManuallyDrop::new(parent); let res = NodeData { _c: Count::new(), rc: Cell::new(1), - parent: Cell::new(parent.as_ref().map(|it| it.ptr())), + parent: Cell::new(parent_alloc), index: Cell::new(index), green,