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

Throw core::panic!("message") as &str instead of String. #78119

Merged
merged 2 commits into from
Oct 25, 2020

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Oct 19, 2020

This makes core::panic!("message") consistent with std::panic!("message"), which throws a &str and not a String.

This also makes any other panics from core::panicking::panic result in a &str rather than a String, which includes compiler-generated panics such as the panics generated for mem::zeroed().


Demonstration:

use std::panic;
use std::any::Any;

fn main() {
    panic::set_hook(Box::new(|panic_info| check(panic_info.payload())));

    check(&*panic::catch_unwind(|| core::panic!("core")).unwrap_err());
    check(&*panic::catch_unwind(|| std::panic!("std")).unwrap_err());
}

fn check(msg: &(dyn Any + Send)) {
    if let Some(s) = msg.downcast_ref::<String>() {
        println!("Got a String: {:?}", s);
    } else if let Some(s) = msg.downcast_ref::<&str>() {
        println!("Got a &str: {:?}", s);
    }
}

Before:

Got a String: "core"
Got a String: "core"
Got a &str: "std"
Got a &str: "std"

After:

Got a &str: "core"
Got a &str: "core"
Got a &str: "std"
Got a &str: "std"

This makes it consistent with std::panic!("message"), which also throws
a &str, not a String.
@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 19, 2020
It now throws a &str instead of a String.
@camelid camelid added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows labels Oct 19, 2020
@joshtriplett
Copy link
Member

Seems reasonable to me, but I think a libs person needs to review this one.

@m-ou-se
Copy link
Member Author

m-ou-se commented Oct 21, 2020

a libs person needs to review this one.

@rust-lang/libs

@Amanieu
Copy link
Member

Amanieu commented Oct 21, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Oct 21, 2020

📌 Commit 9890217 has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 21, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 22, 2020
…as-str, r=Amanieu

Throw core::panic!("message") as &str instead of String.

This makes `core::panic!("message")` consistent with `std::panic!("message")`, which throws a `&str` and not a `String`.

This also makes any other panics from `core::panicking::panic` result in a `&str` rather than a `String`, which includes compiler-generated panics such as the panics generated for `mem::zeroed()`.

---

Demonstration:

```rust
use std::panic;
use std::any::Any;

fn main() {
    panic::set_hook(Box::new(|panic_info| check(panic_info.payload())));

    check(&*panic::catch_unwind(|| core::panic!("core")).unwrap_err());
    check(&*panic::catch_unwind(|| std::panic!("std")).unwrap_err());
}

fn check(msg: &(dyn Any + Send)) {
    if let Some(s) = msg.downcast_ref::<String>() {
        println!("Got a String: {:?}", s);
    } else if let Some(s) = msg.downcast_ref::<&str>() {
        println!("Got a &str: {:?}", s);
    }
}
```

Before:
```
Got a String: "core"
Got a String: "core"
Got a &str: "std"
Got a &str: "std"
```

After:
```
Got a &str: "core"
Got a &str: "core"
Got a &str: "std"
Got a &str: "std"
```
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 22, 2020
…as-str, r=Amanieu

Throw core::panic!("message") as &str instead of String.

This makes `core::panic!("message")` consistent with `std::panic!("message")`, which throws a `&str` and not a `String`.

This also makes any other panics from `core::panicking::panic` result in a `&str` rather than a `String`, which includes compiler-generated panics such as the panics generated for `mem::zeroed()`.

---

Demonstration:

```rust
use std::panic;
use std::any::Any;

fn main() {
    panic::set_hook(Box::new(|panic_info| check(panic_info.payload())));

    check(&*panic::catch_unwind(|| core::panic!("core")).unwrap_err());
    check(&*panic::catch_unwind(|| std::panic!("std")).unwrap_err());
}

fn check(msg: &(dyn Any + Send)) {
    if let Some(s) = msg.downcast_ref::<String>() {
        println!("Got a String: {:?}", s);
    } else if let Some(s) = msg.downcast_ref::<&str>() {
        println!("Got a &str: {:?}", s);
    }
}
```

Before:
```
Got a String: "core"
Got a String: "core"
Got a &str: "std"
Got a &str: "std"
```

After:
```
Got a &str: "core"
Got a &str: "core"
Got a &str: "std"
Got a &str: "std"
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2020
…as-schievink

Rollup of 12 pull requests

Successful merges:

 - rust-lang#75115 (`#[deny(unsafe_op_in_unsafe_fn)]` in sys/cloudabi)
 - rust-lang#76614 (change the order of type arguments on ControlFlow)
 - rust-lang#77610 (revise Hermit's mutex interface to support the behaviour of StaticMutex)
 - rust-lang#77830 (Simplify query proc-macros)
 - rust-lang#77930 (Do not ICE with TraitPredicates containing [type error])
 - rust-lang#78069 (Fix const core::panic!(non_literal_str).)
 - rust-lang#78072 (Cleanup constant matching in exhaustiveness checking)
 - rust-lang#78119 (Throw core::panic!("message") as &str instead of String.)
 - rust-lang#78191 (Introduce a temporary for discriminant value in MatchBranchSimplification)
 - rust-lang#78272 (const_evaluatable_checked: deal with unused nodes + div)
 - rust-lang#78318 (TyCtxt: generate single impl block with `slice_interners` macro)
 - rust-lang#78327 (resolve: Relax macro resolution consistency check to account for any errors)

Failed merges:

r? `@ghost`
@bors bors merged commit e3808ed into rust-lang:master Oct 25, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 25, 2020
@m-ou-se m-ou-se deleted the panic-use-as-str branch February 6, 2021 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants