Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.0.124 #128

Merged
merged 18 commits into from
Sep 4, 2024
Merged

0.0.124 #128

merged 18 commits into from
Sep 4, 2024

Conversation

TheBlueMatt
Copy link
Collaborator

No description provided.

... this has a tendency to cause `reached the recursion limit while
auto-dereferencing` errors, and even a dumb struct copy suffices to
avoid it.
Because LDK often takes `Deref<Target=Trait>` as type parameters,
we'd implemented `Deref { type Target=Self; .. }` for the traits
defined in the bindings crate. This worked well, but because Rust
auto-`Deref`s, it can lead to spurious compilation failures due to
infinite recursion trying to deref.

In the past we've worked around this by coming up with alternative
compilation strategies when faced with `Deref` recursion, but we
don't strictly need to.

Instead, here, we introduce duplicates of all our `Trait` structs
which become the `Deref` `Target`. This way, we can `Deref` into
the `Trait` and maintain LDK compatibility, without having any
infinite `Deref` recursion issues.

One complication is traits which contain associated types to define
`Deref`s to another associated type, e.g.

trait A {
    type B;
    type C: Deref<Target = Self::B>;
}

In this case, `B` needs to be the `TraitRef` and `C` needs to be
the `Trait`. We add code specifically to detect this case.
This adds a "same item but as a reference/non-`is_owned`" helper
method for our opaque wrapper structs, as well as implements
`Deref` on them to the native type. Finally, it implements `Send`
and `Sync` on them by blindly assuming it is implemented on the
underlying struct.
Single-impl traits which are marked no-export (like
`AChannelManager` in LDK) are mapped as the singular implementing
type, rather than a full-blown trait.

Here we handle this case when writing Rust objects (generally used
in writing generic bounds).
Now that we properly handle single-impl traits as Rust types (i.e.
in generic parameters), we have to handle "converting" single-impl
traits. Here we implement this, diverging from the impl of the
underlying impl'd trait by keeping the existing type as-passed and
only ref'ing it if required.
Not idea why this was not true originally, but clearly when writing
a *reference* conversion, we should be doing so as a *reference*.
`is_clonable` will only work for non-reference types, so this check
was actually generally always false.
Not really sure why, but LTO seems to now remove used
implementations of `std` causing link failures in downstream
languages.
@TheBlueMatt TheBlueMatt force-pushed the main branch 2 times, most recently from 6c8b62f to 9a140ea Compare September 4, 2024 00:23
@TheBlueMatt TheBlueMatt merged commit 3e6f522 into lightningdevkit:main Sep 4, 2024
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant