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

Get rid of a warning #634

Merged
merged 1 commit into from
Jul 6, 2024
Merged

Get rid of a warning #634

merged 1 commit into from
Jul 6, 2024

Conversation

tea
Copy link
Contributor

@tea tea commented Jun 25, 2024

I get this error when building the crate using fresh rust:

error: `extern crate` is not idiomatic in the new edition
   --> /home/aturkin/redox/compiler-builtins/build.rs:225:5
    |
225 |     extern crate cc;
    |     ^^^^^^^^^^^^^^^^
    |
    = note: `-D unused-extern-crates` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(unused_extern_crates)]`
help: convert it to a `use`
    |
225 |     use cc;
    |     ~~~

Indeed 2018 edition documentation says extern crate should not be used anymore "in 99% of circumstances". So this PR follows the advice.

@Amanieu
Copy link
Member

Amanieu commented Jun 25, 2024

The use isn't even necessary: crate names are always automatically imported.

@tea tea force-pushed the extern_crate branch 2 times, most recently from edbb831 to ec51173 Compare June 26, 2024 04:50
@tea
Copy link
Contributor Author

tea commented Jun 26, 2024

The use isn't even necessary: crate names are always automatically imported.

Removed now. CI is failing due to some seemingly unrelated stuff.

@tgross35
Copy link
Contributor

tgross35 commented Jun 26, 2024

Could you try setting edition = 2021 in crates/panic-handler/Cargo.toml? That is an interesting failure

@tgross35
Copy link
Contributor

You can probably change to edition = 2021 in the root Cargo.toml too, I don't think we have any reason to stay on 2018

@tea
Copy link
Contributor Author

tea commented Jun 26, 2024

Could you try setting edition = 2021 in crates/panic-handler/Cargo.toml? That is an interesting failure

No dice.

@tgross35
Copy link
Contributor

tgross35 commented Jun 26, 2024

Oh nevermind, this is just a new lint in the latest nightly rust-lang/rust@c4c7859. Looks like lines 135 and 263 try to use define_rust_probestack but the macro cfged out on the relevant platforms.

To match the current behavior you could add another define_rust_probestack macro that returns nothing and has cfg to only be available on the platforms that are missing it (really a better job for cfg_if but we don't have that).

Even if that matches the current behavior though, it might not be the intended behavior (Amanieu will know)

edit: doesn't look like any cfg patterns are missing after all

@tea
Copy link
Contributor Author

tea commented Jun 26, 2024

Oh nevermind, this is just a new lint in the latest nightly rust-lang/rust@c4c7859. Looks like lines 135 and 263 try to use define_rust_probestack but the macro cfged out on the relevant platforms.

That is weird. The macro is defined within the same file; entire file is only cfged in for x86/x64, and conditionals on the 4 macro defines seem to cover every possible target.

Anyway, this is a bit above my head (I'm just learning Rust so decided to do a light deep immersion by trying to add risc-v support to Redox; just doing something easy, y'know :)).

@tgross35
Copy link
Contributor

That is weird. The macro is defined within the same file; entire file is only cfged in for x86/x64, and conditionals on the 4 macro defines seem to cover every possible target.

It is weird, the new lint might just be wrong. I asked at rust-lang/rust#126984.

Anyway, this is a bit above my head (I'm just learning Rust so decided to do a light deep immersion by trying to add risc-v support to Redox; just doing something easy, y'know :)).

You should probably be able to just do what it suggests and add #![allow(out_of_scope_macro_calls)] to the top of the file so the warning doesn't show up, we can figure out the cause later.

Quite the light deep immersion indeed, welcome to Rust :)

@Amanieu Amanieu merged commit eb6d2b6 into rust-lang:master Jul 6, 2024
24 checks passed
@tea tea deleted the extern_crate branch October 17, 2024 06:32
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.

3 participants