-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Avoid calling the column!() macro in panic #43735
Conversation
Yes there is - macros 2.0, but this is beta-nominated so I'll rather play safe. Beta-nominating because regression. |
@@ -8,6 +8,16 @@ | |||
// option. This file may not be copied, modified, or distributed | |||
// except according to those terms. | |||
|
|||
#[macro_export] | |||
// This stability attribute is totally useless. | |||
#[stable(feature = "rust1", since = "1.0.0")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe wishful thinking, but if we ever fix macro stability checking it'd be good to have accurate info here. Also for rustdoc purposes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attribute is useless because a) stability attributes on macros are generally useless, and b) because of the #[cfg(stage0)]
below. This won't have any impact on anything presented to users of the compiler. This entire macro definition may be removed if we switch stage0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh OK, missed the #[cfg(stage0)]
. So it will never even reach rustdoc. Carry on then.
Thanks @est31! Could this be made as "real unstable" via the same mechanism as |
@alexcrichton I don't think this will work? Because the |
Hm yeah that's true. Perhaps the span could be checked though for |
@bors: r+ |
📌 Commit 0c32135 has been approved by |
@alexcrichton figured out a way how to do it :)
@bors: r+ |
📌 Commit 5cf9f63 has been approved by |
Avoid calling the column!() macro in panic Closes #43057 This "fix" adds a new macro called `__rust_unstable_column` and to use it instead of the `column` macro inside panic. The new macro can be shadowed as well as `column` can, but its very likely that there is no code that does this in practice. There is no real way to make "unstable" macros that are usable by stable macros, so we do the next best thing and prefix the macro with `__rust_unstable` to make sure people recognize it is unstable. r? @alexcrichton
☀️ Test successful - status-appveyor, status-travis |
Closes #43057
This "fix" adds a new macro called
__rust_unstable_column
and to use it instead of thecolumn
macro inside panic. The new macro can be shadowed as well ascolumn
can, but its very likely that there is no code that does this in practice.There is no real way to make "unstable" macros that are usable by stable macros, so we do the next best thing and prefix the macro with
__rust_unstable
to make sure people recognize it is unstable.r? @alexcrichton