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

[RFC 2011] Minimal initial implementation #97665

Merged
merged 1 commit into from
Jun 15, 2022
Merged

[RFC 2011] Minimal initial implementation #97665

merged 1 commit into from
Jun 15, 2022

Conversation

c410-f3r
Copy link
Contributor

@c410-f3r c410-f3r commented Jun 2, 2022 •
edited
Loading

Tracking issue: #44838
Third step of #96496

Implementation has ~290 LOC with the bare minimum to be in a functional state. Currently only searches for binary operations to mimic what assert_eq! and assert_ne! already do.

r? @oli-obk

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 2, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 2, 2022
@c410-f3r c410-f3r closed this Jun 2, 2022
@c410-f3r c410-f3r reopened this Jun 2, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@eddyb
Copy link
Member

eddyb commented Jun 3, 2022

Can the PR description be updated with links to the RFC and/or tracking issue? (Ideally the title should also be representative, but that's not as big of a deal)

@c410-f3r c410-f3r changed the title [RFC 2011] Initial implementation [RFC 2011] Minimal initial implementation Jun 3, 2022
@c410-f3r
Copy link
Contributor Author

c410-f3r commented Jun 3, 2022

Can the PR description be updated with links to the RFC and/or tracking issue? (Ideally the title should also be representative, but that's not as big of a deal)

Done

@oli-obk
Copy link
Contributor

oli-obk commented Jun 3, 2022

I won't get to this before 2022-06-13 (in 10 days).

@c410-f3r
Copy link
Contributor Author

c410-f3r commented Jun 3, 2022

I won't get to this before 2022-06-13 (in 10 days).

No worries, take your time. Thanks

oli-obk
oli-obk requested changes Jun 13, 2022
View reviewed changes
compiler/rustc_builtin_macros/src/assert/context.rs Outdated Show resolved Hide resolved
compiler/rustc_builtin_macros/src/assert/context.rs Outdated Show resolved Hide resolved
compiler/rustc_builtin_macros/src/assert/context.rs Outdated Show resolved Hide resolved
compiler/rustc_builtin_macros/src/assert/context.rs Outdated Show resolved Hide resolved
compiler/rustc_builtin_macros/src/assert/context.rs Outdated Show resolved Hide resolved
@c410-f3r
Copy link
Contributor Author

c410-f3r commented Jun 13, 2022 •
edited
Loading

@oli-obk

Thank you for the review. I tried to address as many concerns as possible.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 13, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jun 13, 2022

📌 Commit 1c2c236 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 13, 2022
@Dylan-DPC
Rollup merge of rust-lang#97665 - c410-f3r:assert-compiler, r=oli-obk
7c710f4 
[RFC 2011] Minimal initial implementation

Tracking issue: rust-lang#44838
Third step of rust-lang#96496

Implementation has ~290 LOC with the bare minimum to be in a functional state. Currently only searches for binary operations to mimic what `assert_eq!` and `assert_ne!` already do.

r? `@oli-obk`
@Dylan-DPC Dylan-DPC mentioned this pull request Jun 13, 2022
Closed
@Dylan-DPC Dylan-DPC mentioned this pull request Jun 14, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 14, 2022
@Dylan-DPC
Rollup merge of rust-lang#97665 - c410-f3r:assert-compiler, r=oli-obk
9f02daf 
[RFC 2011] Minimal initial implementation

Tracking issue: rust-lang#44838
Third step of rust-lang#96496

Implementation has ~290 LOC with the bare minimum to be in a functional state. Currently only searches for binary operations to mimic what `assert_eq!` and `assert_ne!` already do.

r? ``@oli-obk``
@Dylan-DPC
Copy link
Member

failed in rollup

@Dylan-DPC
Copy link
Member

@bors r-

@Dylan-DPC
Copy link
Member

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Jun 15, 2022

📌 Commit 605c64a has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 15, 2022
@bors
Copy link
Contributor

bors commented Jun 15, 2022

⌛ Testing commit 605c64a with merge 021f781df9f25e2a55fe359d1872a991e8b58b8a...

@bors
Copy link
Contributor

bors commented Jun 15, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 15, 2022
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
.......... (60/64)
....       (64/64)


/checkout/src/test/rustdoc-gui/toggle-docs.goml An exception occured: socket hang up
== STACKTRACE ==
Error
    at innerRunTestCode (/node-v14.4.0-linux-x64/lib/node_modules/browser-ui-test/src/index.js:533:16)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

@c410-f3r
Copy link
Contributor Author 

/checkout/src/test/rustdoc-gui/toggle-docs.goml An exception occured: socket hang up

WTF... Probably not related

@Dylan-DPC
Copy link
Member

yep it's spurious

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 15, 2022
@c410-f3r
Copy link
Contributor Author

Thanks

@bors
Copy link
Contributor

bors commented Jun 15, 2022

⌛ Testing commit 605c64a with merge ca98305...

@bors
Copy link
Contributor

bors commented Jun 15, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing ca98305 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 15, 2022
@bors bors merged commit ca98305 into rust-lang:master Jun 15, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 15, 2022
This was referenced Jun 15, 2022
Closed
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ca98305): comparison url.

Instruction count

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 0.9% 0.9% 1
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -0.3% -0.3% 1
All 😿🎉 (primary) N/A N/A 0

Max RSS (memory usage)

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 0.8% 0.9% 2
Regressions 😿
(secondary)		 2.2% 3.1% 4
Improvements 🎉
(primary)		 -5.2% -5.2% 1
Improvements 🎉
(secondary)		 -3.5% -4.3% 2
All 😿🎉 (primary) -1.2% -5.2% 3

Cycles

Results
  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -2.2% -2.2% 1
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) -2.2% -2.2% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2022
@bors
Auto merge of rust-lang#98148 - c410-f3r:assert-compiler, r=oli-obk
42dcf70 
[RFC 2011] Expand expressions where possible

Tracking issue: rust-lang#44838
Fourth step of rust-lang#96496

Extends rust-lang#97665 considering expressions that are good candidates for expansion.

r? `@oli-obk`
@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2022

Checked the one regression locally with cachegrind, and it turns out it's actually an improvement, so that's just some perf noise

@sunfishcode sunfishcode mentioned this pull request Jul 7, 2022
Merged
@Dylan-DPC Dylan-DPC mentioned this pull request Aug 24, 2023
Open
3 tasks
@fmease fmease added the F-generic_assert `#![feature(generic_assert)]` label Apr 8, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

Labels
F-generic_assert `#![feature(generic_assert)]` merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
10 participants
@c410-f3r @rust-log-analyzer @eddyb @oli-obk @bors @Dylan-DPC @rust-timer @rust-highfive @fmease @rustbot