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

Distinguish static member function on shared structs #447

Open
XiangpengHao opened this issue Nov 11, 2020 · 7 comments
Open

Distinguish static member function on shared structs #447

XiangpengHao opened this issue Nov 11, 2020 · 7 comments

Comments

@XiangpengHao
Copy link
Contributor

Regard #437, what's your opinion on how to distinguish static member function from non-static ones?
For example:

#[cxx::bridge]
mod ffi {
    struct A {
        a: f32,
    }

    extern "Rust" {
        fn foo(self: &A) -> A; 
        fn bar(self: &A) -> A;
    }
}

impl ffi::A {
    fn foo() -> A {
        A { a: 0 }
    }

    fn bar(&self) -> A {
        A { a: self.a }
    }
}

In the generated header file, foo should be a static function while bar should be non-static.

@dtolnay
Copy link
Owner

dtolnay commented Nov 11, 2020

A Rust signature should be emitted as a non-static member function if and only if it has a self argument. I want to support static member functions but those shouldn't be written with a self argument.

Maybe like this? :

#[cxx::bridge]
mod ffi {
    struct A {
        a: f32,
    }

    extern "Rust" {
        #[Self = "A"]
        fn foo() -> A; 
        fn bar(self: &A) -> A;
    }
}

@XiangpengHao
Copy link
Contributor Author

I'm also thinking about this:

#[cxx::bridge]
mod ffi {
    struct A {
        a: f32,
    }

    extern "Rust" {
        #[struct = "A"]
        fn foo() -> A; 

        #[struct = "A"]
        fn bar(&self) -> A;
    }
}
  1. struct sounds more intuitive than Self
  2. attribute on bar is also madantary, just to be explict and have a uniform api
  3. having (2), we can define bar(&self) so the function signature is consistent with rust conventions.

This will break existing code though.

@mikhailzagurskiy
Copy link

I suggest using another approach - using nested modules to declare static member functions:

struct SimpleClass {
    static void simpleMethod();
}
#[cxx::bridge]
mod ffi {

    pub mod SimpleClass {
        extern "C++" {
            fn simpleMethod();
        }
    }
}

fn main() {
    SimpleClass::simpleMethod();
}

I think, it was really what we want for static member functions

@dtolnay
Copy link
Owner

dtolnay commented Mar 17, 2021

I am on board with that!

@jsgf
Copy link

jsgf commented Sep 1, 2021

My first instinct was to try:

#[cxx_name = "SimpleClass::simpleMethod"]
fn simplething();

but this fails with

 1 │         #[cxx_name = "SimpleClass::simpleMethod"]
   │                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^ unexpected token

Is there some semantic distinction between a static method and a plain function that we need to maintain? If it's just a matter of names then it seems like cxx_name is a straightforward way to express that.

@sbrocket
Copy link
Contributor

@dtolnay I'm interested in contributing support for static member functions. There's a few different design possibilities that have been discussed, summarized below. I'd like to agree on one with you and then I'll work on putting together a pull request.

1. Designate static member functions with a new attribute

This was the first option you mentioned above.

#[cxx::bridge]
mod ffi {
    struct A {
        a: f32,
    }

    extern "Rust" {
        #[Self = "A"]
        fn static_member_of_A() -> A; 
    }
}

I think we'd want to support this attribute applied to the extern block too, similar to #[namespace], for brevity when declaring multiple static member functions.

We can bikeshed the name of the attribute - Self, struct or class, or maybe static - if we choose this one. IMO this is the clearest of these options because we can choose the name of the attribute to clearly relate to static methods.

2. Group static member functions in a nested module

This option was proposed by @mikhailzagurskiy above.

#[cxx::bridge]
mod ffi {
    struct A {
        a: f32,
    }

    mod A {
        extern "C++" {
            fn static_member_of_A() -> A; 
        }
    }
}

It's not clear to me whether they intended this syntax to only support classes that only contain static member functions - which seems too limited to be useful - or not. As it is, this syntax feels weird to me because it looks like you'd have an identifier conflict between the type and module.

3. Group static member functions in an impl block

A spin on Option 2 would be to use an impl block instead of a module. I think this is a bit clearer, but it still has the downside that there's little relation between the standard meaning of this syntax vs. its meaning to the #[cxx::bridge] macro; it's still an impl block containing declarations, not implementations. This also doesn't seem very self-explanatory...without a well-placed comment or digging through cxx docs, I don't think a reader would ever intuit that the impl block makes this a static method.

#[cxx::bridge]
mod ffi {
    struct A {
        a: f32,
    }

    impl A {
        extern "Rust" {
            fn static_member_of_A() -> A; 
        }
    }
}

bsilver8192 added a commit to bsilver8192/autocxx that referenced this issue May 22, 2022
Upcasting can change the value of a pointer in C++. This means anything
upcasting needs both the parent and child class defined, which is
awkward in user code when wrapping functions that accept ownership of a
superclass object (it requires C++ code that depends on
autocxx-generated C++ code that depends on the main C++ code, which then
needs to be depended on by another iteration of autocxx to call it from
Rust).

Ideally this would be a static function in C++ to avoid some potential
name conflicts that this approach produces (with C++ types sharing a
name in separate namespaces), but I don't think cxx supports those yet
(dtolnay/cxx#1036 and dtolnay/cxx#447).
@Nikita240
Copy link

Are there any workarounds for this at the moment? I can't figure out how to expose a constructor from Rust to C++ without wrapping it with a non-associated function.

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

No branches or pull requests

6 participants