Skip to content

Fix inconsistent Clone documentation. #57125

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

Merged
merged 1 commit into from
Jan 1, 2019

Conversation

doitian
Copy link
Contributor

@doitian doitian commented Dec 26, 2018

Now, arrays of any size Clone if the element type is Clone. So remove the
the document that uses this as an example.

refs #57123

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 26, 2018
@doitian
Copy link
Contributor Author

doitian commented Dec 26, 2018

I'm not sure this is the best way to resolve the inconsistency. I cannot find out another example that the type is Copy but has to implement Clone manually.

However, simply removing the example leaves the section How can I implement Clone? strange. Maybe we need to add a simple comment such as

It is enough to derive both Copy and Clone in most conditions.

@doitian doitian force-pushed the inconsistent-clone-doc branch from 0bacada to 0fa0587 Compare December 26, 2018 07:19
@doitian
Copy link
Contributor Author

doitian commented Dec 26, 2018

I found an example from #26925 , which has been added in this the PR to replace the original example.

@doitian doitian force-pushed the inconsistent-clone-doc branch from 0fa0587 to a716dfc Compare December 26, 2018 07:24
@rust-highfive
Copy link
Contributor

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:08f3fb90:start=1545809205949295055,finish=1545809207014223774,duration=1064928719
$ 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-6.0
---
[00:02:52] tidy error: /checkout/src/libcore/clone.rs:93: trailing whitespace
[00:02:54] some tidy checks failed
[00:02:54] 
[00:02:54] 
[00:02:54] 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:02:54] 
[00:02:54] 
[00:02:54] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:02:54] Build completed unsuccessfully in 0:00:46
[00:02:54] Build completed unsuccessfully in 0:00:46
[00:02:54] make: *** [tidy] Error 1
[00:02:54] Makefile:79: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:20d8d21c
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Dec 26 07:29:50 UTC 2018
---
travis_time:end:27a0de38:start=1545809390464448156,finish=1545809390470681490,duration=6233334
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:21334423
$ 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:04993f5f
travis_time:start:04993f5f
$ 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:091521a2
$ 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)

@doitian doitian force-pushed the inconsistent-clone-doc branch from a716dfc to 2a79aee Compare December 26, 2018 08:20
@doitian doitian force-pushed the inconsistent-clone-doc branch from 2a79aee to 8acb353 Compare December 27, 2018 01:59
/// library only implements `Clone` up until arrays of size 32. In this case, the implementation of
/// `Clone` cannot be `derive`d, but can be implemented as:
/// An example is a generic struct holding a function pointer. In this case, the
/// implementation of `Clone` cannot be `derive`d, but can be implemented as:
Copy link
Contributor

Choose a reason for hiding this comment

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

Compound words from quoted and non-quoted parts, like derived, look weird. (I realize it's part of the text that is not being changed here, and there is another instance just before the added example above.)

Copy link
Member

Choose a reason for hiding this comment

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

there doesn't seem to be a reason to markup derive like that IMO

Use function pointer as the example to demonstrate how to implement Clone for
Copy types.

refs rust-lang#57123
@doitian doitian force-pushed the inconsistent-clone-doc branch from 8acb353 to bbc8c93 Compare December 31, 2018 13:23
@bluss
Copy link
Member

bluss commented Dec 31, 2018

@bors r+ rollup

Thanks!

@bors
Copy link
Collaborator

bors commented Dec 31, 2018

📌 Commit bbc8c93 has been approved by bluss

@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 Dec 31, 2018
@bors
Copy link
Collaborator

bors commented Dec 31, 2018

⌛ Testing commit bbc8c93 with merge f35beb3...

bors added a commit that referenced this pull request Dec 31, 2018
Fix inconsistent Clone documentation.

Now, arrays of any size Clone if the element type is Clone. So remove the
the document that uses this as an example.

refs #57123
@bors
Copy link
Collaborator

bors commented Dec 31, 2018

💔 Test failed - status-travis

@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 Dec 31, 2018
@kennytm
Copy link
Member

kennytm commented Jan 1, 2019

@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 Jan 1, 2019
@bors
Copy link
Collaborator

bors commented Jan 1, 2019

⌛ Testing commit bbc8c93 with merge b2b7a06...

bors added a commit that referenced this pull request Jan 1, 2019
Fix inconsistent Clone documentation.

Now, arrays of any size Clone if the element type is Clone. So remove the
the document that uses this as an example.

refs #57123
@bors
Copy link
Collaborator

bors commented Jan 1, 2019

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

@bors bors merged commit bbc8c93 into rust-lang:master Jan 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants