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

Macros exported by compiler_builtins should not be in the prelude #113533

Closed
Amanieu opened this issue Jul 10, 2023 · 2 comments · Fixed by #113557
Closed

Macros exported by compiler_builtins should not be in the prelude #113533

Amanieu opened this issue Jul 10, 2023 · 2 comments · Fixed by #113557
Labels
T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@Amanieu
Copy link
Member

Amanieu commented Jul 10, 2023

compiler_builtins exports some macros for use by its tests, but rustc seems to make these available to any code as if they were part of the prelude. While compiler_builtins should be fixed to not export these, it is also strange that the prelude includes these macros.

// Compile with: rustc foo.rs --target aarch64-unknown-linux-gnu --crate-type lib
#![no_std]

foreach_cas16!();
error: unexpected end of macro invocation
 --> src/lib.rs:3:1
  |
3 | foreach_cas16!();
  | ^^^^^^^^^^^^^^^^ missing tokens in macro arguments
  |
note: while trying to match meta-variable `$r#macro:path`
 --> /home/amanieu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/obj/build/x86_64-unknown-linux-gnu/stage1-std/aarch64-unknown-linux-gnu/release/build/compiler_builtins-0862f16702efa101/out/outlined_atomics.rs:103:51

error: aborting due to previous error
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 10, 2023
Amanieu added a commit to Amanieu/rustc-builtins that referenced this issue Jul 10, 2023
This was causing build failures on AArch64 due to name resolution
ambiguity.
Amanieu added a commit to Amanieu/rustc-builtins that referenced this issue Jul 10, 2023
This was causing build failures on AArch64 due to name resolution
ambiguity.
@saethlin saethlin added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 10, 2023
@lukas-code
Copy link
Member

The prelude for no_std crates currently looks like this:

#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use core::prelude::rust_2021::*;
#[macro_use]
extern crate core;
#[macro_use]
extern crate compiler_builtins;

The #[macro_use] on compiler_builtins comes from here:

thin_vec![cx.attr_word(sym::macro_use, span)],

@Amanieu
Copy link
Member Author

Amanieu commented Jul 10, 2023

In #113557 I changed this to:

// No #[macro_use]
extern crate compiler_builtins as  _;

The API of compiler-builtins is not stable and should not be exposed in the prelude. It's only needed there because compiler_builtins is required at link-time.

Amanieu added a commit to Amanieu/rust that referenced this issue Jul 12, 2023
This is an alternative to rust-lang#113557 which removes `compiler_builtins` from
the prelude and injects it during crate resolution instead.

Fixes rust-lang#113533
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 16, 2023
…chenkov

Hide `compiler_builtins` in the prelude

This crate is a private implementation detail. We only need to insert it into the crate graph for linking and should not expose any of its public API.

Fixes rust-lang#113533
@bors bors closed this as completed in 07f855d Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
4 participants