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

Use SliceIndex for str's Index/IndexMut impls #55683

Closed
wants to merge 3 commits into from

Conversation

sfackler
Copy link
Member

@sfackler sfackler commented Nov 5, 2018

This was missed by accident when the initial implementation landed!

Closes #55603

This was missed by accident when the initial implementation landed!

Closes rust-lang#55603
@alexcrichton
Copy link
Member

@bors: try

Seems ripe for a crater run

@bors
Copy link
Contributor

bors commented Nov 5, 2018

⌛ Trying commit 55beec2 with merge 32bb15d...

bors added a commit that referenced this pull request Nov 5, 2018
Use SliceIndex for str's Index/IndexMut impls

This was missed by accident when the initial implementation landed!

Closes #55603
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:112ada08:start=1541380977566714492,finish=1541380979585902360,duration=2019187868
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---
[00:53:11] .................................................................................................... 4100/4988
[00:53:14] .....................................................................................i.............. 4200/4988
[00:53:20] .................................................................................................... 4300/4988
[00:53:23] .................................................................................................... 4400/4988
[00:53:26] ................................................................................................F..F 4500/4988
[00:53:34] .................................................................................................... 4700/4988
[00:53:37] .................................................................................................... 4800/4988
[00:53:37] .................................................................................................... 4800/4988
        ^^^^ slice indices are of type `usize` or ranges of `usize`
[00:53:42] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:503:22
[00:53:42] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:503:22
[00:53:42] -    = help: the trait `std::ops::Index<{integer}>` is not implemented for `str`
[00:53:42] +    = help: the trait `std::slice::SliceIndex<str>` is not implemented for `{integer}`
[00:53:42] +    = note: required because of the requirements on the impl of `std::ops::Index<{integer}>` for `str`
[00:53:42] 9 error: aborting due to previous error
[00:53:42] 10 
[00:53:42] 
[00:53:42] 
[00:53:42] 
[00:53:42] The actual stderr differed from the expected stderr.
[00:53:42] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/str/str-idx/str-idx.stderr
[00:53:42] To update references, rerun the tests and pass the `--bless` flag
[00:53:42] To only update this specific test, also pass `--test-args str/str-idx.rs`
[00:53:42] error: 1 errors occurred comparing output.
[00:53:42] status: exit code: 1
[00:53:42] status: exit code: 1
[00:53:42] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/str/str-idx.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/str/str-idx/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/str/str-idx/auxiliary" "-A" "unused"
[00:53:42] ------------------------------------------
[00:53:42] 
[00:53:42] 
[00:53:42]en":[],"rendered":null}],"rendered":"error[E0277]: the trait bound `{integer}: std::slice::SliceIndex<str>` is not satisfied\n  --> /checkout/src/test/ui/str/str-idx.rs:13:17\n   |\nLL |     let c: u8 = s[4]; //~ ERROR the type `str` cannot be indexed by `{integer}`\n   |                 ^^^^ slice indices are of type `usize` or ranges of `usize`\n   |\n   = help: the trait `std::slice::SliceIndex<str>` is not implemented for `{integer}`\n   = note: required because of the requirements on the impl of `std::ops::Index<{integer}>` for `str`\n\n"}
[00:53:42] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:53:42] {"message":"For more information about this error, try `rustc --explain E0277`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0277`.\n"}
[00:53:42] ------------------------------------------
[00:53:42] 
[00:53:42] thread '[ui] ui/str/str-idx.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3284:9
[00:53:42] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:53:42] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:53:42] 
[00:53:42] ---- [ui] ui/str/str-mut-idx.rs stdout ----
[00:53:42] diff of stderr:
[00:53:42] 
[00:53:42] 22    = note: to learn more, visit <https://doc.rust-lang.org/book/second-edition/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
[00:53:42] 23    = note: the left-hand-side of an assignment must have a statically known size
[00:53:42] 24 
[00:53:42] - error[E0277]: the type `str` cannot be mutably inde\ntrait Foo {\n    fn bar(&self);\n}\n\n// we now declare a function which takes an object implementing the Foo trait\nfn some_func<T: Foo>(foo: T) {\n    foo.bar();\n}\n\nfn main() {\n    // we now call the method with the i32 type, which doesn't implement\n    // the Foo trait\n    some_func(5i32); // error: the trait bound `i32 : Foo` is not satisfied\n}\n```\n\nIn order to fix this error, verify that the type you're using does implement\nthe trait. Example:\n\n```\ntrait Foo {\n    fn bar(&self);\n}\n\nfn some_func<T: Foo>(foo: T) {\n    foo.bar(); // we can now use this method since i32 implements the\n               // Foo trait\n}\n\n// we implement the trait on the i32 type\nimpl Foo for i32 {\n    fn bar(&self) {}\n}\n\nfn main() {\n    some_func(5i32); // ok!\n}\n```\n\nOr in a generic context, an erroneous code example would look like:\n\n```compile_fail,E0277\nfn some_func<T>(foo: T) {\n    println!(\"{:?}\", foo); // error: the trait `core::fmt::Debug` is not\n                           //        implemented for the type `T`\n}\n\nfn main() {\n    // We now call the method with the i32 type,\n    // which *does* implement the Debug trait.\n    some_func(5i32);\n}\n```\n\nNote that the error here is in the definition of the generic function: Although\nwe only call it with a parameter that does implement `Debug`, the compiler\nstill rejects the function: It must work with all possible input types. In\norder to make this example compile, we need to restrict the generic type we're\naccepting:\n\n```\nuse std::fmt;\n\n// Restrict the input type to types that implement Debug.\nfn some_func<T: fmt::Debug>(foo: T) {\n   inux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib
55708 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release
52788 ./obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/release/deps
51388 ./obj/build/x86_64-unknown-linux-gnu/stage0/bin
50128 ./src/llvm/test/CodeGen/X86

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Nov 5, 2018

☀️ Test successful - status-travis
State: approved= try=True

@sfackler
Copy link
Member Author

sfackler commented Nov 5, 2018

This did expose an issue with the on unimplemented message we have on SliceIndex: https://github.com/rust-lang/rust/blob/master/src/libcore/slice/mod.rs#L2372.

Specifically, strings can't be indexed, just subsliced. This was already a problem since str::get uses SliceIndex currently. Is there better phrasing we can use there, or should we just get rid of the attribute?

@alexcrichton
Copy link
Member

@craterbot run start=master#e6c5cf923489f4aed6c9e36a170fab9b6179d8da end=try#32bb15d09bdec52daf41ad57d2d6c192d7c7a4e9 mode=check-only

@craterbot
Copy link
Collaborator

👌 Experiment pr-55683 created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Nov 5, 2018
@craterbot
Copy link
Collaborator

🚧 Experiment pr-55683 is now running on agent aws-2.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-55683 is completed!
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 7, 2018
@alexcrichton
Copy link
Member

An impressive zero actual regressions! R=me with passing travis

This is unfortunate but since the trait is used by both slices and
strings it was already inaccurate.
@sfackler
Copy link
Member Author

sfackler commented Nov 7, 2018

I went ahead and just removed the custom on_unimplemented attribute for now :(

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:1646e830:start=1541567564427811924,finish=1541567618375964315,duration=53948152391
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---

[00:03:42] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:42] tidy error: /checkout/src/test/ui/str/str-idx.rs:13: line longer than 100 chars
[00:03:44] some tidy checks failed
[00:03:44] 
[00:03:44] 
[00:03:44] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:44] 
[00:03:44] 
[00:03:44] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:44] Build completed unsuccessfully in 0:00:50
[00:03:44] Build completed unsuccessfully in 0:00:50
[00:03:44] Makefile:79: recipe for target 'tidy' failed
[00:03:44] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0447b004
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:03ff229c:start=1541567853513641712,finish=1541567853520465220,duration=6823508
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:038dadfd
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0f0e4ca8
travis_time:start:0f0e4ca8
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:00b07ced
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Mark-Simulacrum
Copy link
Member

@sfackler Looks like CI here is failing, can you take a look?

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2018
@TimNN
Copy link
Contributor

TimNN commented Nov 20, 2018

Ping from triage @sfackler: It looks like your PR is failing on travis.

@TimNN
Copy link
Contributor

TimNN commented Nov 27, 2018

Ping from triage @sfackler !

@TimNN
Copy link
Contributor

TimNN commented Dec 4, 2018

Ping from triage @sfackler: It looks like this PR hasn't received any updates in a while, so I'm closing it per out guidelines. Thank you for your contributions and please feel free to re-open in the future.

@TimNN TimNN closed this Dec 4, 2018
@TimNN TimNN added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants