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

Tracking issue for RFC 2011: nicer assert messages #44838

Open
1 of 3 tasks
alexcrichton opened this issue Sep 25, 2017 · 23 comments
Open
1 of 3 tasks

Tracking issue for RFC 2011: nicer assert messages #44838

alexcrichton opened this issue Sep 25, 2017 · 23 comments
Labels
A-error-handling Area: Error handling A-fmt Area: std::fmt B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-generic_assert `#![feature(generic_assert)]` Libs-Tracked Libs issues that are tracked on the team's project board. PG-error-handling Project group: Error handling (https://github.com/rust-lang/project-error-handling) S-tracking-impl-incomplete Status: The implementation is incomplete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

alexcrichton commented Sep 25, 2017

This is a tracking issue for RFC 2011.
The feature gate for the issue is #![feature(generic_assert)].

Sub tracking-issue: #96949.

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

  • Implementation (see section Implementation history)
  • Final comment period (FCP)1
  • Stabilization PR

Blockers

Implementation history

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@alexcrichton alexcrichton added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 25, 2017
@TimNN TimNN added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Sep 27, 2017
@bstrie

This comment has been minimized.

@durka

This comment has been minimized.

@bstrie

This comment has been minimized.

@emilio
Copy link
Contributor

emilio commented Feb 7, 2018

Quick question, is it expected for this to have some sort of switch to less-nice error messages? Sometimes people care about code size, and including a bunch of debug formatters on release builds may not be acceptable.

@ishitatsuyuki
Copy link
Contributor

This was one of the concerns in the RFC, and following are some points:

  • The performance impact may be minimal, as the first step is just making assert having assert_eq's functionality.
  • If notable regression was found, we may decide to disable some of the rules for release builds.
  • For certain critical path, debug assertions or custom formatters may be used to minimize (control) the performance impact, just as before.

@sinkuu
Copy link
Contributor

sinkuu commented Mar 8, 2018

I am trying to implement this, but have not been successful so far. The problem is that we don't know if a operand is Copy during macro expansion.

Status quo(sinkuu@ba00a0e):

assert!(a == b && c)

expands to

let mut __capture0 = None;
let mut __capture1 = None;
let mut __capture2 = None;
// `try_copy` copies it if it implements `Copy`
if !({ let __tmp = a; __capture0 = __tmp.try_copy(); __tmp }
   == { let __tmp = b; __capture1 = __tmp.try_copy(); __tmp }
   && { let __tmp = c; __capture2 = __tmp.try_copy(); __tmp }) {
    panic!("assertion failed: a == b && c\nwith expansion: {:?} == {:?} && {:?}", ...);
}

But this moves non-Copy operands into assert, and causes use-after-move afterward.

@ishitatsuyuki Do you have concrete implementation strategies in mind?

@ishitatsuyuki
Copy link
Contributor

@sinkuu Nice work! However, PartialEq takes self by reference, which means you don't have to copy/clone it. In this case, we may have to call the trait methods directly instead of the operators.

In the case of moving Ops like Add, your approach is good and should just work.

Another thing is that we should use our own enum instead of Option here, as we need to handle the case of non-Copy type and the case of short-circuit differently.

@shepmaster
Copy link
Member

You might be able to use the tricks that assert_eq! uses to take references to the arguments as well.

bors added a commit that referenced this issue Mar 16, 2018
Make `assert` a built-in procedural macro

Makes `assert` macro a built-in one without touching its functionality. This is a prerequisite for RFC 2011 (#44838).
@gilescope
Copy link
Contributor

I’m probably not meant to comment now given the RFC has been merged, but spare a thought for if it’s possible to line up both sides of an == vertically above and below one another as that can make things much easier to spot the difference. Really love this rfc btw, looking forward to it.

@ishitatsuyuki
Copy link
Contributor

@gilescope You're welcome to comment on how we should form the message. The RFC is just meant to be a brief proposal, and there are libraries in other languages that has much more helpful expansion outputs. (Example: https://github.com/power-assert-js/power-assert)

@c410-f3r
Copy link
Contributor

c410-f3r commented Apr 1, 2022

A few people previously tried but I don't think anyone is still working on it. dtolnay's solution seems promising and if you are interested in implementing you should use their code as a starting point.

@ishitatsuyuki Thanks, I shall give it a try

@c410-f3r
Copy link
Contributor

Implementation is waiting for review -> #96496

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue 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 issue 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 added a commit to Dylan-DPC/rust that referenced this issue 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`
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 15, 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`
bors added a commit to rust-lang-ci/rust that referenced this issue 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`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 22, 2022
[RFC 2011] Optimize non-consuming operators

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

The most non-invasive approach that will probably have very little to no performance impact.

## Current behaviour

Captures are handled "on-the-fly", i.e., they are performed in the same place expressions are located.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !(
  { ***try capture `a` and then return `a`*** } > 1 && { ***try capture `b` and then return `b`*** } < 100
) {
  panic!( ... );
}
```

As such, some overhead is likely to occur (Specially with very large chains of conditions).

## New behaviour for non-consuming operators

When an operator is known to not take `self`, then it is possible to capture variables **AFTER** the condition.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !( a > 1 && b < 100 ) {
  { ***try capture `a`*** }
  { ***try capture `b`*** }
  panic!( ... );
}
```

So the possible impact on the runtime execution time will be diminished.

r? `@oli-obk`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 24, 2022
[RFC 2011] Optimize non-consuming operators

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

The most non-invasive approach that will probably have very little to no performance impact.

## Current behaviour

Captures are handled "on-the-fly", i.e., they are performed in the same place expressions are located.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !(
  { ***try capture `a` and then return `a`*** } > 1 && { ***try capture `b` and then return `b`*** } < 100
) {
  panic!( ... );
}
```

As such, some overhead is likely to occur (Specially with very large chains of conditions).

## New behaviour for non-consuming operators

When an operator is known to not take `self`, then it is possible to capture variables **AFTER** the condition.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !( a > 1 && b < 100 ) {
  { ***try capture `a`*** }
  { ***try capture `b`*** }
  panic!( ... );
}
```

So the possible impact on the runtime execution time will be diminished.

r? ``@oli-obk``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 28, 2022
[RFC 2011] Optimize non-consuming operators

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

The most non-invasive approach that will probably have very little to no performance impact.

## Current behaviour

Captures are handled "on-the-fly", i.e., they are performed in the same place expressions are located.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !(
  { ***try capture `a` and then return `a`*** } > 1 && { ***try capture `b` and then return `b`*** } < 100
) {
  panic!( ... );
}
```

As such, some overhead is likely to occur (Specially with very large chains of conditions).

## New behaviour for non-consuming operators

When an operator is known to not take `self`, then it is possible to capture variables **AFTER** the condition.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !( a > 1 && b < 100 ) {
  { ***try capture `a`*** }
  { ***try capture `b`*** }
  panic!( ... );
}
```

So the possible impact on the runtime execution time will be diminished.

r? ```@oli-obk```
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 28, 2022
[RFC 2011] Optimize non-consuming operators

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

The most non-invasive approach that will probably have very little to no performance impact.

## Current behaviour

Captures are handled "on-the-fly", i.e., they are performed in the same place expressions are located.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !(
  { ***try capture `a` and then return `a`*** } > 1 && { ***try capture `b` and then return `b`*** } < 100
) {
  panic!( ... );
}
```

As such, some overhead is likely to occur (Specially with very large chains of conditions).

## New behaviour for non-consuming operators

When an operator is known to not take `self`, then it is possible to capture variables **AFTER** the condition.

```rust
// `let a = 1; let b = 2; assert!(a > 1 && b < 100);`

if !( a > 1 && b < 100 ) {
  { ***try capture `a`*** }
  { ***try capture `b`*** }
  panic!( ... );
}
```

So the possible impact on the runtime execution time will be diminished.

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

The only thing blocking progress is the lack of formatting in constant environments, i.e., things like const _: () = { panic!("{}", 1i32); }; don't work. This current limitation is something that I will try to figure out after enabling the capture of variables in matches! declarations.

Hope to come back here with good news in the next few months.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 26, 2023
[RFC-2011] Expand more expressions

cc rust-lang#44838

Expands `if`, `let`, `match` and also makes `generic_assert_internals` an allowed feature when using `assert!`. `#![feature(generic_assert)]` is still needed to activate everything.

```rust
#![feature(generic_assert)]

fn fun(a: Option<i32>, b: Option<i32>, c: Option<i32>) {
  assert!(
    if a.is_some() { 1 } else { 2 } == 3
      && if let Some(elem) = b { elem == 4 } else { false }
      && match c { Some(_) => true, None => false }
  );
}

fn main() {
  fun(Some(1), None, Some(2));
}

// Assertion failed: assert!(
//   if a.is_some() { 1 } else { 2 } == 3
//     && if let Some(elem) = b { elem == 4 } else { false }
//     && match c { Some(_) => true, None => false }
// );
//
// With captures:
//   a = Some(1)
//   b = None
//   c = Some(2)
```
compiler-errors added a commit to compiler-errors/rust that referenced this issue May 26, 2023
[RFC-2011] Expand more expressions

cc rust-lang#44838

Expands `if`, `let`, `match` and also makes `generic_assert_internals` an allowed feature when using `assert!`. `#![feature(generic_assert)]` is still needed to activate everything.

```rust
#![feature(generic_assert)]

fn fun(a: Option<i32>, b: Option<i32>, c: Option<i32>) {
  assert!(
    if a.is_some() { 1 } else { 2 } == 3
      && if let Some(elem) = b { elem == 4 } else { false }
      && match c { Some(_) => true, None => false }
  );
}

fn main() {
  fun(Some(1), None, Some(2));
}

// Assertion failed: assert!(
//   if a.is_some() { 1 } else { 2 } == 3
//     && if let Some(elem) = b { elem == 4 } else { false }
//     && match c { Some(_) => true, None => false }
// );
//
// With captures:
//   a = Some(1)
//   b = None
//   c = Some(2)
```
bors added a commit to rust-lang-ci/rust that referenced this issue May 27, 2023
[RFC-2011] Expand more expressions

cc rust-lang#44838

Expands `if`, `let`, `match` and also makes `generic_assert_internals` an allowed feature when using `assert!`. `#![feature(generic_assert)]` is still needed to activate everything.

```rust
#![feature(generic_assert)]

fn fun(a: Option<i32>, b: Option<i32>, c: Option<i32>) {
  assert!(
    if a.is_some() { 1 } else { 2 } == 3
      && if let Some(elem) = b { elem == 4 } else { false }
      && match c { Some(_) => true, None => false }
  );
}

fn main() {
  fun(Some(1), None, Some(2));
}

// Assertion failed: assert!(
//   if a.is_some() { 1 } else { 2 } == 3
//     && if let Some(elem) = b { elem == 4 } else { false }
//     && match c { Some(_) => true, None => false }
// );
//
// With captures:
//   a = Some(1)
//   b = None
//   c = Some(2)
```
@fmease fmease added the F-generic_assert `#![feature(generic_assert)]` label Apr 8, 2024
@fmease fmease added the S-tracking-impl-incomplete Status: The implementation is incomplete. label Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error-handling Area: Error handling A-fmt Area: std::fmt B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-generic_assert `#![feature(generic_assert)]` Libs-Tracked Libs issues that are tracked on the team's project board. PG-error-handling Project group: Error handling (https://github.com/rust-lang/project-error-handling) S-tracking-impl-incomplete Status: The implementation is incomplete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests