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

mark std::str::replace(,n) as #[must_use] #50177

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

matthiaskrgr
Copy link
Member

@matthiaskrgr matthiaskrgr commented Apr 23, 2018

let x = "a b c c";
x.replacen("c", "d", 2");
might not do what people might think it does.

@rust-highfive
Copy link
Collaborator

r? @aidanhs

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 23, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Apr 23, 2018

The same reasoning applies to the replace function. Doesn't it?

@@ -247,6 +247,7 @@ impl str {
/// assert_eq!(s, s.replacen("cookie monster", "little lamb", 10));
/// ```
#[stable(feature = "str_replacen", since = "1.16.0")]
#[must_use]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for consistency, the #[must_use] attribute usually comes before the stability attribute.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will update. I'll wait with that + the replace() must_use attr until travis is done, in case this introduces bootstrap compiler warnings.

@matthiaskrgr matthiaskrgr changed the title mark std::str::replacen as #[must_use] mark std::str::replace(,n) as #[must_use] Apr 23, 2018
@@ -37,6 +37,7 @@
// Many of the usings in this module are only used in the test configuration.
// It's cleaner to just turn off the unused_imports warning than to fix them.
#![allow(unused_imports)]
#![feature(fn_must_use)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. I hoped this would get rid of

[00:24:20] warning: `#[must_use]` on methods is experimental (see issue #43302)
[00:24:20]    --> liballoc/str.rs:211:5
[00:24:20]     |
[00:24:20] 211 |     #[must_use]
[00:24:20]     |     ^^^^^^^^^^^
[00:24:20]     |
[00:24:20]     = help: add #![feature(fn_must_use)] to the crate attributes to enable
[00:24:20]
[00:24:20] warning: `#[must_use]` on methods is experimental (see issue #43302)
[00:24:20]    --> liballoc/str.rs:251:5
[00:24:20]     |
[00:24:20] 251 |     #[must_use]
[00:24:20]     |     ^^^^^^^^^^^
[00:24:20]     |
[00:24:20]     = help: add #![feature(fn_must_use)] to the crate attributes to enable
[00:24:20]

but apparently it does't. What am I doing wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add #![feature(fn_must_use)] to src/liballoc/lib.rs instead.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Apr 24, 2018

📌 Commit 5d37ba1 has been approved by oli-obk

@bors 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 Apr 24, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2018

@bors rollup

@bors
Copy link
Contributor

bors commented Apr 25, 2018

⌛ Testing commit 5d37ba1 with merge 765cd68743868b2c9568307cf43d5df96dc361e8...

@bors
Copy link
Contributor

bors commented Apr 25, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 25, 2018
@rust-highfive
Copy link
Collaborator

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:01:26]  Downloading cc v1.0.10
[00:01:26] error: unable to get packages from source
[00:01:26] 
[00:01:26] Caused by:
[00:01:26]   failed to get 200 response from `https://crates.io/api/v1/crates/cc/1.0.10/download`, got 404
[00:01:26] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:26] Build completed unsuccessfully in 0:00:36
[00:01:26] Makefile:81: recipe for target 'prepare' failed
[00:01:26] make: *** [prepare] Error 1
[00:01:27]  Downloading lazy_static v0.2.11
[00:01:27] error: unable to get packages from source
[00:01:27] 
[00:01:27] Caused by:
[00:01:27] Caused by:
[00:01:27]   failed to get 200 response from `https://crates.io/api/v1/crates/lazy_static/0.2.11/download`, got 404
[00:01:27] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:27] Build completed unsuccessfully in 0:00:00
[00:01:27] Makefile:81: recipe for target 'prepare' failed
[00:01:27] make: *** [prepare] Error 1
[00:01:27]  Downloading serde v1.0.40
[00:01:28] error: unable to get packages from source
[00:01:28] 
[00:01:28] Caused by:
[00:01:28] Caused by:
[00:01:28]   failed to get 200 response from `https://crates.io/api/v1/crates/serde/1.0.40/download`, got 404
[00:01:28] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:28] Build completed unsuccessfully in 0:00:00
[00:01:28] make: *** [prepare] Error 1
[00:01:28] Makefile:81: recipe for target 'prepare' failed
[00:01:28]  Downloading cmake v0.1.30
[00:01:28] warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/cmake/0.1.30/download`, got 500
[00:01:28] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/cmake/0.1.30/download`, got 500
[00:01:28] error: unable to get packages from source
[00:01:28] error: unable to get packages from source
[00:01:28] 
[00:01:28] Caused by:
[00:01:28]   failed to get 200 response from `https://crates.io/api/v1/crates/cmake/0.1.30/download`, got 500
[00:01:28] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:28] Build completed unsuccessfully in 0:00:00
[00:01:28] Makefile:81: recipe for target 'prepare' failed
[00:01:28] make: *** [prepare] Error 1
[00:01:29]  Downloading toml v0.4.6
[00:01:29] error: unable to get packages from source
[00:01:29] 
[00:01:29] Caused by:
[00:01:29] Caused by:
[00:01:29]   failed to get 200 response from `https://crates.io/api/v1/crates/toml/0.4.6/download`, got 404
[00:01:29] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:29] Build completed unsuccessfully in 0:00:00
[00:01:29] make: *** [prepare] Error 1
[00:01:29] Makefile:81: recipe for target 'prepare' failed
[00:01:29] The command has failed after 5 attempts.

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 1.
travis_time:start:0d1f1954
$ 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)

@nagbot-rs
Copy link

nagbot-rs commented Apr 25, 2018 via email

@bors
Copy link
Contributor

bors commented Apr 25, 2018

@nagbot-rs: 🔑 Insufficient privileges: not in try users

@aturon
Copy link
Member

aturon commented Apr 25, 2018

@bors: retry

@bors 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 Apr 25, 2018
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 26, 2018
…_use, r=oli-obk

mark std::str::replace(,n) as #[must_use]

let x = "a b c c";
x.replacen("c", "d", 2");
might not do what people might think it does.
bors added a commit that referenced this pull request Apr 26, 2018
Rollup of 4 pull requests

Successful merges:

 - #50177 (mark std::str::replace(,n) as #[must_use])
 - #50207 (Hash EntryKind::AssociatedConst const data)
 - #50214 (Js improvements)
 - #50219 (Added missing `.` in docs.)

Failed merges:

 - #50229 (Add setting to go to item if there is only one result)
@bors
Copy link
Contributor

bors commented Apr 26, 2018

☔ The latest upstream changes (presumably #50245) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 26, 2018
@bors bors merged commit 5d37ba1 into rust-lang:master Apr 26, 2018
@matthiaskrgr matthiaskrgr deleted the std_std_replacen__must_use branch April 29, 2018 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants