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

Tons of unexpected_cfgs failures in portable-simd and backtrace doctests #124740

Closed
RalfJung opened this issue May 5, 2024 · 15 comments · Fixed by rust-lang/miri-test-libstd#66
Closed

Comments

@RalfJung
Copy link
Member

RalfJung commented May 5, 2024

Running std doctests in Miri results in tons of errors since today's nightly:

  0.000010   ---- library/core/src/../../portable-simd/crates/core_simd/src/vector.rs - core_simd::vector::Simd<T,N>::store_select (line 640) stdout ----
  0.000008   error: unexpected `cfg` condition value: `as_crate`
  0.000010    --> library/core/src/../../portable-simd/crates/core_simd/src/vector.rs:642:7
  0.000008     |
  0.000011   5 | #[cfg(feature = "as_crate")] use core_simd::simd;
  0.000010     |       ^^^^^^^^^^^^^^^^^^^^
  0.000011     |
  0.000013     = note: expected values for `feature` are: `debug_refcell`, `panic_immediate_abort`
  0.000007     = help: consider adding `as_crate` as a feature in `Cargo.toml`
  0.000011     = note: see <https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg> for more information about checking conditional configuration
  0.000007   note: the lint level is defined here
  0.000010    --> library/core/src/../../portable-simd/crates/core_simd/src/vector.rs:638:9
  0.000008     |
  0.000010   1 | #![deny(warnings)]
  0.000008     |         ^^^^^^^^
  0.000010     = note: `#[deny(unexpected_cfgs)]` implied by `#[deny(warnings)]`
  0.000010   
  0.000010   error: unexpected `cfg` condition value: `as_crate`
  0.000008    --> library/core/src/../../portable-simd/crates/core_simd/src/vector.rs:643:11
  0.000010     |
  0.000008   6 | #[cfg(not(feature = "as_crate"))] use core::simd;
  0.000010     |           ^^^^^^^^^^^^^^^^^^^^
  0.000008     |
  0.000010     = note: expected values for `feature` are: `debug_refcell`, `panic_immediate_abort`
  0.000008     = help: consider adding `as_crate` as a feature in `Cargo.toml`
  0.000011     = note: see <https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg> for more information about checking conditional configuration
  0.000009   
  0.000010   error: aborting due to 2 previous errors
  0.000008   
  0.000010   Couldn't compile the test.

Interestingly, the in-tree test suite is working, so there's likely some special bootstrap magic somewhere that out-of-tree tests do not benefit from?

cc @Urgau

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 5, 2024
@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

These files are from portable-simd, which means they are sometimes their own crate and sometimes a module imported into core. There is indeed no as_crate feature in core, so no idea how these tests are passing in our CI.

Do we inject a -A unexpected_cfgs somewhere for doctests, or something like that?

@RalfJung RalfJung changed the title Tons of unexpected_cfgs failures in std doctests Tons of unexpected_cfgs failures in portable-simd doctests May 5, 2024
@RalfJung RalfJung changed the title Tons of unexpected_cfgs failures in portable-simd doctests Tons of unexpected_cfgs failures in portable-simd and backtrace doctests May 5, 2024
@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

There's also one in std, from the backtrace module, which similarly is also sometimes a crate and sometimes a module:

  0.000008   ---- library/std/src/../../backtrace/src/lib.rs - backtrace_rs (line 20) stdout ----
  0.000007   error: unexpected `cfg` condition value: `std`
  0.000007    --> library/std/src/../../backtrace/src/lib.rs:23:7
  0.000008     |
  0.000007   5 | #[cfg(feature = "std")] {
  0.000008     |       ^^^^^^^^^^^^^^^
  0.000007     |
  0.000008     = note: expected values for `feature` are: `addr2line`, `backtrace`, `compiler-builtins-c`, `compiler-builtins-mangled-names`, `compiler-builtins-mem`, `compiler-builtins-no-asm`, `compiler-builtins-weak-intrinsics`, `llvm-libunwind`, `miniz_oxide`, `object`, `panic-unwind`, `panic_immediate_abort`, `panic_unwind`, `profiler`, `profiler_builtins`, `std_detect_dlsym_getauxval`, `std_detect_env_override`, `std_detect_file_io`, `system-llvm-libunwind`
  0.000008     = help: consider adding `std` as a feature in `Cargo.toml`
  0.000007     = note: see <https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg> for more information about checking conditional configuration
  0.000007   note: the lint level is defined here
  0.000007    --> library/std/src/../../backtrace/src/lib.rs:19:9
  0.000007     |
  0.000007   1 | #![deny(warnings)]
  0.000007     |         ^^^^^^^^
  0.000007     = note: `#[deny(unexpected_cfgs)]` implied by `#[deny(warnings)]`
  0.000008   
  0.000007   error: aborting due to 1 previous error
  0.000007   
  0.000007   Couldn't compile the test.

@workingjubilee
Copy link
Member

Huh.

@Kobzol
Copy link
Contributor

Kobzol commented May 5, 2024

It's quite possible that this is caused by the CI mess. One of the insta-merged PRs was a miri sync.

@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

It's possible but seems unlikely to me. I guess we'll have to see.

@Urgau
Copy link
Member

Urgau commented May 5, 2024

Interestingly, the in-tree test suite is working, so there's likely some special bootstrap magic somewhere that out-of-tree tests do not benefit from?

Yes, there is,

// Enable compile-time checking of `cfg` names, values and Cargo `features`.
//
// Note: `std`, `alloc` and `core` imports some dependencies by #[path] (like
// backtrace, core_simd, std_float, ...), those dependencies have their own
// features but cargo isn't involved in the #[path] process and so cannot pass the
// complete list of features, so for that reason we don't enable checking of
// features for std crates.
cargo.arg("-Zcheck-cfg");
if mode == Mode::Std {
rustflags.arg("--check-cfg=cfg(feature,values(any()))");
}

I would suggest doing the same and adding as part of the build --check-cfg=cfg(feature,values(any())), possibly in RUSTFLAGS or somewhere else.

@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

That seems to help, thanks. :)

@Urgau
Copy link
Member

Urgau commented May 5, 2024

Another thing we could do now that check-cfg is stable, is adding this exception directly in the build.rs of core, alloc and std with the Cargo instruction cargo::rustc-check-cfg.

@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

If you end up doing that, please ping me so I can revert rust-lang/miri-test-libstd#66.

@Urgau
Copy link
Member

Urgau commented May 5, 2024

@RalfJung I don't think allowing all values for the feature cfg is going to be enough. There are many other custom cfgs used in core/alloc/std,

(Some(Mode::Std), "stdarch_intel_sde", None),
(Some(Mode::Std), "no_fp_fmt_parse", None),
(Some(Mode::Std), "no_global_oom_handling", None),
(Some(Mode::Std), "no_rc", None),
(Some(Mode::Std), "no_sync", None),
(Some(Mode::Std), "netbsd10", None),
(Some(Mode::Std), "backtrace_in_libstd", None),
/* Extra values not defined in the built-in targets yet, but used in std */
(Some(Mode::Std), "target_env", Some(&["libnx", "p2"])),
(Some(Mode::Std), "target_os", Some(&["visionos"])),
(Some(Mode::Std), "target_arch", Some(&["arm64ec", "spirv", "nvptx", "xtensa"])),

In fact I can see them in the latest CI run under the "Setup environment". If you want to completely silence them, allowing the lint (-Aunexpected_cfgs in RUSTFLAGS) is probably (in the short term) the best course of action.

@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

This was enough though to make the tests pass again... 🤷

@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

Oh, they are shown as warning, not hard errors.

Sure, I can also allow the lint entirely.

@RalfJung
Copy link
Member Author

RalfJung commented May 5, 2024

Actually, those warnings come from the sysroot build, not the local test build. So this will need a patch in https://github.com/RalfJung/rustc-build-sysroot/ . Also, -Aunexpected_cfgs does not help for the doctests, for some reason.

(FWIW, debugging rustc invocations became a lot harder when they all grew to more than twice their size due to all these --check-cfg flags. If there's any way to hide these flags e.g. in argument files or so, that would be great...)

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 5, 2024
@Urgau
Copy link
Member

Urgau commented May 22, 2024

If you end up doing that, please ping me so I can revert rust-lang/miri-test-libstd#66.

@RalfJung We did add the corresponding --check-cfg args to std, alloc and core in #125296

(FWIW, debugging rustc invocations became a lot harder when they all grew to more than twice their size due to all these --check-cfg flags. If there's any way to hide these flags e.g. in argument files or so, that would be great...)

We are tracking this in rust-lang/cargo#13941

@RalfJung
Copy link
Member Author

We did add the corresponding --check-cfg args to std, alloc and core in #125296

Awesome, that seems to be enough for miri-test-libstd as well. :)

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 a pull request may close this issue.

6 participants