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

Constify assert_eq! and assert_ne! #103639

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions library/core/src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ macro_rules! assert_eq {
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None);
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None, $crate::concat!("assertion failed: `", $crate::stringify!($left), " == ", $crate::stringify!($right), "`"));
}
}
}
Expand All @@ -55,7 +55,7 @@ macro_rules! assert_eq {
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)), $crate::concat!("assertion failed: `", $crate::stringify!($left), " == ", $crate::stringify!($right), "`"));
}
}
}
Expand Down Expand Up @@ -92,7 +92,7 @@ macro_rules! assert_ne {
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None);
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None, $crate::concat!("assertion failed: `", $crate::stringify!($left), " != ", $crate::stringify!($right), "`"));
}
}
}
Expand All @@ -105,7 +105,7 @@ macro_rules! assert_ne {
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)), $crate::concat!("assertion failed: `", $crate::stringify!($left), " != ", $crate::stringify!($right), "`"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that at compile-time, assert_eq/ne will print the stringify'd form of the values, rather than Debug format them, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because we can't format the actual values in compile time unless there is a way to make Display const.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this PR requires us to always support the stringify fallback, no? That is, we won't be able to add a ~const Display requirement to the arguments in the future, if this PR is merged.

We might still support printing via specialization though, if ~const Display is available. Maybe, to ensure we've not painted ourselves into a corner, the stringify fallback could also be enabled via specialization on non-const contexts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? This isn't and shouldn't be insta-stable, which means we can change the implementation should future developments allow const display.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right sorry I've missed the #[rustc_const_unstable(feature = "const_assert_eq", issue = "none")] annotation. Should also answer the question of @Mark-Simulacrum .

}
}
}
Expand Down
40 changes: 38 additions & 2 deletions library/core/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,17 +199,53 @@ pub enum AssertKind {
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller]
#[doc(hidden)]
pub fn assert_failed<T, U>(
#[rustc_const_unstable(feature = "const_assert_eq", issue = "none")]
pub const fn assert_failed<T, U>(
kind: AssertKind,
left: &T,
right: &U,
args: Option<fmt::Arguments<'_>>,
string_for_const_msg: &str,
) -> !
where
T: fmt::Debug + ?Sized,
U: fmt::Debug + ?Sized,
{
assert_failed_inner(kind, &left, &right, args)
#[track_caller]
const fn assert_failed_const_impl<T: ?Sized, U: ?Sized>(
_: AssertKind,
_: &T,
_: &U,
_: Option<fmt::Arguments<'_>>,
msg: &str,
) -> ! {
panic_str(msg)
}

#[track_caller]
#[inline]
fn assert_failed_runtime_impl<T, U>(
kind: AssertKind,
left: &T,
right: &U,
args: Option<fmt::Arguments<'_>>,
_: &str,
) -> !
where
T: fmt::Debug + ?Sized,
U: fmt::Debug + ?Sized,
{
assert_failed_inner(kind, &left, &right, args)
}

// SAFETY: we are panicking in both branches, so this is consistent.
unsafe {
crate::intrinsics::const_eval_select(
(kind, left, right, args, string_for_const_msg),
assert_failed_const_impl,
assert_failed_runtime_impl,
)
}
}

/// Internal function for `assert_match!`
Expand Down
170 changes: 96 additions & 74 deletions tests/mir-opt/issue_99325.main.built.after.mir

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ fn array_casts() -> () {
let mut _32: &usize; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _33: &usize; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _34: std::option::Option<std::fmt::Arguments<'_>>; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _35: &str; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _36: &str; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
scope 1 {
debug x => _1; // in scope 1 at $DIR/retag.rs:+1:9: +1:14
let _2: *mut usize; // in scope 1 at $DIR/retag.rs:+2:9: +2:10
Expand All @@ -45,7 +47,7 @@ fn array_casts() -> () {
debug p => _9; // in scope 5 at $DIR/retag.rs:+6:9: +6:10
let _20: &usize; // in scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _21: &usize; // in scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _35: &usize; // in scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _37: &usize; // in scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
scope 6 {
}
scope 7 {
Expand Down Expand Up @@ -116,12 +118,12 @@ fn array_casts() -> () {
_15 = (*_16); // scope 6 at $DIR/retag.rs:+7:25: +7:34
_14 = &_15; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_18); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_35 = const _; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_37 = const _; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: &usize, val: Unevaluated(array_casts, [], Some(promoted[0])) }
Retag(_35); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_18 = &(*_35); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Retag(_37); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_18 = &(*_37); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Deinit(_13); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
(_13.0: &usize) = move _14; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
(_13.1: &usize) = move _18; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Expand Down Expand Up @@ -167,10 +169,18 @@ fn array_casts() -> () {
Deinit(_34); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
discriminant(_34) = 0; // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Retag(_34); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_28 = core::panicking::assert_failed::<usize, usize>(move _29, move _30, move _32, move _34); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_35); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_36); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_36 = const "assertion failed: `unsafe { *p.add(1) } == 1`"; // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: for<'a, 'b, 'c> fn(core::panicking::AssertKind, &'a usize, &'b usize, Option<Arguments<'c>>) -> ! {core::panicking::assert_failed::<usize, usize>}, val: Value(<ZST>) }
// + literal: Const { ty: &str, val: Value(Slice(..)) }
Retag(_36); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_35 = &(*_36); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_28 = core::panicking::assert_failed::<usize, usize>(move _29, move _30, move _32, move _34, move _35); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: for<'a, 'b, 'c, 'd> fn(core::panicking::AssertKind, &'a usize, &'b usize, Option<Arguments<'c>>, &'d str) -> ! {core::panicking::assert_failed::<usize, usize>}, val: Value(<ZST>) }
}

bb4: {
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/consts/assert_eq_gate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const _: () = assert_eq!(1, 1);
//~^ ERROR `core::panicking::assert_failed` is not yet stable as a const fn
//~| HELP add `#![feature(const_assert_eq)]` to the crate attributes to enable

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/consts/assert_eq_gate.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: `core::panicking::assert_failed` is not yet stable as a const fn
--> $DIR/assert_eq_gate.rs:1:15
|
LL | const _: () = assert_eq!(1, 1);
| ^^^^^^^^^^^^^^^^
|
= help: add `#![feature(const_assert_eq)]` to the crate attributes to enable
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

15 changes: 15 additions & 0 deletions tests/ui/consts/assertions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![feature(const_assert_eq)]

const _BAD1: () = {
assert_eq!(1, 2)
}; //~^ ERROR: evaluation of constant value failed

const _BAD2: () = {
assert_ne!(1, 1)
}; //~^ ERROR: evaluation of constant value failed

const _BAD3: () = {
assert!(false)
}; //~^ ERROR: evaluation of constant value failed

fn main() {}
27 changes: 27 additions & 0 deletions tests/ui/consts/assertions.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error[E0080]: evaluation of constant value failed
--> $DIR/assertions.rs:4:5
|
LL | assert_eq!(1, 2)
| ^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: `1 == 2`', $DIR/assertions.rs:4:5
|
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> $DIR/assertions.rs:8:5
|
LL | assert_ne!(1, 1)
| ^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: `1 != 1`', $DIR/assertions.rs:8:5
|
= note: this error originates in the macro `assert_ne` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> $DIR/assertions.rs:12:5
|
LL | assert!(false)
| ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/assertions.rs:12:5
|
= note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0080`.