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

Use compiler_builtins for mem{cmp,cpy,move,set} #9

Merged
merged 1 commit into from Sep 13, 2021

Conversation

cole-miller
Copy link
Contributor

As proposed in #5. This is just a straightforward code replacement, I'm not sure what else if anything needs to be done. I've left bcmp as-is since I can't see any reason to use compiler_builtins there.

c-scape/src/c.rs Outdated Show resolved Hide resolved
@cole-miller
Copy link
Contributor Author

cole-miller commented Sep 13, 2021

I've turned on name mangling for compiler_builtins. Now getting this error:

error[E0658]: use of unstable library feature 'rustc_private': this crate is being loaded from the sysroot, an unstable location; did you mean to load this crate from crates.io via `Cargo.toml` instead?
   --> c-scape/src/c.rs:896:5
    |
896 |     compiler_builtins::mem::memcmp(a as *const u8, b as *const u8, len)
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #27812 <https://github.com/rust-lang/rust/issues/27812> for more information
    = help: add `#![feature(rustc_private)]` to the crate attributes to enable

I'm not sure how to proceed since the dependency is in Cargo.toml already and #![feature(rustc_private)] seems like a bad idea. I thought it might be caused by the compiler-builtins feature of compiler_builtins, but disabling that doesn't change anything.

@cole-miller
Copy link
Contributor Author

cole-miller commented Sep 13, 2021

Followed @bjorn3's suggestion and just declared these in an extern "C" block instead of implementing wrappers. memcmp and memmove (edit: and bcmp) are commented out for now to avoid unused warnings, but presumably we'll want them in there once the c module is exposed.

@sunfishcode
Copy link
Owner

I somehow didn't notice that compiler-builtins directly provides no_mangle extern "C" definitions, but it makes sense!

@sunfishcode sunfishcode merged commit 2811199 into sunfishcode:main Sep 13, 2021
@cole-miller cole-miller deleted the compiler-builtins-mem branch September 13, 2021 12:12
@sunfishcode sunfishcode mentioned this pull request Sep 15, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants