fix silent overflows on `Step` impls #36365

matthew-piziak · 2016-09-09T16:22:08Z

@eddyb

sfackler · 2016-09-10T02:39:13Z

src/libcoretest/iter.rs

@@ -916,6 +916,20 @@ fn test_range_step() {
 }

 #[test]
+#[should_panic]
+fn test_range_overflow_unsigned() {
+    let mut it = u8::MAX..;


Won't these tests fail if compiled in release mode?

Ahh, that's a good point. Then these have to be run-fail with the overflow checking forced on (see src/test/run-fail/*overflow*.rs).

See #36372 for reference on making these tests.

eddyb · 2016-09-13T15:16:44Z

@bors r+ Thanks!

bors · 2016-09-13T15:16:45Z

📌 Commit 065bf4b has been approved by eddyb

bors · 2016-09-14T01:56:51Z

⌛ Testing commit 065bf4b with merge 0154682...

bors · 2016-09-14T02:18:00Z

💔 Test failed - auto-win-msvc-64-opt-rustbuild

alexcrichton · 2016-09-14T05:42:04Z

@bors: retry

On Tue, Sep 13, 2016 at 7:18 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-msvc-64-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt-rustbuild/builds/2486

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#36365 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95EWeF6rKv73hOHXH5igvSEAkWotsks5qp1lbgaJpZM4J5OGd
.

bors · 2016-09-15T07:08:43Z

⌛ Testing commit 065bf4b with merge 16ca4ec...

bors · 2016-09-15T08:19:48Z

💔 Test failed - auto-win-gnu-32-opt-rustbuild

bluss · 2016-09-24T11:24:05Z

Test doesn't pass and needs update

alexcrichton · 2016-09-26T23:16:08Z

Could this also use the strategy in another PR to use Add::add and Sub::sub (UFCS) instead of the unstable attributes?

matthew-piziak · 2016-10-08T22:27:18Z

Sorry for the inactivity. I've been traveling for a few weeks.

@alexcrichton Can you elaborate, or link the PR if you remember which one it was?

eddyb · 2016-10-08T23:04:58Z

src/libcore/iter/range.rs

@@ -95,11 +95,13 @@ macro_rules! step_impl_unsigned {
            }

            #[inline]
+            #[rustc_inherit_overflow_checks]
            fn add_one(&self) -> Self {
                *self + 1


Add::add(*self, 1) is what @alexcrichton meant. Similar situation on the rest.

… silent-overflow

MIN and MAX were breaking the build for me. min_value and max_value do not.

alexcrichton · 2016-10-11T22:06:30Z

Looks like travis failures may be legit?

into silent-overflow

matthew-piziak · 2016-10-23T02:34:44Z

I'm missing something here. My tests are failing on explicit panic instead of overflowing as expected.

alexcrichton · 2016-10-25T03:24:50Z

The current failures look like:

failures:
thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:298

---- [run-fail] run-fail/iter-step-overflow-debug.rs stdout ----

error: failure produced the wrong error: exit code: 0
status: exit code: 0
command: /build/build/x86_64-unknown-linux-gnu/test/run-fail/iter-step-overflow-debug.stage2-x86_64-unknown-linux-gnu 
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'attempt to add with overflow', src/libcore/ops.rs:315
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread 'main' panicked at 'attempt to add with overflow', src/libcore/ops.rs:315

------------------------------------------

thread '[run-fail] run-fail/iter-step-overflow-debug.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:2377
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- [run-fail] run-fail/iter-step-overflow-ndebug.rs stdout ----

error: no error pattern specified in "/build/src/test/run-fail/iter-step-overflow-ndebug.rs"
thread '[run-fail] run-fail/iter-step-overflow-ndebug.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:1588


failures:
    [run-fail] run-fail/iter-step-overflow-debug.rs
    [run-fail] run-fail/iter-step-overflow-ndebug.rs

I think these tests should be run-pass tests instead of run-fail tests perhaps?

alexcrichton · 2016-10-31T02:40:30Z

Looks like travis may unfortunately still be failing?

---- [run-pass] run-pass/iter-step-overflow-ndebug.rs stdout ----

error: test run failed!
status: exit code: 101
command: /build/build/x86_64-unknown-linux-gnu/test/run-pass/iter-step-overflow-ndebug.stage2-x86_64-unknown-linux-gnu 
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'assertion failed: `(left == right)` (left: `255`, right: `0`)', /build/src/test/run-pass/iter-step-overflow-ndebug.rs:15
note: Run with `RUST_BACKTRACE=1` for a backtrace.

------------------------------------------

thread '[run-pass] run-pass/iter-step-overflow-ndebug.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:2377
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    [run-pass] run-pass/iter-step-overflow-ndebug.rs

steveklabnik · 2016-11-07T18:33:31Z

Looks like travis is passing now, r? @eddyb

eddyb · 2016-11-07T18:45:20Z

@bors r+

bors · 2016-11-07T18:45:23Z

📌 Commit 8f19d5c has been approved by eddyb

bors · 2016-11-07T19:48:16Z

⌛ Testing commit 8f19d5c with merge 57f971b...

@eddyb

fix silent overflows on `Step` impls Part of #36110 r? @eddyb

bors · 2016-11-07T23:04:18Z

fix silent overflows on Step impls

f6c4fcf

r? @eddyb

rust-highfive assigned eddyb Sep 9, 2016

sfackler reviewed Sep 10, 2016
View reviewed changes

matthew-piziak added 2 commits September 12, 2016 11:23

add run-fail tests

19fd7fc

remove non-run-fail tests

065bf4b

eddyb reviewed Oct 8, 2016

View reviewed changes

matthew-piziak added 3 commits October 9, 2016 06:35

use UFCS with Add::add and Sub::sub instead of unstable attributes

dc607de

Merge branch 'silent-overflow' of github.com:matthew-piziak/rust into…

a802ec1

… silent-overflow

replace MIN/MAX constants with min_value/max_value functions

dba6678

MIN and MAX were breaking the build for me. min_value and max_value do not.

matthew-piziak added 3 commits October 11, 2016 20:17

remove erroneous semicolons

db1e85d

unpwrap iterators in tests

cb26d55

Merge branch 'silent-overflow' of https://github.com/matthew-piziak/rust

2b4c778

into silent-overflow

move overflow tests from rust-fail to run-pass

dd6e8c5

add max value from iterator

8f19d5c

eddyb approved these changes Nov 7, 2016

View reviewed changes

bors added a commit that referenced this pull request Nov 7, 2016

Auto merge of #36365 - matthew-piziak:silent-overflow, r=eddyb

57f971b

fix silent overflows on `Step` impls Part of #36110 r? @eddyb

bors mentioned this pull request Nov 7, 2016

macros: improve reexports #37463

Merged

bors merged commit 8f19d5c into rust-lang:master Nov 7, 2016

bluss mentioned this pull request Nov 23, 2016

[Regression] test fails on 1.14.0-beta.2 (e627a2e6e 2016-11-16) with 'attempt to add with overflow' #37959

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix silent overflows on `Step` impls #36365

fix silent overflows on `Step` impls #36365

matthew-piziak commented Sep 9, 2016

sfackler Sep 10, 2016

eddyb Sep 10, 2016

sfackler Sep 10, 2016

eddyb commented Sep 13, 2016

bors commented Sep 13, 2016

bors commented Sep 14, 2016

bors commented Sep 14, 2016

alexcrichton commented Sep 14, 2016

bors commented Sep 15, 2016

bors commented Sep 15, 2016

bluss commented Sep 24, 2016

alexcrichton commented Sep 26, 2016

matthew-piziak commented Oct 8, 2016

eddyb Oct 8, 2016

alexcrichton commented Oct 11, 2016

matthew-piziak commented Oct 23, 2016

alexcrichton commented Oct 25, 2016

alexcrichton commented Oct 31, 2016

steveklabnik commented Nov 7, 2016

eddyb commented Nov 7, 2016

bors commented Nov 7, 2016

bors commented Nov 7, 2016

bors commented Nov 7, 2016

fix silent overflows on Step impls #36365

fix silent overflows on Step impls #36365

Conversation

matthew-piziak commented Sep 9, 2016

sfackler Sep 10, 2016

Choose a reason for hiding this comment

eddyb Sep 10, 2016

Choose a reason for hiding this comment

sfackler Sep 10, 2016

Choose a reason for hiding this comment

eddyb commented Sep 13, 2016

bors commented Sep 13, 2016

bors commented Sep 14, 2016

bors commented Sep 14, 2016

alexcrichton commented Sep 14, 2016

bors commented Sep 15, 2016

bors commented Sep 15, 2016

bluss commented Sep 24, 2016

alexcrichton commented Sep 26, 2016

matthew-piziak commented Oct 8, 2016

eddyb Oct 8, 2016

Choose a reason for hiding this comment

alexcrichton commented Oct 11, 2016

matthew-piziak commented Oct 23, 2016

alexcrichton commented Oct 25, 2016

alexcrichton commented Oct 31, 2016

steveklabnik commented Nov 7, 2016

eddyb commented Nov 7, 2016

bors commented Nov 7, 2016

bors commented Nov 7, 2016

bors commented Nov 7, 2016

fix silent overflows on `Step` impls #36365

fix silent overflows on `Step` impls #36365