Module registration currently doesn't work

[extendr]

Module registration currently doesn't work

[andy-thomason]

Just to fill everyone in:

The R_init_ function is the entrypoint called by R when the DLL loads.

The wrap__ functions are the C language wrappers for the .Call interface.

The init__ functions are called from the R_init_ function provided an #[extendr_module] section
is created.

The init__ functions may have stopped working when the registration function was updated. I could never get the symbol-style ".Call" bindings to work despite the advertised interface.

[clauswilke]

It's more complex than removing the lib from the line I highlighted above. Just doing that doesn't fix the routine registration. As far as I can tell, there's also a mixup between function names starting with wrap__ and those starting with init__. The code looks like it's registering functions with init__, but we actually need to call functions starting with wrap__:

extendr/extendr-macros/src/lib.rs

Lines 349 to 350 in 4177272

	let fninitnames = fnnames.iter().map(|id| format_ident!("{}{}", INIT_PREFIX, id));
	let implinitnames = implnames.iter().map(|id| format_ident!("{}{}", INIT_PREFIX, id));

However, just changing init__ to wrap__ doesn't work, in fact it makes it worse. Then things break on all platforms. So there's something I don't understand here.

@hobofan You've looked at this code most recently. Do you feel comfortable debugging this? You can use the latest version of the helloextendr package as a starting point to test. Unfortunately it's most obvious on Windows if things don't work, but you can test that by making a PR and seeing how it does on GH Actions.

You'll want to comment out the manual registration of the routines:
https://github.com/extendr/helloextendr/blob/16a65c8801f25e354e9eba4ffed631f2fca47c70/src/entrypoint.c#L12-L27

And uncomment the module registration:
https://github.com/extendr/helloextendr/blob/16a65c8801f25e354e9eba4ffed631f2fca47c70/src/rust/src/lib.rs#L8-L14

[hobofan]

@clauswilke I can take a look at it (as it also seems to be a good opportunity to get to know the registration mechanism more), but it might be ~10 days until I have the time to do so. So if someone else is eager, feel free to go ahead!

[andy-thomason]

Take for example the hello example:

use extendr_api::*;

#[extendr]
fn hello() -> &'static str {
    "hello"
}

// Macro to generate exports
extendr_module! {
    mod hello;
    fn hello;
}

If we run cargo expand on this we get:

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::v1::*;
#[macro_use]
extern crate std;
use extendr_api::*;
fn hello() -> &'static str {
    "hello"
}
#[no_mangle]
#[allow(non_snake_case)]
pub extern "C" fn wrap__hello() -> extendr_api::SEXP {
    unsafe {
        use extendr_api::FromRobj;
        extendr_api::Robj::from(hello()).get()
    }
}
#[allow(non_snake_case)]
fn init__hello(info: *mut extendr_api::DllInfo, call_methods: &mut Vec<extendr_api::CallMethod>) {
    call_methods.push(extendr_api::CallMethod {
        call_symbol: std::ffi::CString::new("wrap__hello").unwrap(),
        func_ptr: wrap__hello as *const u8,
        num_args: 0i32,
    })
}
#[no_mangle]
#[allow(non_snake_case)]
pub extern "C" fn R_init_libhello(info: *mut extendr_api::DllInfo) {
    let mut call_methods = Vec::new();
    init__hello(info, &mut call_methods);
    unsafe { extendr_api::register_call_methods(info, call_methods.as_ref()) };
}

The init function provides the metadata required to register the function.

[clauswilke]

Thanks for the clarification. I think it'd be a good idea to add a lot of this info as comments to the code, as macro code is incredibly difficult to figure out by just reading it.

[andy-thomason]

I'll put some examples of the above in the code...

[andy-thomason]

It gets more complex when you start exporting interfaces!

I don't know why we are exporting R_init_lib[name]. The "documentation" know as the R source code
uses R_init_[dllname].

https://github.com/wch/r-source/blob/trunk/src/main/Rdynload.c#L598-L612

[clauswilke]

Yes, R_init_lib[name] is definitely wrong. The rest looks correct. It works when I do the equivalent in C, or at least I don't see the difference yet:
https://github.com/extendr/helloextendr/blob/main/src/entrypoint.c

Note: I'm using "helloextendr" as the name, but that's not the issue. I named the module accordingly:
https://github.com/extendr/helloextendr/blob/main/src/rust/src/lib.rs

[clauswilke]

I'm looking through register_call_methods() and I'm wondering whether it generates the correct data structures that R_registerRoutines() understands:

extendr/extendr-api/src/lib.rs

Lines 100 to 125 in 2a19999

	pub unsafe fn register_call_methods(info: *mut libR_sys::DllInfo, methods: &[CallMethod]) {
	let mut rmethods: Vec<_> = methods
	.iter()
	.map(|m| libR_sys::R_CallMethodDef {
	name: m.call_symbol.as_ptr(),
	fun: Some(std::mem::transmute(m.func_ptr)),
	numArgs: m.num_args,
	})
	.collect();
	rmethods.push(libR_sys::R_CallMethodDef {
	name: std::ptr::null(),
	fun: None,
	numArgs: 0,
	});
	libR_sys::R_registerRoutines(
	info,
	std::ptr::null(),
	rmethods.as_ptr(),
	std::ptr::null(),
	std::ptr::null(),
	);
	libR_sys::R_useDynamicSymbols(info, 0);
	libR_sys::R_forceSymbols(info, 1);
	}

First, fun is of type DL_FUNC = ::std::option::Option<unsafe extern "C" fn() -> *mut ::std::os::raw::c_void>;. Is this guaranteed to be just a single pointer that is set to 0 for the None option? Second, rmethods is a Rust Vec<> that is converted into a pointer via rmethods.as_ptr(). Is this guaranteed to be laid out just like a C array?

[clauswilke]

@andy-thomason @hobofan I have committed an R package that serves as integration test and is automatically built and checked on GH Actions. When you make changes to the registration routine, you can modify the sources accordingly and see if the package still builds. The C/Rust source code is here:
https://github.com/extendr/extendr/tree/master/tests/extendrtests/src

[andy-thomason]

The module code did work in the past but has been neglected for a while. The classes example was the best example. Now we are using makevars the name of the DLL has changed, breaking the entry point. One method I used in the past was to build r from source and add logging to find out why things did not work. I was planning to do this with the symbol based registration.

…

On Fri, 18 Dec 2020, 22:11 Claus Wilke, ***@***.***> wrote: @andy-thomason <https://github.com/andy-thomason> @hobofan <https://github.com/hobofan> I have committed an R package that serves as integration test and is automatically built and checked on GH Actions. When you make changes to the registration routine, you can modify the sources accordingly and see if the package still builds. The C/Rust source code is here: https://github.com/extendr/extendr/tree/master/tests/extendrtests/src — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#75 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAL36XE6T2QX4MGO3X3B2RDSVPHP3ANCNFSM4U5OINBQ> .

[andy-thomason]

Don't let me stop you prodding it with a stick, @hobofan. Logging in the functions called by the macro code would be good, too.

…

On Fri, 18 Dec 2020, 22:22 Andy Thomason, ***@***.***> wrote: The module code did work in the past but has been neglected for a while. The classes example was the best example. Now we are using makevars the name of the DLL has changed, breaking the entry point. One method I used in the past was to build r from source and add logging to find out why things did not work. I was planning to do this with the symbol based registration. On Fri, 18 Dec 2020, 22:11 Claus Wilke, ***@***.***> wrote: > @andy-thomason <https://github.com/andy-thomason> @hobofan > <https://github.com/hobofan> I have committed an R package that serves > as integration test and is automatically built and checked on GH Actions. > When you make changes to the registration routine, you can modify the > sources accordingly and see if the package still builds. The C/Rust source > code is here: > https://github.com/extendr/extendr/tree/master/tests/extendrtests/src > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#75 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAL36XE6T2QX4MGO3X3B2RDSVPHP3ANCNFSM4U5OINBQ> > . >