Skip to content

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Sep 15, 2025

Fixes regression introduced by #1302, resulting in too strict borrow checks.

The lifetime's intention was to express the "outlives" relation from &self to PassiveGd<'_, T>. This worked, however it also introduced a breaking change in the BaseRef + BaseMut guards. Code such as the following now caused borrow errors:

for i in 0..self.base().get_child_count() {
    self.base_mut().rotate(10.0);
}

This isn't very intuitive, nor particularly helpful for soundness. This PR reverts the PartialGd<'gd, T> to PartialGd<T> with unsafe constructors. In practice, quite some unsafety was still needed anyway (e.g. BaseMut needed an unsafe 'static intermediate reference).

@Bromeon Bromeon added bug c: core Core components labels Sep 15, 2025
The lifetime's intention was to express the "outlives" relation from `&self` to
`PassiveGd<'_, T>`. This worked, however it also introduced a breaking change in the
`BaseRef` + `BaseMut` guards. Code such as the following now caused borrow errors:

    for i in 0..self.base().get_child_count() {
        self.base_mut().rotate(10.0);
    }

This isn't very intuitive, nor particularly helpful for soundness. This reverts the
`PartialGd<'gd, T>` to `PartialGd<T>` with unsafe constructors. In practice, quite
some unsafety was still needed anyway (e.g. `BaseMut` needed an unsafe `'static`
intermediate reference).
@Bromeon Bromeon force-pushed the bugfix/base-borrows branch from c465441 to 4a5dfd7 Compare September 15, 2025 21:43
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1312

Copy link
Contributor

@Yarwin Yarwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Yarwin Yarwin added this pull request to the merge queue Sep 16, 2025
Merged via the queue into master with commit b0ba40e Sep 16, 2025
17 checks passed
@Yarwin Yarwin deleted the bugfix/base-borrows branch September 16, 2025 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: core Core components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants