Use String, not &str in some collection examples #51081

kornelski · 2018-05-26T11:29:06Z

Discussed in #46966

Overuse of borrowed values in data structures is a common mistake I see in Rust user forums. Users who copy&paste such examples end up fighting with the borrow checker as soon as they replace string literals with some real values.

This changes a couple of examples to use String, and it adds opportunity to demonstrate use of Borrow.

rust-highfive · 2018-05-26T11:29:16Z

r? @sfackler

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

rust-highfive · 2018-05-26T12:48:24Z

The job x86_64-gnu-llvm-3.9 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.

[00:45:51] ..........................................................i.........................................
[00:45:55] ....................................................................................................
[00:46:01] ....................................................................................................
[00:46:08] .....................................................................................i..............
[00:46:10] ...iiiiiiiii...................................................
[00:46:10] 
[00:46:10] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:46:59] ..........................................................i.........................................
[00:47:04] ....................................................................................................
[00:47:09] ....................................................................................................
[00:47:15] .....................................................................................i..............
[00:47:18] ...iiiiiiiii...................................................
[00:47:18] 
[00:47:18]  finished in 67.848
[00:47:18] travis_fold:end:test_ui_nll

---
[01:15:23] 
[01:15:23]    Doc-tests std
[01:15:25] 
[01:15:25] running 926 tests
[01:15:57] ii......F...........................................................................................
[01:16:26] ...i......i...i......i..............................................................................
[01:16:34] ....................................................................................................
[01:16:51] ................iiii........ii......................................................................
[01:16:59] ....................................................................................................
[01:16:59] ....................................................................................................
[01:17:15] ...............................................................iiii.................................
[01:17:38] ....................................................................................................
[01:17:48] ...........................................................................iiii.....................
[01:17:52] ..........................
[01:17:52] failures:
[01:17:52] 
[01:17:52] ---- collections/hash/map.rs - collections::hash::map::HashMap (line 274) stdout ----
[01:17:52] error[E0277]: the trait bound `std::string::String: std::borrow::Borrow<&str>` is not satisfied
[01:17:52]   --> collections/hash/map.rs:305:24
[01:17:52]    |
[01:17:52] 34 |     match book_reviews.get(book) {
[01:17:52]    |                        ^^^ the trait `std::borrow::Borrow<&str>` is not implemented for `std::string::String`
[01:17:52]    |
[01:17:52]    = help: the following implementations were found:
[01:17:52]              <std::string::String as std::borrow::Borrow<str>>
[01:17:52] thread 'collections/hash/map.rs - collections::hash::map::HashMap (line 274)' panicked at 'couldn't compile the test', librustdoc/test.rs:325:17
[01:17:52] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:17:52] 
[01:17:52] 
---
[01:17:52] 
[01:17:52] error: test failed, to rerun pass '--doc'
[01:17:52] 
[01:17:52] 
[01:17:52] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "-p" "std" "--" "--quiet"
[01:17:52] 
[01:17:52] 
[01:17:52] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:17:52] Build completed unsuccessfully in 0:34:22
[01:17:52] Build completed unsuccessfully in 0:34:22
[01:17:52] Makefile:58: recipe for target 'check' failed
[01:17:52] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:015afa76
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

ishitatsuyuki · 2018-05-26T13:54:10Z

src/libstd/collections/hash/map.rs

-/// book_reviews.insert("Pride and Prejudice",               "Very enjoyable.");
-/// book_reviews.insert("The Adventures of Sherlock Holmes", "Eye lyked it alot.");
+/// // Review some books.
+/// book_reviews.insert(String::from("Adventures of Huckleberry Finn"),


.to_owned() is shorter and IMO more expressive.

I've used .to_owned() 24eb6069b461fcc337507d0ec0c76db1b1d2c3fb. I'm taking cue from WHATWG here — the HTML5 spec deliberately uses different styles across examples, which gives a broader view of the language.

The standard style, which the docs follow, is to use .to_string() everywhere.

sfackler · 2018-05-26T15:49:45Z

r? @GuillaumeGomez

GuillaumeGomez · 2018-05-26T21:58:20Z

I'm still against it so I'll set another reviewer.

r? @steveklabnik

shepmaster · 2018-06-02T15:01:18Z

Ping from triage @steveklabnik !

steveklabnik

r=me after every String creation is changed to .to_string. Thanks a ton!

steveklabnik · 2018-06-05T09:12:43Z

src/libstd/collections/hash/map.rs

-/// book_reviews.insert("Pride and Prejudice",               "Very enjoyable.");
-/// book_reviews.insert("The Adventures of Sherlock Holmes", "Eye lyked it alot.");
+/// // Review some books.
+/// book_reviews.insert(String::from("Adventures of Huckleberry Finn"),


The standard style, which the docs follow, is to use .to_string() everywhere.

steveklabnik

One last thing I forgot, sorry. The previous formatting wasn't correct, and so this formatting isn't either. rustfmt does

    book_reviews.insert(
        "Adventures of Huckleberry Finn".to_string(),
        "My favorite book.".to_string(),
    );

could you fix that as well? r=me after. sorry :(

kornelski · 2018-06-05T16:10:30Z

OK, formatted

pietroalbini · 2018-06-18T09:11:53Z

Ping from triage! This PR needs a review, can @steveklabnik or someone else from @rust-lang/docs review this?

GuillaumeGomez · 2018-06-18T10:47:19Z

Basically, only a squash is needed and then it seems to be ready.

kornelski · 2018-06-18T11:41:11Z

Squashed

steveklabnik · 2018-06-18T15:04:27Z

Thanks!

@bors: r+ rollup

bors · 2018-06-18T15:04:29Z

📌 Commit 01e82f1 has been approved by steveklabnik

bors · 2018-06-19T13:09:10Z

⌛ Testing commit 01e82f1 with merge a646c91...

Use String, not &str in some collection examples Discussed in #46966 Overuse of borrowed values in data structures is a common mistake I see in Rust user forums. Users who copy&paste such examples end up fighting with the borrow checker as soon as they replace string literals with some real values. This changes a couple of examples to use `String`, and it adds opportunity to demonstrate use of `Borrow`.

bors · 2018-06-19T15:05:12Z

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing a646c91 to master...

rust-highfive assigned sfackler May 26, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 26, 2018

ishitatsuyuki reviewed May 26, 2018

View reviewed changes

kornelski force-pushed the examplestr branch from 03f3387 to 24eb606 Compare May 26, 2018 15:11

rust-highfive assigned GuillaumeGomez and unassigned sfackler May 26, 2018

rust-highfive assigned steveklabnik and unassigned GuillaumeGomez May 26, 2018

steveklabnik suggested changes Jun 5, 2018

View reviewed changes

kornelski force-pushed the examplestr branch from 24eb606 to bd6ce1d Compare June 5, 2018 10:02

steveklabnik suggested changes Jun 5, 2018

View reviewed changes

kornelski force-pushed the examplestr branch from bd6ce1d to 61fd485 Compare June 5, 2018 16:10

pietroalbini added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Jun 18, 2018

Prefer use of owned values in examples

01e82f1

kornelski force-pushed the examplestr branch from 61fd485 to 01e82f1 Compare June 18, 2018 11:39

steveklabnik approved these changes Jun 18, 2018

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2018

bors merged commit 01e82f1 into rust-lang:master Jun 19, 2018

kornelski deleted the examplestr branch June 19, 2018 15:08

Use String, not &str in some collection examples #51081

Use String, not &str in some collection examples #51081

Uh oh!

Conversation

kornelski commented May 26, 2018

Uh oh!

rust-highfive commented May 26, 2018

Uh oh!

rust-highfive commented May 26, 2018

Uh oh!

ishitatsuyuki May 26, 2018

Choose a reason for hiding this comment

Uh oh!

kornelski May 26, 2018

Choose a reason for hiding this comment

Uh oh!

steveklabnik Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

sfackler commented May 26, 2018

Uh oh!

GuillaumeGomez commented May 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shepmaster commented Jun 2, 2018

Uh oh!

steveklabnik left a comment

Choose a reason for hiding this comment

Uh oh!

steveklabnik Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

steveklabnik left a comment

Choose a reason for hiding this comment

Uh oh!

kornelski commented Jun 5, 2018

Uh oh!

pietroalbini commented Jun 18, 2018

Uh oh!

GuillaumeGomez commented Jun 18, 2018

Uh oh!

kornelski commented Jun 18, 2018

Uh oh!

steveklabnik commented Jun 18, 2018

Uh oh!

bors commented Jun 18, 2018

Uh oh!

bors commented Jun 19, 2018

Uh oh!

bors commented Jun 19, 2018

Uh oh!

Uh oh!

GuillaumeGomez commented May 26, 2018 •

edited

Loading