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

Nightly regression: panic::set_hook() signature incompatible if used with core::panic::PanicInfo #126766

Closed
Manishearth opened this issue Jun 20, 2024 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. relnotes Marks issues that should be documented in the release notes of the next release. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Milestone

Comments

@Manishearth
Copy link
Member

Manishearth commented Jun 20, 2024

Code

I tried this code:

#![no_std]

// Yes, this is `no_std` + `extern crate std`, which happens a lot in
// crates that are `cfg(not(something), no_std)`

extern crate std;
extern crate alloc;
use alloc::boxed::Box;

fn main() {
    
    std::panic::set_hook(Box::new(panic_handler));
}

fn panic_handler(info: &core::panic::PanicInfo) {

}

playground

I expected to see this happen: It compiles just fine

Instead, this happened: I get the following error:

error[E0631]: type mismatch in function arguments
  --> src/main.rs:9:26
   |
9  |     std::panic::set_hook(Box::new(panic_handler));
   |                          ^^^^^^^^^^^^^^^^^^^^^^^ expected due to this
...
12 | fn panic_handler(info: &core::panic::PanicInfo) {
   | ----------------------------------------------- found signature defined here
   |
   = note: expected function signature `for<'a, 'b> fn(&'a PanicHookInfo<'b>) -> _`
              found function signature `fn(&PanicInfo<'_>) -> _`
   = note: required for the cast from `Box<for<'a, 'b> fn(&'a PanicInfo<'b>) {panic_handler}>` to `Box<(dyn for<'a, 'b> Fn(&'a PanicHookInfo<'b>) + Send + Sync + 'static)>`

On nightly PanicInfo is a type alias for PanicHookInfo, however the alias only exists in the std crate. If you reference PanicInfo from core, that is now a different type that does not unify with PanicHookInfo.

It is relatively common in crates that support no_std to always use the lowest possible crate to reference for any reexported type, which is why our code was referencing core::panic even though this particular snippet of code needs std to work overall.

Version it worked on

It most recently worked on: Rust 1.79, Rust 1.80-beta3

Version with regression

rustc --version --verbose:

1.81.0-nightly (2024-06-19 d8a38b00024cd7156dea)
@Manishearth Manishearth added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jun 20, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 20, 2024
@workingjubilee workingjubilee added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 20, 2024
@asquared31415
Copy link
Contributor

It doesn't even need to be a no_std crate, simply using use core::panic::PanicInfo; and using that PanicInfo in the panic hook signature reproduces this error.

@bjorn3
Copy link
Member

bjorn3 commented Jun 21, 2024

#115974 was merged under the assumption that std crates are unlikely to use core::panic::PanicInfo instead of std::panic::PanicInfo when referring to the type of the panic hook argument. It is expected that core::panic::PanicInfo for the panic hook doesn't work after that PR.

@Manishearth Manishearth changed the title Nightly regression: panic::set_hook() signature incompatible in the absence of the std prelude Nightly regression: panic::set_hook() signature incompatible if used with core::panic::PanicInfo Jun 21, 2024
@Manishearth
Copy link
Member Author

Nice catch!

@asquared31415
Copy link
Contributor

#115974 was merged under the assumption that std crates are unlikely to use core::panic::PanicInfo instead of std::panic::PanicInfo when referring to the type of the panic hook argument. It is expected that core::panic::PanicInfo for the panic hook doesn't work after that PR.

rust-analyzer has a setting for "prefer core over std for imports" that myself and i assume many others use, in all projects (also at this point I use core when possible by habit). If it's assumed that users should be using the std version when std is available, at the least the error should provide a fix that changes the import, and rust-analyzer should know how to exclude some imports from its setting.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 23, 2024
@joshtriplett joshtriplett added the relnotes Marks issues that should be documented in the release notes of the next release. label Jul 10, 2024
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 11, 2024
@cuviper cuviper added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Jul 31, 2024
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 31, 2024
@cuviper cuviper added this to the 1.81.0 milestone Jul 31, 2024
@lqd lqd removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 31, 2024
@m-ou-se m-ou-se removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Aug 15, 2024
@workingjubilee workingjubilee added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 16, 2024
@Mark-Simulacrum Mark-Simulacrum added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Oct 11, 2024
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 11, 2024
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 12, 2024
@apiraino
Copy link
Contributor

Closing since regression is mentioned in the release notes

@apiraino apiraino closed this as not planned Won't fix, can't repro, duplicate, stale Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. relnotes Marks issues that should be documented in the release notes of the next release. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests