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.

Sign up for GitHub

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

Jump to bottom

std: promote debug_assert to assert #98696

Closed
wants to merge 2 commits into from
Closed

std: promote debug_assert to assert #98696

wants to merge 2 commits into from

Conversation

vincenzopalazzo
Copy link
Member

@vincenzopalazzo vincenzopalazzo commented Jun 30, 2022 •
edited
Loading

This is a continuation of work started in #94712 based on the issue #94705

I cleanup all the debug_assert in the std, most of them was related to the UNIX crate

r? @RalfJung

Signed-off-by: Vincenzo Palazzo vincenzopalazzodev@gmail.com

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 30, 2022
@rustbot
Copy link
Collaborator

rustbot commented Jun 30, 2022

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

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @RalfJung (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2022
@RalfJung
Copy link
Member

This needs a libs-team reviewer who is familiar with these APIs, sorry. I was just the guy who pointed out the problem. :)

r? runst-lang/libs

@rust-highfive rust-highfive assigned kennytm and unassigned RalfJung Jun 30, 2022
@RalfJung
Copy link
Member

r? rust-lang/libs

@vincenzopalazzo vincenzopalazzo force-pushed the macros/assert_assumtion branch from 61f3557 to c620d31 Compare June 30, 2022 01:53
@vincenzopalazzo vincenzopalazzo changed the title std: 1/N promot debug_assert to assert std: promote debug_assert to assert Jul 1, 2022
@vincenzopalazzo vincenzopalazzo marked this pull request as draft July 1, 2022 00:53
@vincenzopalazzo vincenzopalazzo force-pushed the macros/assert_assumtion branch 2 times, most recently from 7725a86 to 43b5ce0 Compare July 1, 2022 01:13
@vincenzopalazzo vincenzopalazzo marked this pull request as ready for review July 1, 2022 20:43
@vincenzopalazzo vincenzopalazzo force-pushed the macros/assert_assumtion branch from 43b5ce0 to ac5395c Compare July 1, 2022 20:43
tavianator
tavianator reviewed Jul 3, 2022
View reviewed changes
Copy link
Contributor

@tavianator tavianator left a comment

Choose a reason for hiding this comment

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

Is it worth making a macro so that the assertion messages don't have to repeat the function name?

library/std/src/sys/unix/locks/pthread_condvar.rs Outdated Show resolved Hide resolved
library/std/src/sys/unix/locks/pthread_rwlock.rs Outdated Show resolved Hide resolved
library/std/src/sys/unix/thread.rs Outdated Show resolved Hide resolved
library/std/src/sys/unix/fs.rs Outdated Show resolved Hide resolved
@vincenzopalazzo vincenzopalazzo force-pushed the macros/assert_assumtion branch from f7f9943 to 50da48b Compare July 3, 2022 21:11
@vincenzopalazzo
promote debug_assert to assert
028f141 
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the macros/assert_assumtion branch from 50da48b to f8dabd6 Compare July 19, 2022 20:12
@vincenzopalazzo
remove unecessary assert, and improve others
649aad9 
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the macros/assert_assumtion branch from ea5e25c to 649aad9 Compare July 19, 2022 22:48
@vincenzopalazzo
Copy link
Member Author

This is a monkey solution! I will clean up and reopen a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tavianator tavianator tavianator left review comments

Assignees

@joshtriplett joshtriplett

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@vincenzopalazzo @rustbot @rust-highfive @RalfJung @tavianator @kennytm @joshtriplett