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

autocfg assumes there is no std #2431

Closed
youknowone opened this issue Jul 24, 2022 · 4 comments · Fixed by #2436
Closed

autocfg assumes there is no std #2431

youknowone opened this issue Jul 24, 2022 · 4 comments · Fixed by #2436
Labels
A-cargo Area: affects the cargo wrapper (cargo miri) C-bug Category: This is a bug.

Comments

@youknowone
Copy link

youknowone commented Jul 24, 2022

This code was running ok with previous version of miri, but not today version.

// cargo add indexmap
fn main() {
    let _ = indexmap::IndexMap::<u32, String>::new();
}

error:

error[E0107]: this struct takes 3 generic arguments but 2 generic arguments were supplied
  --> src/main.rs:2:23
   |
2  |     let _ = indexmap::IndexMap::<u32, String>::new();
   |                       ^^^^^^^^   ---  ------ supplied 2 generic arguments
   |                       |
   |                       expected 3 generic arguments
   |
note: struct defined here, with 3 generic parameters: `K`, `V`, `S`
  --> /home/youknowone/.cargo/registry/src/github.com-1ecc6299db9ec823/indexmap-1.9.1/src/map.rs:76:12
   |
76 | pub struct IndexMap<K, V, S> {
   |            ^^^^^^^^ -  -  -
help: add missing generic argument
   |
2  |     let _ = indexmap::IndexMap::<u32, String, S>::new();
   |                                             +++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0107`.
error: test failed, to rerun pass '--bin miritest'

In the indexmap code, I found it is hitting #[cfg(not(has_std))]

#[cfg(has_std)]
pub struct IndexMap<K, V, S = RandomState> {
    pub(crate) core: IndexMapCore<K, V>,
    hash_builder: S,
}
#[cfg(not(has_std))]
pub struct IndexMap<K, V, S> {
    pub(crate) core: IndexMapCore<K, V>,
    hash_builder: S,
}
@RalfJung
Copy link
Member

RalfJung commented Jul 24, 2022

So looks like this code here now decided that we don't have std. What exactly does autocfg::new().emit_sysroot_crate("std") do?

My hunch would be that it has to do with this recent change:

miri/cargo-miri/bin.rs

Lines 757 to 766 in 8fdb720

// We'd prefer to just clear this env var, but cargo does not always honor `RUSTC_WRAPPER`
// (https://github.com/rust-lang/cargo/issues/10885). There is no good way to single out these invocations;
// some build scripts use the RUSTC env var as well. So we set it directly to the `miri` driver and
// hope that all they do is ask for the version number -- things could quickly go downhill from here.
// In `main`, we need the value of `RUSTC` to distinguish RUSTC_WRAPPER invocations from rustdoc
// or TARGET_RUNNER invocations, so we canonicalize it here to make it exceedingly unlikely that
// there would be a collision with other invocations of cargo-miri (as rustdoc or as runner).
// We explicitly do this even if RUSTC_STAGE is set, since for these builds we do *not* want the
// bootstrap `rustc` thing in our way! Instead, we have MIRI_HOST_SYSROOT to use for host builds.
cmd.env("RUSTC", &fs::canonicalize(find_miri()).unwrap());

@RalfJung RalfJung added C-bug Category: This is a bug. A-cargo Area: affects the cargo wrapper (cargo miri) labels Jul 24, 2022
@RalfJung RalfJung changed the title assuming no_std even without MIRI_NO_STD autocfg assumes there is no std Jul 24, 2022
@RalfJung
Copy link
Member

RalfJung commented Jul 24, 2022

Oh lol, it runs rustc on some code and checks the results. Yeah that has no realistic chance of working. It worked mostly by chance before...

We'd have to somehow ensure that MIRI_BE_RUSTC=host is set when build scripts are run. And even then the test is kinda meaningless since it would not be run for the target we are actually building for. Autocfg also passes on the TARGET it seems, but for the target we simply don't have a sysroot it could use. So this was probably already broken before in cross-compilation mode.

Though OTOH all it really needs is a check build and that should be possible...

@RalfJung
Copy link
Member

Ah here we go, I have an error message:

  error: miri can only run programs that have a main function

So looks like we need to make Miri parse the --emit flag (or access whatever parsed form of it that rustc has) and skip interpretation if this is not actually supposed to produce a binary.

@youknowone
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo Area: affects the cargo wrapper (cargo miri) C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants