Rename .cap() methods to .capacity() #60340

mgeier · 2019-04-27T19:33:39Z

As mentioned in #60316, there are a few .cap() methods, which seem out-of-place because such methods are called .capacity() in the rest of the code.

This PR renames them to .capacity() but leaves RawVec::cap() in there for backwards compatibility.

I didn't try to mark the old version as "deprecated", because I guess this would cause too much noise.

rust-highfive · 2019-04-27T19:33:49Z

r? @joshtriplett

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

rust-highfive · 2019-04-27T20:33:44Z

The job x86_64-gnu-llvm-6.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:027a071e:start=1556396909133286972,finish=1556396909921163385,duration=787876413
$ 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
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[00:04:23]    Compiling hashbrown v0.3.0
[00:04:30] error: method is never used: `cap`
[00:04:30]    --> src/libstd/sync/mpsc/sync.rs:479:5
[00:04:30]     |
[00:04:30] 479 |     fn cap(&self) -> usize { self.capacity() }  // For backwards compatibility
[00:04:30]     |
[00:04:30]     = note: `-D dead-code` implied by `-D warnings`
[00:04:30] 
[00:04:30] error: aborting due to previous error
---
travis_time:end:335d9f1b:start=1556397191938661426,finish=1556397191944353508,duration=5692082
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04fdf1c0
$ 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:1a9bf30d
travis_time:start:1a9bf30d
$ 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:07f090c2
$ 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)

... but leave the old names in there for backwards compatibility.

mgeier · 2019-04-28T07:00:58Z

Turns out that VecDeque<T> has both methods at the same time: a (public) .capacity() and a (private) .cap() (latter always returns 1 more than former). This is a bit confusing, and probably a potential source for bugs within the implementation.

Mark-Simulacrum · 2019-05-18T14:26:22Z

r? @alexcrichton

We don't need to keep the cap method on RawVec around; RawVec is a private internal type so there's no need to preserve compatibility.

I'm uncertain what to do about the VecDeque double-method (i.e., capacity and cap); I suspect better naming for the internal method might be in order, but it's hard to say exactly what the right approach there is.

alexcrichton · 2019-05-20T14:00:05Z

src/liballoc/raw_vec.rs

        if mem::size_of::<T>() == 0 {
            !0
        } else {
            self.cap
        }
    }

+    // For backwards compatibility
+    #[inline(always)]


Instead of #[inline(always)] could this be #[doc(hidden)] and #[rustc_deprecated]?

In case you are asking me: I'm a total Rust newbie, and I don't know what should be used here.

Just tell me, and I can change the PR. Or feel free to make any changes yourself.

@Mark-Simulacrum mentioned that RawVec is a private internal type and we don't need to keep compatibility?

alexcrichton · 2019-05-20T14:00:33Z

Seems reasonable to me! Just one nit otherwise r=me

mgeier · 2019-05-21T13:12:46Z

Thanks @alexcrichton for the review!

What about the private method VecDeque::cap()?
Shall we rename it to something less ambiguous?

alexcrichton · 2019-05-21T13:49:09Z

I don't personally have a preference one way or another.

Dylan-DPC-zz · 2019-06-03T13:43:43Z

ping from triage @mgeier any updates on this?

jonas-schievink · 2019-06-18T16:03:44Z

Visiting for triage, @mgeier can you address the review comment?

As suggested in rust-lang#60340 (comment)

mgeier · 2019-06-25T13:47:31Z

Sorry for the long delay!

I've removed RawVec::cap() in abe3bdf, as suggested in #60340 (comment).

I couldn't come up with a better name for the internal function VecDeque::cap(), so I've left that unchanged.

alexcrichton · 2019-06-26T06:26:54Z

@bors: r+

Centril · 2019-06-26T10:04:25Z

@bors rollup=never

joelpalmer · 2019-07-22T12:24:34Z

Hi @mgeier - ping from Triage. Closing due to inactivity. Please re-open before pushing changes. Thanks for the PR.

@rustbot modify labels to +S-inactive-closed, -S-waiting-on-author

Dylan-DPC-zz · 2019-07-22T12:27:14Z

@joelpalmer ~~this failed ~~because of another PR~~ in the rollup (check #60340 (comment)) so let's keep it open~~

oops we can close it due to inactivity

mgeier · 2019-07-22T19:06:09Z

@joelpalmer

I have made the requested changes and reported the latest status in #60340 (comment).

What is there left to do for me?

Dylan-DPC-zz · 2019-07-22T19:10:38Z

@mgeier you need to address the failing tests

mgeier · 2019-07-24T10:50:50Z

Thanks @Dylan-DPC, can you please point me to the failing tests?

Dylan-DPC-zz · 2019-07-24T13:54:12Z

@mgeier looks to have been due to a spurious issue.

@alexcrichton can you review this?

alexcrichton · 2019-07-25T14:11:51Z

@bors: r+

bors · 2019-07-25T14:11:53Z

📌 Commit abe3bdf has been approved by alexcrichton

bors · 2019-07-25T18:45:51Z

⌛ Testing commit abe3bdf with merge 890881f...

Rename .cap() methods to .capacity() As mentioned in #60316, there are a few `.cap()` methods, which seem out-of-place because such methods are called `.capacity()` in the rest of the code. This PR renames them to `.capacity()` but leaves `RawVec::cap()` in there for backwards compatibility. I didn't try to mark the old version as "deprecated", because I guess this would cause too much noise.

bors · 2019-07-25T22:34:17Z

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing 890881f to master...

rust-highfive · 2019-07-25T22:35:27Z

📣 Toolstate changed by #60340!

Tested on commit 890881f.
Direct link to PR: #60340

💔 rustfmt on windows: test-pass → build-fail (cc @topecongiro, @rust-lang/infra).
💔 rustfmt on linux: test-pass → build-fail (cc @topecongiro, @rust-lang/infra).

@topecongiro

Tested on commit rust-lang/rust@890881f. Direct link to PR: <rust-lang/rust#60340> 💔 rustfmt on windows: test-pass → build-fail (cc @topecongiro, @rust-lang/infra). 💔 rustfmt on linux: test-pass → build-fail (cc @topecongiro, @rust-lang/infra).

rust-highfive assigned joshtriplett Apr 27, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 27, 2019

This comment has been minimized.

Sign in to view

mgeier force-pushed the cap-vs-capacity branch from 7d8f23d to 6003797 Compare April 27, 2019 19:47

This comment has been minimized.

Sign in to view

mgeier force-pushed the cap-vs-capacity branch 2 times, most recently from c8b7629 to 4d5a405 Compare April 27, 2019 19:59

This comment has been minimized.

Sign in to view

mgeier force-pushed the cap-vs-capacity branch from 4d5a405 to c00f1ef Compare April 27, 2019 20:13

This comment has been minimized.

Sign in to view

mgeier force-pushed the cap-vs-capacity branch from c00f1ef to f8fd539 Compare April 27, 2019 20:27

Rename .cap() methods to .capacity()

0967d28

... but leave the old names in there for backwards compatibility.

mgeier force-pushed the cap-vs-capacity branch from f8fd539 to 0967d28 Compare April 27, 2019 20:43

rust-highfive assigned alexcrichton and unassigned joshtriplett May 18, 2019

alexcrichton reviewed May 20, 2019

View reviewed changes

Dylan-DPC-zz 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 Jun 3, 2019

Remove RawVec::cap()

abe3bdf

As suggested in rust-lang#60340 (comment)

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 26, 2019

rustbot 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 Jul 22, 2019

Dylan-DPC-zz closed this Jul 22, 2019

Dylan-DPC-zz reopened this Jul 22, 2019

Dylan-DPC-zz added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. labels Jul 24, 2019

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 Jul 25, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 25, 2019

bors merged commit abe3bdf into rust-lang:master Jul 25, 2019

This was referenced Jul 25, 2019

Remove helper-methods from Alloc trait and add associated Err type. #60771

Closed

Finalize the error type for try_reserve #61780

Merged

Improve ptr_rotate performance, tests, and benches #61937

Merged

matthiaskrgr mentioned this pull request Jul 26, 2019

fails to build with latest nightly/master rustc rust-lang/rustfmt#3703

Closed

mgeier deleted the cap-vs-capacity branch August 6, 2019 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename .cap() methods to .capacity() #60340

Rename .cap() methods to .capacity() #60340

mgeier commented Apr 27, 2019 •

edited

Loading

rust-highfive commented Apr 27, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-highfive commented Apr 27, 2019

mgeier commented Apr 28, 2019

Mark-Simulacrum commented May 18, 2019

alexcrichton May 20, 2019

mgeier May 21, 2019 •

edited

Loading

alexcrichton commented May 20, 2019

mgeier commented May 21, 2019

alexcrichton commented May 21, 2019

Dylan-DPC-zz commented Jun 3, 2019

jonas-schievink commented Jun 18, 2019

mgeier commented Jun 25, 2019

alexcrichton commented Jun 26, 2019

Centril commented Jun 26, 2019

joelpalmer commented Jul 22, 2019

Dylan-DPC-zz commented Jul 22, 2019 •

edited

Loading

mgeier commented Jul 22, 2019

Dylan-DPC-zz commented Jul 22, 2019

mgeier commented Jul 24, 2019

Dylan-DPC-zz commented Jul 24, 2019

alexcrichton commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

rust-highfive commented Jul 25, 2019

Rename .cap() methods to .capacity() #60340

Rename .cap() methods to .capacity() #60340

Conversation

mgeier commented Apr 27, 2019 • edited Loading

rust-highfive commented Apr 27, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-highfive commented Apr 27, 2019

mgeier commented Apr 28, 2019

Mark-Simulacrum commented May 18, 2019

alexcrichton May 20, 2019

Choose a reason for hiding this comment

mgeier May 21, 2019 • edited Loading

Choose a reason for hiding this comment

alexcrichton commented May 20, 2019

mgeier commented May 21, 2019

alexcrichton commented May 21, 2019

Dylan-DPC-zz commented Jun 3, 2019

jonas-schievink commented Jun 18, 2019

mgeier commented Jun 25, 2019

alexcrichton commented Jun 26, 2019

Centril commented Jun 26, 2019

joelpalmer commented Jul 22, 2019

Dylan-DPC-zz commented Jul 22, 2019 • edited Loading

mgeier commented Jul 22, 2019

Dylan-DPC-zz commented Jul 22, 2019

mgeier commented Jul 24, 2019

Dylan-DPC-zz commented Jul 24, 2019

alexcrichton commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

rust-highfive commented Jul 25, 2019

mgeier commented Apr 27, 2019 •

edited

Loading

mgeier May 21, 2019 •

edited

Loading

Dylan-DPC-zz commented Jul 22, 2019 •

edited

Loading