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

feat(ir) Add ability to ignore specific struct fields #579

Closed
wants to merge 2 commits into from

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Sep 24, 2020

With this PR, it is now possible to add /// cbindgen:ignore on specific struct fields to ignore them.

Thus:

pub struct OneFieldIgnored {
    x: i32,
    /// cbindgen:ignore
    y: i64,
    z: i32,
}

is bound to:

typedef struct {
    int32_t x;
    int32_t z;
} OneFieldIgnored;

It is useful for a project I'm working on, but it can also be helpful for issue like #149.

Thoughts?

It is now possible to add `/// cbindgen:ignore` on specific struct
fields to ignore them.

Thus:

```
pub struct OneFieldIgnored {
    x: i32,
    /// cbindgen:ignore
    y: i64,
    z: i32,
}
```

is bound to:

```
typedef struct {
    int32_t x;
    int32_t z;
} OneFieldIgnored;
```
@emilio
Copy link
Collaborator

emilio commented Sep 25, 2020

How is this useful? OneFieldIgnored would have different layout in C than in rust, causing at best just confusion (z from C would be y), and at worst stack overruns and segfaults.

@Hywan
Copy link
Contributor Author

Hywan commented Sep 25, 2020

Imagine you have a type typedef struct S S; that you have to implement in Rust. It is created by a s_new() function, and deleted by s_delete(s *s) function.

But maybe, in Rust, you need to attach some fields to it, like struct S { inner: some_crate::S }. You don't want cbindgen to create a binding for inner, whether it is a public or a private field.

I don't have a usecase for multiple fields actually, just a struct with one field.

From the point of view of C, S is an opaque type in this case.

@emilio
Copy link
Collaborator

emilio commented Sep 25, 2020

Ok, that does make sense to me. Then I think this is the wrong thing to do. An opaque annotation for which we don't generate any fields or such would be slightly better. Cbindgen already has the concept of opaque types.

@emilio
Copy link
Collaborator

emilio commented Sep 25, 2020

Though at that point why marking the struct as #[repr(C)] to begin with? cbindgen will treat it as opaque if you don't, and that will allow you to get rust layout optimizations and so on in your struct, so it seems even better?

@Hywan
Copy link
Contributor Author

Hywan commented Sep 28, 2020

I agree it makes no sense. Sorry!

@Hywan Hywan closed this Sep 28, 2020
@Hywan
Copy link
Contributor Author

Hywan commented Sep 29, 2020

@emilio

An opaque annotation for which we don't generate any fields or such would be slightly better. Cbindgen already has the concept of opaque types.

How do you generate an opaque type with cbindgen?

@emilio
Copy link
Collaborator

emilio commented Sep 29, 2020

Generally, by making it not #[repr(C)].

@petrochenkov
Copy link
Contributor

I wanted to use this for a union like this:

union U {
    a: [u64; 2],
    b: u128,
}

cbindgen cannot produce anything useful for u128, so it's ok to make this field unavailable from the C side while keeping all the other fields available.
I guess I'll have use the "typical cbindgen workaround" instead.

/// cbindgen:ignore
union U {
    a: [u64; 2],
    b: u128,
}

union U_FFI {
    a: [u64; 2],
}
[export]
include = ["U_FFI"]
[export.rename]
U = "U_FFI"

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.

3 participants