Handle the const struct * and struct * patterns by pvdrz · Pull Request #2304 · rust-lang/rust-bindgen (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

pvdrz

Given that C keeps a different namespace for structs and aliases. The following pattern

typedef const struct foo { void *bar; } *foo;

is valid C code and produces both a struct and a pointer called foo. Given that Rust does not make this distinction, we add the _ptr prefix to the pointer type alias to avoid any name collisions.

Fixes: #2227

goffrie

if !ctx.options().c_naming {
if let Some(inner_name) = dbg!(inner_spelling
.strip_prefix("const struct ")
.and_then(|s

Choose a reason for hiding this comment

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

This textual approach seems unreliable; what if there isn't a space before the *? Possibly other cases?

Choose a reason for hiding this comment

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

This textual representation is generated by clang and it doesn't seem to be affected by adding/removing spaces from the source code. For example:

typedef const struct foo {
    char bar;
}*foo;

works just fine.

@pvdrz pvdrz changed the titleHandle the const struct * pattern. Handle the const struct * and struct * patterns

Oct 18, 2022

@pvdrz

@goffrie thanks for your review! I updated the PR with some changes if you want to give it a second look

emilio

// collisions.
if !ctx.options().c_naming {
if let Some(inner_name) = inner_spelling
.strip_prefix("const struct ")

Choose a reason for hiding this comment

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

This feels a bit hackish, relying on libclang's serialization... Can we use pointee_type() and cursor equality instead? If not I guess this is ok...

Choose a reason for hiding this comment

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

I did a commit using pointee_type but I don't get the cursor equality part, wdym?

@pvdrz

Given that C keeps a different namespace for structs and aliases. The following patterns

typedef const struct foo {
    void *inner;
} *foo;

typedef struct bar {
    void *inner;
} *bar;

are valid C code and produces both a struct and a pointer called foo and bar in different namespaces. Given that Rust does not make this distinction, we add the _ptr prefix to the pointer type aliases to avoid any name collisions.

emilio

@pvdrz

emilio

Choose a reason for hiding this comment

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

Thanks!

@pvdrz pvdrz deleted the const-struct-ptr branch

November 10, 2022 15:43

@ojeda ojeda mentioned this pull request

Nov 10, 2022

33 tasks