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

x.py bench --stage 0 src/libcore fails with deprecation errors #61816

Closed
AaronKutch opened this issue Jun 13, 2019 · 3 comments · Fixed by #61844
Closed

x.py bench --stage 0 src/libcore fails with deprecation errors #61816

AaronKutch opened this issue Jun 13, 2019 · 3 comments · Fixed by #61844
Labels
C-bug Category: This is a bug. C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@AaronKutch
Copy link
Contributor

I would like to benchmark a specific subset of all the benchmarks, but adding src/libcore onto the command makes it fail:

$ ./x.py bench --stage 0 src/libcore
Updating only changed submodules
Submodules updated in 0.59 seconds
    Finished dev [unoptimized] target(s) in 0.37s
Building stage0 std artifacts (x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu)
    Finished release [optimized] target(s) in 4.00s
Copying stage0 std from stage0 (x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu / x86_64-pc-windows-gnu)
Building stage0 test artifacts (x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu)
    Finished release [optimized] target(s) in 0.56s
Copying stage0 test from stage0 (x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu / x86_64-pc-windows-gnu)
Benchmarking core stage0 (x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu)
   Compiling core v0.0.0 (C:\Users\Allen\Documents\GitHub\rust\src\libcore)
error: `...` range patterns are deprecated
   --> src\libcore\../libcore/benches\ascii.rs:194:21
    |
194 |                 b'a'...b'z' => true,
    |                     ^^^ help: use `..=` for an inclusive range
    |
    = note: `-D ellipsis-inclusive-range-patterns` implied by `-D rust-2018-idioms`

error: `...` range patterns are deprecated
   --> src\libcore\../libcore/benches\ascii.rs:206:21
    |
206 |                 b'a'...b'z' => true,
    |                     ^^^ help: use `..=` for an inclusive range

error: `...` range patterns are deprecated
   --> src\libcore\../libcore/benches\ascii.rs:218:21
    |
218 |                 b'a'...b'z' => true,
    |                     ^^^ help: use `..=` for an inclusive range

error: `...` range patterns are deprecated
   --> src\libcore\../libcore/benches\ascii.rs:230:21
    |
230 |                 b'a'...b'z' => true,
    |                     ^^^ help: use `..=` for an inclusive range

error: `...` range patterns are deprecated
   --> src\libcore\../libcore/benches\ascii.rs:194:21
    |
194 |                 b'a'...b'z' => true,
    |                     ^^^ help: use `..=` for an inclusive range

error: aborting due to 5 previous errors

error: Could not compile `core`.

To learn more, run the command again with --verbose.


command did not execute successfully: "C:\\Users\\Allen\\Documents\\GitHub\\rust\\build\\x86_64-pc-windows-gnu\\stage0\\bin\\cargo.exe" "bench" "--target" "x86_64-pc-windows-gnu" "-j" "4" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "C:\\Users\\Allen\\Documents\\GitHub\\rust\\src/libstd/Cargo.toml" "-p" "core" "--" "--quiet"
expected success, got: exit code: 101


        finished in 8.642

1 command(s) did not execute successfully:

  - "C:\\Users\\Allen\\Documents\\GitHub\\rust\\build\\x86_64-pc-windows-gnu\\stage0\\bin\\cargo.exe" "bench" "--target" "x86_64-pc-windows-gnu" "-j" "4" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "C:\\Users\\Allen\\Documents\\GitHub\\rust\\src/libstd/Cargo.toml" "-p" "core" "--" "--quiet"
@AaronKutch
Copy link
Contributor Author

Also, I should mention that this occurs even after running x.py clean

@Centril
Copy link
Contributor

Centril commented Jun 14, 2019

@AaronKutch Could you file a PR with ..= instead of ...?

@Centril Centril added C-bug Category: This is a bug. C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 14, 2019
@ehuss
Copy link
Contributor

ehuss commented Jun 14, 2019

I was curious why this hadn't been noticed earlier. Apparently benchmarks aren't tested on CI to verify that they still build. This libcore benchmark has been broken since #60133. I'm quite surprised nobody has run x.py bench src/libcore since then.

Another wrinkle that confused me is that ./x.py test --stage=0 --no-doc src/libcore also fails. Why does this command build benchmarks? Well this little snippet makes --no-doc also enable benchmarks. I suspect that was a mistake, or maybe the mistake is that benchmarks aren't always tested?

The libs team may want to consider possibly testing benchmarks in CI to ensure that they stay functional. libcore at least only takes a few seconds to build and run its benchmarks in test mode (one iteration).

@jonas-schievink jonas-schievink added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Jun 14, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 14, 2019
Change `...` to `..=` where applicable

This is mainly to fix rust-lang#61816, but I decided to manually check a few thousand `...` throughout the code base to check for any other cases. I think I found a documentation bug in `src\libsyntax\ast.rs` where both `1..` and `1...` where mentioned. If there is internal support for both `1..` and `1..=` (that can exist before error handling gets to it), then I can add that back.
There were some other cases that look like `// struct Closure<'l0...'li, T0...Tj, CK, CS, U0...Uk> {`, `// <P0 as Trait<P1...Pn>>::Foo: 'a`, and `assert!(min <= max, "discriminant range is {}...{}", min, max);`, but I am not sure if I should change those.
There are a bunch of cases in the `/test/` directory that could be changed, but I presume I should just leave those be.
Centril added a commit to Centril/rust that referenced this issue Jun 14, 2019
Change `...` to `..=` where applicable

This is mainly to fix rust-lang#61816, but I decided to manually check a few thousand `...` throughout the code base to check for any other cases. I think I found a documentation bug in `src\libsyntax\ast.rs` where both `1..` and `1...` where mentioned. If there is internal support for both `1..` and `1..=` (that can exist before error handling gets to it), then I can add that back.
There were some other cases that look like `// struct Closure<'l0...'li, T0...Tj, CK, CS, U0...Uk> {`, `// <P0 as Trait<P1...Pn>>::Foo: 'a`, and `assert!(min <= max, "discriminant range is {}...{}", min, max);`, but I am not sure if I should change those.
There are a bunch of cases in the `/test/` directory that could be changed, but I presume I should just leave those be.
Centril added a commit to Centril/rust that referenced this issue Jun 15, 2019
Change `...` to `..=` where applicable

This is mainly to fix rust-lang#61816, but I decided to manually check a few thousand `...` throughout the code base to check for any other cases. I think I found a documentation bug in `src\libsyntax\ast.rs` where both `1..` and `1...` where mentioned. If there is internal support for both `1..` and `1..=` (that can exist before error handling gets to it), then I can add that back.
There were some other cases that look like `// struct Closure<'l0...'li, T0...Tj, CK, CS, U0...Uk> {`, `// <P0 as Trait<P1...Pn>>::Foo: 'a`, and `assert!(min <= max, "discriminant range is {}...{}", min, max);`, but I am not sure if I should change those.
There are a bunch of cases in the `/test/` directory that could be changed, but I presume I should just leave those be.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants