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

Add generic_assert #2011

Merged
merged 4 commits into from
Sep 25, 2017
Merged

Add generic_assert #2011

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
319ac5f
Add generic_assert
ishitatsuyuki May 30, 2017
9a2cc39
Add handling regarding non-Debug type
ishitatsuyuki Jun 2, 2017
0531c8c
Address FCP consensus
ishitatsuyuki Sep 9, 2017
89ee687
Update wording, and mentions of performance
ishitatsuyuki Sep 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions text/0000-generic-assert.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,27 @@ Make the `assert!` macro generic to all expressions, and extend the readability
# Motivation
[motivation]: #motivation

While clippy warns about `assert!` usage that should be replaced by `assert_eq!`, it's quite annoying to migrating around.
While clippy warns about `assert!` usage that should be replaced by `assert_eq!`, it's quite annoying to migrate around.

Unit test frameworks like [Catch](https://github.com/philsquared/Catch) for C++ does cool message printing already by using macros.

# Detailed design
[design]: #detailed-design

We're going to parse AST and break up them by operators (excluding `.` (dot, member access operator)). Function calls and bracket surrounded blocks are considered as one block and don't get split further.
We're going to parse AST and break up them by operators (excluding `.` (dot, member access operator)). Function calls and bracket surrounded blocks are considered as one block and don't get expanded. The exact expanding rules should be determined when implemented, but an example is provided for reference.

On assertion failure, the expression itself is stringified, and another line with intermediate values are printed out. The values should be printed with `Debug`, and a plain text fallback if the following conditions fail:
- the type doesn't implement `Debug`.
- the operator is non-comparison (those in `std::ops`) and the type (may also be a reference) doesn't implement `Copy`.

To make sure that there's no side effects involved (e.g. running `next()` twice on `Iterator`), each value should be stored as temporaries and dumped on assertion failure.

The new assert messages are likely to generate longer code, and it may be simplified for release builds (if benchmarks confirm the slowdown).
Copy link

Choose a reason for hiding this comment

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

There's really no reason to include this functionality in release builds at all. You don't want the increased code size, nor this information, in released software.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of Rust's advantage is that you can get backtrace properly even in release builds. Basically, if you don't know how the assertion failed, it would be almost impossible to diagnose the crash.

Copy link

Choose a reason for hiding this comment

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

Right, and I can appreciate that, but there can also be other constraints in play such as in embedded systems. You might want to disable all the extras and just handle errors that occur by restarting.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll add that in commercial software it's also desirable to hide all information about implementation details.

Copy link
Member

Choose a reason for hiding this comment

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

@repax: You could easily redefine your own assert! to hide every detail.

#[cfg(not(debug_assertions))]  // <------
macro_rules! assert {
    ($e:expr) => {
        if !$e {
            panic!("assertion failure");
        }
    };
    ($e:expr, $($rest:tt)*) => {
        assert!($e)
    }
}

fn main() {
    let a = 2;
    let b = 2;
    let c = 5;
    assert!(a + b == c, "note: failed assertion {} + {} == {}", a, b, c);
}

Copy link

Choose a reason for hiding this comment

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

@kennytm: Why not just make this a compile-time option? Sometimes you want this debugging convenience, often times not. In designing features like this I think it's important to take into account the breadth of contexts in which rust code may operate in the wild.

Copy link
Member

@kennytm kennytm Sep 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@repax Define a cargo feature and use #[cfg(feature = "redacted_assert")] then.

Copy link
Contributor

Choose a reason for hiding this comment

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

My macro probably wouldn't apply to 3rd party crates, so this doesn't quite solve the bloat issue.


## Examples

These examples are purely for reference. The implementor is free to change the rules.

```rust
let a = 1;
let b = 2;
Expand Down Expand Up @@ -120,12 +124,14 @@ With expansion: (a) == (b)'
[alternatives]: #alternatives

- Defining via `macro_rules!` was considered, but the recursive macro can often reach the recursion limit.
- Negating the operator (`!=` to `==`) was considered, but this isn't suitable for all cases as not all types are total ordering.

# Unresolved questions
[unresolved]: #unresolved-questions

These questions should be settled during the implementation process.

## Error messages
- Should we use the negated version of operators (e.g. `!=` for eq) to explain the failure?
- Should we dump the AST as a formatted one?
- How are we going to handle multi-line expressions?

Expand Down