From 319ac5f33b77a77793b716cf01ebdd221564872f Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Tue, 30 May 2017 13:19:18 +0900 Subject: [PATCH 1/4] Add generic_assert --- text/0000-generic-assert.md | 118 ++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 text/0000-generic-assert.md diff --git a/text/0000-generic-assert.md b/text/0000-generic-assert.md new file mode 100644 index 00000000000..100e0960d5d --- /dev/null +++ b/text/0000-generic-assert.md @@ -0,0 +1,118 @@ +- Feature Name: generic_assert +- Start Date: 2017-05-24 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Make the `assert!` macro generic to all expressions, and extend the readability of debug dumps. + +# Motivation +[motivation]: #motivation + +While clippy warns about `assert!` usage that should be replaced by `assert_eq!`, it's quite annoying to migrating 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. + +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 compilation failure if not implemented (custom message should be used for that case). + +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. + +## Examples + +```rust +let a = 1; +let b = 2; +assert!(a == b); +``` + +``` +thread '
' panicked at 'assertion failed: +Expected: a == b +With expansion: 1 == 2' +``` + +With addition operators: + +```rust +let a = 1; +let b = 1; +let c = 3; +assert!(a + b == c); +``` + +``` +thread '
' panicked at 'assertion failed: +Expected: a + b == c +With expansion: 1 + 1 == 3' +``` + +Bool only: +```rust +let v = vec![0u8;1]; +assert!(v.is_empty()); +``` + +``` +thread '
' panicked at 'assertion failed: +Expected: v.is_empty()' +``` + +With short-circuit: +```rust +assert!(true && false && true); +``` + +``` +thread '
' panicked at 'assertion failed: +Expected: true && false && true +With expansion: true && false && (not evaluated)' +``` + +With bracket blocks: +```rust +let a = 1; +let b = 1; +let c = 3; +assert!({a + b} == c); +``` + +``` +thread '
' panicked at 'assertion failed: +Expected: {a + b} == c +With expansion: 2 == 3' +``` + +# How We Teach This +[how-we-teach-this]: #how-we-teach-this + +- Port the documentation (and optionally compiler source) to use `assert!`. +- Mark the old macros (`assert_{eq,ne}!`) as deprecated. + +# Drawbacks +[drawbacks]: #drawbacks + +- This will generate a wave of deprecation warnings, which will be some cost for users to migrate. However, this doesn't mean that this is backward-incompatible, as long as the deprecated macros aren't removed. +- This has a potential performance degradation on complex expressions, due to creating more temporaries on stack (or register). + +# Alternatives +[alternatives]: #alternatives + +- Defining via `macro_rules!` was considered, but the recursive macro can often reach the recursion limit. + +# Unresolved questions +[unresolved]: #unresolved-questions + +## 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? + +## Operators +- Should we handle non-comparison operators? From 9a2cc3931d40cbd1278ef1c6ccad970580a05b32 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Fri, 2 Jun 2017 16:47:54 +0900 Subject: [PATCH 2/4] Add handling regarding non-Debug type --- text/0000-generic-assert.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/text/0000-generic-assert.md b/text/0000-generic-assert.md index 100e0960d5d..d0c6725e338 100644 --- a/text/0000-generic-assert.md +++ b/text/0000-generic-assert.md @@ -20,7 +20,9 @@ Unit test frameworks like [Catch](https://github.com/philsquared/Catch) for C++ 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. -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 compilation failure if not implemented (custom message should be used for that case). +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. @@ -89,6 +91,19 @@ Expected: {a + b} == c With expansion: 2 == 3' ``` +With fallback: +```rust +let a = NonDebug{}; +let b = NonDebug{}; +assert!(a == b); +``` +``` +thread '
' panicked at 'assertion failed: +Expected: a == b +With expansion: (a) == (b)' +``` + + # How We Teach This [how-we-teach-this]: #how-we-teach-this From 0531c8c1d0268ec931d3ada8a9b3f6a308209230 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Sat, 9 Sep 2017 15:37:29 +0800 Subject: [PATCH 3/4] Address FCP consensus --- text/0000-generic-assert.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/text/0000-generic-assert.md b/text/0000-generic-assert.md index d0c6725e338..2a2d541e2b6 100644 --- a/text/0000-generic-assert.md +++ b/text/0000-generic-assert.md @@ -11,14 +11,14 @@ 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`. @@ -26,8 +26,12 @@ On assertion failure, the expression itself is stringified, and another line wit 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). + ## Examples +These examples are purely for reference. The implementor is free to change the rules. + ```rust let a = 1; let b = 2; @@ -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? From 89ee6876280e52cd706ac73e16800fc883c246cb Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Fri, 15 Sep 2017 16:17:05 +0900 Subject: [PATCH 4/4] Update wording, and mentions of performance --- text/0000-generic-assert.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-generic-assert.md b/text/0000-generic-assert.md index 2a2d541e2b6..a5b04642263 100644 --- a/text/0000-generic-assert.md +++ b/text/0000-generic-assert.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -Make the `assert!` macro generic to all expressions, and extend the readability of debug dumps. +Make the `assert!` macro recognize more expressions (utilizing the power of procedural macros), and extend the readability of debug dumps. # Motivation [motivation]: #motivation @@ -118,7 +118,7 @@ With expansion: (a) == (b)' [drawbacks]: #drawbacks - This will generate a wave of deprecation warnings, which will be some cost for users to migrate. However, this doesn't mean that this is backward-incompatible, as long as the deprecated macros aren't removed. -- This has a potential performance degradation on complex expressions, due to creating more temporaries on stack (or register). +- This has a potential performance degradation on complex expressions, due to creating more temporaries on stack (or register). However, if this had clear impacts confirmed through benchmarks, we should use some kind of alternative implementation for release builds. # Alternatives [alternatives]: #alternatives