Skip to content

[RFC 2011] Minimal initial implementation#97665

Merged
bors merged 1 commit into
rust-lang:masterfrom
c410-f3r:assert-compiler
Jun 15, 2022
Merged

[RFC 2011] Minimal initial implementation#97665
bors merged 1 commit into
rust-lang:masterfrom
c410-f3r:assert-compiler

Conversation

@c410-f3r

@c410-f3r c410-f3r commented Jun 2, 2022

Copy link
Copy Markdown
Contributor

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
Comment thread src/test/ui/rfc-2011-nicer-assert-messages/codegen.stdout Outdated
@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.

@rust-log-analyzer

This comment has been minimized.

@eddyb

eddyb commented Jun 3, 2022

Copy link
Copy Markdown
Contributor

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

c410-f3r commented Jun 3, 2022

Copy link
Copy Markdown
Contributor Author

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

oli-obk commented Jun 3, 2022

Copy link
Copy Markdown
Contributor

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

@c410-f3r

c410-f3r commented Jun 3, 2022

Copy link
Copy Markdown
Contributor Author

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

No worries, take your time. Thanks

Comment thread compiler/rustc_builtin_macros/src/assert/context.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/assert/context.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/assert/context.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/assert/context.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/assert/context.rs Outdated
@c410-f3r

c410-f3r commented Jun 13, 2022

Copy link
Copy Markdown
Contributor Author

@oli-obk

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

@oli-obk

oli-obk commented Jun 13, 2022

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jun 13, 2022

Copy link
Copy Markdown
Collaborator

📌 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
[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 added a commit to Dylan-DPC/rust that referenced this pull request Jun 14, 2022
[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
Copy Markdown
Member

failed in rollup

@Dylan-DPC

Copy link
Copy Markdown
Member

@bors r-

@Dylan-DPC

Copy link
Copy Markdown
Member

@bors r=oli-obk

@bors

bors commented Jun 15, 2022

Copy link
Copy Markdown
Collaborator

📌 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

bors commented Jun 15, 2022

Copy link
Copy Markdown
Collaborator

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

@bors

bors commented Jun 15, 2022

Copy link
Copy Markdown
Collaborator

💔 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
Copy Markdown
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
Copy Markdown
Contributor Author
/checkout/src/test/rustdoc-gui/toggle-docs.goml An exception occured: socket hang up

WTF... Probably not related

@Dylan-DPC

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Thanks

@bors

bors commented Jun 15, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 605c64a with merge ca98305...

@bors

bors commented Jun 15, 2022

Copy link
Copy Markdown
Collaborator

☀️ 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
@rust-timer

Copy link
Copy Markdown
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
[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

oli-obk commented Jun 21, 2022

Copy link
Copy Markdown
Contributor

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

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

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

Development

Successfully merging this pull request may close these issues.

10 participants