C API wrapper #14
C API wrapper #14
Conversation
|
Right, to be able to make the lib no_std I need lang_items and core_intrinsics features, which are nightly only and that is why Travis fails now. How do you prefer to resolve this? Leave this C API as nightly only feature and adjust Travis build accordingly, or revert back to std version (which should build on stable)? |
rustc-demangle-capi/src/lib.rs
Outdated
| pub unsafe extern fn rustc_demangle(mangled: *const u8, out: *mut u8, out_size: usize) -> i64 { | ||
| // homebrew version of strlen | ||
| let mut mangled_len = 0; | ||
| while *(mangled.add(mangled_len)) != 0 { |
There was a problem hiding this comment.
Could this use CStr::from_ptr perhaps?
There was a problem hiding this comment.
Yes, if we use std. CStr is not available in core.
rustc-demangle-capi/src/lib.rs
Outdated
| /// Returns 0 if `mangled` is not Rust symbol or if `out` buffer is too small | ||
| /// Returns 1 otherwise | ||
| #[no_mangle] | ||
| pub unsafe extern fn rustc_demangle(mangled: *const u8, out: *mut u8, out_size: usize) -> i64 { |
There was a problem hiding this comment.
I think this should return i32, right? Or maybe link to the libc crate and return c_int?
There was a problem hiding this comment.
With std I was using use std::os::raw::{c_char,c_int};. But you are right, it probably should be i32 instead of i64, or preferably c_int if I can get it somewhere easily. I suppose I could add libc as a dependency, although it feels bit heavy solution for getting few newtypes. Of course using std would solve this issue.
|
I'll do some more testing with std and see if it's weight can be reduced, and also what its impact on final linked application executable size will be. Of course I can also live with the bigger binaries, but it's bit sad to make stuff 4x bigger for relatively small benefit. |
|
You are right, with full LTO the difference between std and nostd basically disappears. These are the numbers for std version with LTO: Curiously enough, using this std version of rustc-demangle needs also libdl to be linked into the application executable. Not sure what is going on there, I though .a archives were supposed to be statically linked. But that's no biggie imo. |
|
One thing, should we have separate _hash and _nohash (final names tbd) functions instead of just arbitrarily using the "without hash" format here? |
Cargo.toml
Outdated
|
|
||
| [profile.release] | ||
| lto = true | ||
| codegen-units = 1 |
There was a problem hiding this comment.
With lto = true this and incremental = false shouldn't be necessary to specify as well
Resolves #13
I tried both std and no_std based versions; while std makes the code bit prettier, the compiled library size (predictably) blows up so I think no_std version is preferable here.
no_std:
std:
The panic_fmt etc functions are directly copy-pasted from the manual section about no_std, not sure if there is something more to be considered for libraries. Admittedly having a library that might call abort is not ideal, but on the other hand I don't think there are any intentional panics here.