Constify `assert_eq!` and `assert_ne!` #103639

fee1-dead · 2022-10-27T16:13:37Z

We provide an inferior type of formatting for use when panicking in compile time, which should not come with additional costs as the additional argument should be inlined and optimized away when in runtime.

As for additional code in assert_failed: this is an implementation detail that is hard to find and not intended to be exposed anyways, so I don't think it will hurt readability much.

rustbot · 2022-10-27T16:13:41Z

r? @thomcc

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

rustbot · 2022-10-27T16:13:44Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

bors · 2022-10-31T07:24:37Z

☔ The latest upstream changes (presumably #103787) made this pull request unmergeable. Please resolve the merge conflicts.

thomcc · 2022-11-04T04:45:37Z

Rerolling this. I think it's a good idea but I'm not sure if this is "api change" or "just let wg-const-eval do their thing"

r? libs

apiraino · 2022-11-29T15:44:42Z

I am not sure this needs T-compiler review/signoff. In case, please re-add the team, thanks :)

@rustbot label -T-compiler

fee1-dead · 2022-12-30T07:42:36Z

r? libs

bors · 2023-01-07T19:30:32Z

☔ The latest upstream changes (presumably #105323) made this pull request unmergeable. Please resolve the merge conflicts.

Mark-Simulacrum · 2023-01-21T16:03:15Z

library/core/src/macros/mod.rs

@@ -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), "`"));


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

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

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?

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.

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 .

Mark-Simulacrum · 2023-01-21T16:04:03Z

Is this immediately accessible on stable? If not, is there a test validating that already?

We provide an inferior type of formatting for use when panicking in compile time, which should not come with additional costs as the additional argument should be inlined and optimized away when in runtime

fee1-dead · 2023-01-27T07:33:37Z

@rustbot ready

bors · 2023-02-04T18:41:05Z

☔ The latest upstream changes (presumably #107267) made this pull request unmergeable. Please resolve the merge conflicts.

a1phyr · 2023-02-10T09:28:57Z

Preview work on #79100 showed that these macros are quite perf-sensitive, and adding a concat! and two stringify! seems quite heavy.
I wouldn't merge this without a perf run.

rustbot assigned thomcc Oct 27, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler 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 Oct 27, 2022

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the const_assert_eq branch from 7b4bc71 to e0e35db Compare October 27, 2022 16:26

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the const_assert_eq branch from e0e35db to 3749356 Compare October 27, 2022 17:53

This comment has been minimized.

Sign in to view

rustbot assigned m-ou-se and unassigned thomcc Nov 4, 2022

fee1-dead mentioned this pull request Nov 8, 2022

Make Hash, Hasher and BuildHasher #[const_trait] and make Sip const Hasher #104060

Merged

rustbot removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 29, 2022

rustbot assigned Mark-Simulacrum and unassigned m-ou-se Dec 30, 2022

fee1-dead force-pushed the const_assert_eq branch from b48319c to 2353dcf Compare December 30, 2022 07:59

This comment has been minimized.

Sign in to view

Mark-Simulacrum reviewed Jan 21, 2023

View reviewed changes

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 21, 2023

clarfonthey mentioned this pull request Jan 22, 2023

Flatten/inline format_args!() and (string and int) literal arguments into format_args!() #106824

Merged

Constify assert_eq! and assert_ne!

2229863

We provide an inferior type of formatting for use when panicking in compile time, which should not come with additional costs as the additional argument should be inlined and optimized away when in runtime

fee1-dead added 2 commits January 27, 2023 07:23

bless mir

4b7dc2c

add gate test and bless mir

89dac4c

fee1-dead force-pushed the const_assert_eq branch from 2353dcf to 89dac4c Compare January 27, 2023 07:32

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 27, 2023

fee1-dead marked this pull request as draft February 25, 2023 10:15

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 25, 2023

fee1-dead closed this Mar 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constify `assert_eq!` and `assert_ne!` #103639

Constify `assert_eq!` and `assert_ne!` #103639

fee1-dead commented Oct 27, 2022 •

edited

Loading

rustbot commented Oct 27, 2022

rustbot commented Oct 27, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Oct 31, 2022

thomcc commented Nov 4, 2022

apiraino commented Nov 29, 2022

fee1-dead commented Dec 30, 2022

This comment has been minimized.

bors commented Jan 7, 2023

Mark-Simulacrum Jan 21, 2023

fee1-dead Jan 22, 2023

est31 Jan 22, 2023

fee1-dead Jan 22, 2023

est31 Jan 22, 2023

Mark-Simulacrum commented Jan 21, 2023

fee1-dead commented Jan 27, 2023

bors commented Feb 4, 2023

a1phyr commented Feb 10, 2023

Constify assert_eq! and assert_ne! #103639

Constify assert_eq! and assert_ne! #103639

Conversation

fee1-dead commented Oct 27, 2022 • edited Loading

rustbot commented Oct 27, 2022

rustbot commented Oct 27, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Oct 31, 2022

thomcc commented Nov 4, 2022

apiraino commented Nov 29, 2022

fee1-dead commented Dec 30, 2022

This comment has been minimized.

bors commented Jan 7, 2023

Mark-Simulacrum Jan 21, 2023

Choose a reason for hiding this comment

fee1-dead Jan 22, 2023

Choose a reason for hiding this comment

est31 Jan 22, 2023

Choose a reason for hiding this comment

fee1-dead Jan 22, 2023

Choose a reason for hiding this comment

est31 Jan 22, 2023

Choose a reason for hiding this comment

Mark-Simulacrum commented Jan 21, 2023

fee1-dead commented Jan 27, 2023

bors commented Feb 4, 2023

a1phyr commented Feb 10, 2023

Constify `assert_eq!` and `assert_ne!` #103639

Constify `assert_eq!` and `assert_ne!` #103639

fee1-dead commented Oct 27, 2022 •

edited

Loading