Closed
Description
The behavior of unreachable
does not depend the edition of the crate that invoked the macro. That's fine for todo!()
and unimplemented!()
due to the way they are implemented, but in #92068, it was discovered that unreachable!()
behaves subtly different, due to the way it has a special one-argument case. Looks like missed this when working on the 2021 panic macros. :(
Metadata
Metadata
Assignees
Labels
Area: The 2021 editionArea: All kinds of macros (custom derive, macro_rules!, proc macros, ..)Category: This is a bug.High priorityRelevant to the library API team, which will review and decide on the PR/issue.This issue / PR is in PFCP or FCP with a disposition to merge it.The final comment period is finished for this PR / Issue.Marks issues that should be documented in the release notes of the next release.
Activity
[-]unimplemented!("{}") works on Rust 2021[/-][+]unreachable!("{}") works on Rust 2021[/+]m-ou-se commentedon Dec 20, 2021
todo!()
andunimplemented!()
are implemented like this:rust/library/core/src/macros/mod.rs
Lines 738 to 741 in 23f6923
They never accepted
todo!("{}")
ortodo!(123)
since their arguments are always directly passed toformat_args!()
. So, they didn't have to change for Rust 2021.unreachable!()
however, is implemented differently:rust/library/core/src/macros/mod.rs
Lines 592 to 602 in 23f6923
It doesn't just forward to
panic!()
orformat_args!()
, but instead it does its ownconcat
shenanigans, and has special handling of the single argument case.m-ou-se commentedon Dec 20, 2021
So then the question is: Do we consider this a bug in Rust 2021 and fix it in Rust 1.59 for that edition? Or leave this in and only fix it in a later edition (or never)?
There's quite a few cases on crates.io of Rust 2015/2018 code depending on this behavior, but Rust 2021 is very new at this point. Adding this change/fix to Rust 2021 probably doesn't break anyone, yet.
m-ou-se commentedon Dec 20, 2021
Behavior:
m-ou-se commentedon Dec 20, 2021
We can't fix
unreachable
without making it a builtin macro, likepanic!()
. That's because (unlike e.g.debug_assert
), this macro doesn't just forward to assert/panic, but has its own problematic implementation with a special case that shouldn't exist.jyn514 commentedon Dec 20, 2021
I'm not sure I follow - can't we remove the concat special case and forward directly to panic, like todo does?
m-ou-se commentedon Dec 20, 2021
@jyn514 Then
unreachable!(123)
would stop working on Rust 2015/2018.m-ou-se commentedon Dec 20, 2021
From a quick grep through crates.io, it looks like there's about 15 crates on crates.io depending on the current behavior that would break if we made it work like todo!(). Here's a few examples from the grep results:
joshtriplett commentedon Dec 20, 2021
Making
unreachable
built-in to fix this on Rust 2021 seems reasonable to me.jhpratt commentedon Dec 21, 2021
IMO as long as there's a crater run fixing this is reasonable. It's certainly a bug.
67 remaining items