From 32949733eeaa62ff91b08dfbe51e55fa0c737834 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Thu, 31 Oct 2024 11:01:59 +0100 Subject: [PATCH] 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 aa66be0..a6d84c6 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 })