Deprecating i/u suffixes in libcoretest #21895

alfiedotwtf · 2015-02-03T13:53:30Z

No description provided.

rust-highfive · 2015-02-03T13:53:43Z

(rust_highfive has picked a reviewer for you, use r? to override)

tbu- · 2015-02-03T15:47:55Z

src/libcoretest/any.rs

@@ -18,7 +18,7 @@ static TEST: &'static str = "Test";

 #[test]
 fn any_referenced() {
-    let (a, b, c) = (&5u as &Any, &TEST as &Any, &Test as &Any);
+    let (a, b, c) = (&5 as &Any, &TEST as &Any, &Test as &Any);

    assert!(a.is::<uint>());


That needs more fixing.

This makes the the literal a u32, but later it's checked if it's a uint.

alfiedotwtf · 2015-02-03T20:16:31Z

@tbu- could you please elaborate?

alfiedotwtf · 2015-02-03T21:19:48Z

I think type inference knows it's a uint as 'make check' passed all tests.

tbu- · 2015-02-03T21:24:12Z

Let me claim that this is impossible or a bug in the type inference.

tbu- · 2015-02-03T21:26:46Z

a.rs:

use std::any::Any;
fn main() {
    println!("{}", (&5 as &Any).is::<uint>())
}

Prints:

$ rustc a.rs && ./a
false

tbu- · 2015-02-03T21:27:32Z

If you use make -j <anything greater than 1> check, it can squelch test errors.

alfiedotwtf · 2015-02-03T21:36:58Z

What's the default, as I didn't use -j?

tbu- · 2015-02-03T21:40:40Z

Default should be fine. But have you seen my code snippet that demonstrates that this test cannot succeed?

alfiedotwtf · 2015-02-03T21:43:35Z

Yeah, you're definitely correct. I was just worried about how my tests were passing. And if so, there must be other commits out there that aren't being caught too.

I'll fix my change... but why did make check pass?!

Edit: I'll make check again without the fix and review the output.

alfiedotwtf · 2015-02-03T23:41:50Z

Looking at the output, there is zero mention of libcoretest. So I guess these tests aren't even run with 'make check'?

tbu- · 2015-02-03T23:43:30Z

Mh. They should be.

alfiedotwtf · 2015-02-04T00:07:20Z

Oh, this explains so much :(

cfg: antlr4 not available, skipping lexer test...
cfg: flex not available, skipping parser test...

Installing and running tests again. Sorry for the run around.

alfiedotwtf · 2015-02-04T02:57:51Z

Well this is weird. libcoretests are definately not firing on my local machine. I've made a post on internal:

http://internals.rust-lang.org/t/make-check-not-firing-libcoretest-tests/1534

tbu- · 2015-02-04T11:25:54Z

@Alfie These error messages should have nothing to do with libcoretest. You could try make check-coretest-stage1.

alfiedotwtf · 2015-02-04T12:53:13Z

Yep. I upgraded Valgrind and now something completely unrelated is failing:

test old_io::net::tcp::test::bind_error ... FAILED
test old_io::net::udp::test::bind_error ... FAILED

So back to the drawing board - now that I have all dependencies (flex, bison, antlr4 and valgrind were missing or old), I'm currently running 'make check' on upstream's master to see if there's anything else wrong with my local machine before continuing with this PR.

alfiedotwtf · 2015-02-10T11:29:42Z

@tbu- sorry about the delay. Worked out everything I needed, and was able to identify all the issues. Make check happy now, so this PR is good to go.

tbu- · 2015-02-10T11:34:27Z

src/libcoretest/any.rs

@@ -18,7 +18,7 @@ static TEST: &'static str = "Test";

 #[test]
 fn any_referenced() {
-    let (a, b, c) = (&5u as &Any, &TEST as &Any, &Test as &Any);
+    let (a, b, c) = (&5us as &Any, &TEST as &Any, &Test as &Any);


This should probably be changed to use i32, there's no need for a platform dependent integer type here.

tbu- · 2015-02-10T11:35:05Z

Except for the one note I left, this looks fine.

alfiedotwtf · 2015-02-10T13:13:33Z

Thanks for the suggestion, but I tried modifying as you said (5us to 5i32) and it failed:

failures:

---- any::any_referenced stdout ----
    thread 'any::any_referenced' panicked at 'assertion failed: a.is::<uint>()', /home/alfie/rust/src/libcoretest/any.rs:23

So unless you have any other suggestions, 1f00690 was the last one that passed all tests.

tbu- · 2015-02-10T13:49:44Z

@Alfie

#[test]
fn any_referenced() {
    let (a, b, c) = (&5i32 as &Any, &TEST as &Any, &Test as &Any);
    assert!(a.is::<i32>());
    assert!(!b.is::<i32>());
    assert!(!c.is::<i32>());
    assert!(!a.is::<&'static str>());
    assert!(b.is::<&'static str>());
    assert!(!c.is::<&'static str>());
    assert!(!a.is::<Test>());
    assert!(!b.is::<Test>());
    assert!(c.is::<Test>());
}

I.e. change all the checks for uint to checks for i32, too. Thanks for your endurance!

alfiedotwtf · 2015-02-10T22:18:04Z

@tbu- Ahh I see. I only wanted the bare minimum (so only changing the suffixes), but that one makes sense. Fixed now. Thanks!

alfiedotwtf · 2015-02-12T12:49:39Z

ping @tbu-

pnkfelix · 2015-02-12T13:11:31Z

@bors r+ bffbcb5

alfiedotwtf · 2015-02-12T13:18:36Z

Awesome. Thanks @pnkfelix and @tbu-!

bors · 2015-02-12T14:58:14Z

⌛ Testing commit bffbcb5 with merge cca1cf6...

bors · 2015-02-12T17:12:51Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

tbu- · 2015-02-12T23:39:23Z

Great to see it landed. Thanks for your contribution. :)

rust-highfive assigned brson Feb 3, 2015

tbu- reviewed Feb 3, 2015
View reviewed changes

tbu- reviewed Feb 10, 2015
View reviewed changes

Deprecating i/u suffixes in libcoretest

bffbcb5

bors added a commit that referenced this pull request Feb 12, 2015

Auto merge of #21895 - alfie:libcoretest, r=pnkfelix

cca1cf6

bors merged commit bffbcb5 into rust-lang:master Feb 12, 2015

Deprecating i/u suffixes in libcoretest #21895

Deprecating i/u suffixes in libcoretest #21895

Uh oh!

Conversation

alfiedotwtf commented Feb 3, 2015

Uh oh!

rust-highfive commented Feb 3, 2015

Uh oh!

tbu- Feb 3, 2015

Choose a reason for hiding this comment

Uh oh!

tbu- Feb 3, 2015

Choose a reason for hiding this comment

Uh oh!

alfiedotwtf commented Feb 3, 2015

Uh oh!

alfiedotwtf commented Feb 3, 2015

Uh oh!

tbu- commented Feb 3, 2015

Uh oh!

tbu- commented Feb 3, 2015

Uh oh!

tbu- commented Feb 3, 2015

Uh oh!

alfiedotwtf commented Feb 3, 2015

Uh oh!

tbu- commented Feb 3, 2015

Uh oh!

alfiedotwtf commented Feb 3, 2015

Uh oh!

alfiedotwtf commented Feb 3, 2015

Uh oh!

tbu- commented Feb 3, 2015

Uh oh!

alfiedotwtf commented Feb 4, 2015

Uh oh!

alfiedotwtf commented Feb 4, 2015

Uh oh!

tbu- commented Feb 4, 2015

Uh oh!

alfiedotwtf commented Feb 4, 2015

Uh oh!

alfiedotwtf commented Feb 10, 2015

Uh oh!

tbu- Feb 10, 2015

Choose a reason for hiding this comment

Uh oh!

tbu- commented Feb 10, 2015

Uh oh!

alfiedotwtf commented Feb 10, 2015

Uh oh!

tbu- commented Feb 10, 2015

Uh oh!

alfiedotwtf commented Feb 10, 2015

Uh oh!

alfiedotwtf commented Feb 12, 2015

Uh oh!

pnkfelix commented Feb 12, 2015

Uh oh!

alfiedotwtf commented Feb 12, 2015

Uh oh!

bors commented Feb 12, 2015

Uh oh!

bors commented Feb 12, 2015

Uh oh!

tbu- commented Feb 12, 2015

Uh oh!

Uh oh!