Port init-image-library to rust

[benreyn]

Hello there! 👋

Im excited about the opportunity to use remacs to help me get a better lower-level understanding of Rust, C, and Emacs.

My build is still failing due to...

error[E0432]: unresolved import `crate::remacs_sys::lookup_image_type`
 --> src/image.rs:8:31
  |
8 | use crate::{lisp::LispObject, remacs_sys::lookup_image_type};
  |                               ^^^^^^^^^^^^-----------------
  |                               |           |
  |                               |           help: a similar name exists in the module: `Flookup_image_map`
  |                               no `lookup_image_type` in `remacs_sys`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
error: Could not compile `remacs`.

To learn more, run the command again with --verbose.
make[1]: *** [../rust_src/target/release/libremacs_lib.a] Error 101
make: *** [src] Error 2

Any pointers on how to expose lookup_image_type from C to Rust?

Will close #1252

[shaleh]

group the `remacs_sys items together please.
This should be:

use crate::lisp::LispObject;
use crate::remacs_sys::{valid_image_p, lookup_image_type};

[benreyn]

Even if that means moving the valid_image_p out from under the #[cfg(feature = "glyph-debug")]?

[shaleh]

If the glyph-debug remains then import it separately. Maybe that is why the two other imports were grouped together? Was that done by you or by rustformat?

[benreyn]

I grouped the imports as they are currently. lisp::LispObject was needed by both init_image_library and imagep but valid_image_p was only used by imagep, which is under #[cfg(feature = "glyph-debug")]. It felt right to only import valid_image_p in the scenarios where it is needed. Happy to expose everything though if thats the preference.

[shaleh]

use crate::lisp::LispObject;
use remacs_sys::lookup_image_type;

Then see what rustformat does with it.....

[shaleh]

lookup_image_type is declared static in the C code. static marks the function as only accessible to that C file. The name is not exposed. This is why Rust cannot see it. We often need to remove the static modifier and then copy the function definition into a header so it will be visible to Rust.

[benreyn]

@shaleh, so I got this branch to build on my local environment, and the smoke test I wrote is passing. TravisCI however is still failing. Any ideas?

[shaleh]

hmm, it appears to be dying because rustfmt is not installed for the linux without X Window System test case. That should be its own PR. Not sure why it is dying now.... Similarly, the Mac build is dying because it needs --without-makeinfo.

[benreyn]

So, neither of this Travis failures are actually related to this PR? Im happy to take a stab at those updates with some guidance.

[benreyn]

@shaleh Just following up on this. Are there any todo's for me on this PR or are there unrelated issues causing the test failures.

[shaleh]

Thanks for the reminder @benreyn. I need to look into this again.

The code in the PR looks good. I would typically suggest not using the r# style naming. But here, it is important to keep the variable name for export.

[shaleh]

Kicking the PR, let's see what happens.

[shaleh]

I am fairly certain this will need to be rebased to succeed at the unit tests.

[benreyn]

Thanks for circling back on it @shaleh. Ill get it rebased sometime today.

[benreyn]

@shaleh Just rebased and pushed. Lets see what Ci does 🤞

[benreyn]

Looks like CI is still failing here @shaleh. I havent had a chance to check it out locally and see whats going on.

[shaleh]

@benreyn you missed the 'static' keyword here:

remacs/src/image.c

Lines 9705 to 9706 in 7aa6840

	static struct image_type *
	lookup_image_type (Lisp_Object type)

This means the function is not exported and available to Rust.

[shaleh]

This function should not have the static keyword. Sorry if I was not clear. It is important that neither this location nor the earlier reference to the function in the same file has static.

[benreyn]

Thanks for catching that @shaleh. Im still learning the ins and outs of how Remacs is pieced together.

[shaleh]

No worries @benreyn. This is an odd project because of the mix of C and Rust.

'static' means a function is not exported from the C file it is declared in. For variables, they will automatically be set to zero. Not initialized -- zero.

There is also a requirement in C that a function be declared before it is used. Which is why the function you were touching had two instances of it. One was a forward declaration on one line and the later the full definition.