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

Handle fully qualified paths and use aliases #7

Open
eqrion opened this issue May 9, 2017 · 14 comments
Open

Handle fully qualified paths and use aliases #7

eqrion opened this issue May 9, 2017 · 14 comments

Comments

@eqrion
Copy link
Collaborator

eqrion commented May 9, 2017

Each Rust Item has a path that includes the crate and mods needed to get to it. Currently cbindgen just ignores all segments of a path except the last one, which is the name of the item.

This works as long as there are no duplicates in different modules, but isn't correct and will probably cause someone an issue someday.

@eqrion
Copy link
Collaborator Author

eqrion commented Oct 10, 2017

My comments earlier were wrong.

cbindgen used to refuse to parse paths that had multiple segments. This wasn't great so I fixed it in 0155728.

@AdamRzepka
Copy link

AdamRzepka commented May 21, 2018

Any updates on this? I've got multiple struct Configs in different modules and want to export them all.

  • What's the best workaround currently?
  • Do you have any specific plans on implementing this?
  • If it's not very hard, I could try to implement the feature (equipped with some instructions).

@eqrion
Copy link
Collaborator Author

eqrion commented May 21, 2018

Unfortunately, I'm not sure there is a workaround yet besides renaming the types. My first thought would have been to use the structs behind type aliases with different names, but that won't work if you want to expose them..

I don't currently have specific plans on implementing this. Unfortunately this is a tricky issue with a couple different options.

  1. Implement a custom path resolution algorithm on top of syn
  2. Use rls-analysis to hook into the work going on there for goto definition
  3. Write a rustc integration to get this data

I've been told by @gankro that option 1 is not going to be fun as this is a very complicated and not well understood part of rustc and has evolved. I think this part of the rust reference is relevant.

Using rls-analysis could be viable, but needs investigation. My concern with using rls would be if it is using a lossy/mostly correct goto definition feature and not focused on strict correctness.

I have no idea of how feasible option 3 would be.

@eqrion eqrion changed the title Handle item paths better Handle fully qualified paths and use aliases Aug 21, 2018
@regexident
Copy link
Contributor

Issue: Import renames à la use Foo as Bar; are not resolved properly.

A minimalistic crate like this …

// lib.rs

pub struct Foo { pub field: bool }

use Foo as Bar;

#[no_mangle]
pub unsafe extern "C" fn placebo(_bar: &Bar) {}

… generates a header like this …

// lib.h

#include <cstdint>
#include <cstdlib>

extern "C" {

void placebo(const Bar *_bar);

} // extern "C"

… emitting the following warning:

WARN: Can't find Bar. This usually means that this type was incompatible or not found.

… while it should generate something like this instead:

// lib.h

#include <cstdint>
#include <cstdlib>

struct Foo;

extern "C" {

void placebo(const Foo *_bar);

} // extern "C"

What's wrong:

  1. cbindgen doesn't recognize that Foo is used in an exported fn thus does not generate it.
  2. _bar: &Bar is not properly resolved to _bar: &Foo in the function argument.

(Cross-posted from #200, which was closed in favor of this original issue)

@regexident
Copy link
Contributor

Issue: Incorrect type resolving for differently scoped types of same name.

A minimalistic crate like this …

// lib.rs

pub mod foo {
    pub struct Foobar<T> { pub field: T }
}

pub mod bar {
    pub struct Foobar { pub field: u8 }

    #[no_mangle]
    pub unsafe extern "C" fn placebo(_foobar: &Foobar) {}
}

… generates a header like this …

// lib.h

#include <cstdint>
#include <cstdlib>

template<typename T>
struct Foobar;

extern "C" {

void placebo(const Foobar *_foobar);

} // extern "C"

… while it should generate something like this instead:

#include <cstdint>
#include <cstdlib>

struct Foobar;

extern "C" {

void placebo(const Foobar *_foobar);

} // extern "C"

What's wrong:

  1. Foobar<T> is not used in any exported fn and hence shouldn't be generated.
  2. Foobar however is used in an exported fn and hence should be generated.

Observations:

  1. Changing the order of mod foo and mod bar makes it behave correctly.
  2. Adding #[repr(C)] to bar::Foobar makes it behave correctly, strangely enough.
  3. The <T> doesn't seem to be strictly necessary for triggering this bug, but makes things easier to spot in the generated header.

(Cross-posted from #199, which was closed in favor of this original issue)

@regexident
Copy link
Contributor

I just pushed a PR (#201) that aims to move us a bit closer towards a direction of fixing this.

@ysangkok
Copy link

@saschanaz
Copy link

Did #233 fix this?

@emilio
Copy link
Collaborator

emilio commented Sep 28, 2020

Not really, though it added the infrastructure to potentially do it.

@UebelAndre
Copy link

Any updates here? Would love to see this implemented 🙏

@UebelAndre
Copy link

Not really, though it added the infrastructure to potentially do it.

Can you elaborate on this a bit? I know nothing about the internals of this project but would really like to see this feature implemented 😅

@regexident
Copy link
Contributor

@UebelAndre A look at the diff shows that the PR replaces the use of bare strings with

pub struct Path {
    name: String,
}

As such the PR has improved type-safety, but has not actually made the jump from "name-based" to "full qualified path-based", unlike what the name might suggest.

@aidanhs
Copy link
Contributor

aidanhs commented Nov 10, 2023

My workaround for the "Issue: Import renames à la use Foo as Bar; are not resolved properly." issue is a macro for opaque newtype wrapping.

macro_rules! opaque {
    ($exported:ident, $internal:ty) => {
        pub struct $exported($internal);
        impl From<$internal> for $exported {
            fn from(v: $internal) -> Self {
                Self(v)
            }
        }
        impl From<$exported> for $internal {
            fn from(value: $exported) -> Self {
                value.0
            }
        }
    };
}

opaque!(LoadConfig, load::Config);
opaque!(EmitConfig, emit::Config);

(where the parse section of the cbindgen config carefully doesn't include either of the load or emit crates)

This only works in my specific case because these types are opaque (they're always provided behind a pointer). I don't know a workaround for non-opaque types.

@lu-zero
Copy link
Contributor

lu-zero commented Aug 11, 2024

hyper has plenty of structs named Sender across its codebase if you want yet another case study.

src/body/incoming.rs:pub(crate) struct Sender {
src/client/dispatch.rs:pub(crate) struct Sender<T, U> {
src/common/watch.rs:pub(crate) struct Sender {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants