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.

Sign up for GitHub

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

Jump to bottom

Use drop_in_place in array::IntoIter::drop #65821

Merged
merged 1 commit into from
Nov 19, 2019
Merged

Use drop_in_place in array::IntoIter::drop #65821

merged 1 commit into from
Nov 19, 2019

Conversation

SimonSapin
Copy link
Contributor

This skips the loop when the element type is known not to have drop glue, even in debug mode.

@rust-highfive
Copy link
Collaborator

r? @Kimundi

(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 Oct 25, 2019
@JohnCSimon
Copy link
Member

Ping from triage.
@Kimundi can you please review this PR?
@SimonSapin
Thank you!

@JohnCSimon
Copy link
Member

Pinging again from triage.
@Kimundi can you please review this PR?
@SimonSapin
Thank you!

@Dylan-DPC-zz
Copy link

r? @Amanieu

@rust-highfive rust-highfive assigned Amanieu and unassigned Kimundi Nov 12, 2019
@Amanieu
Copy link
Member

Amanieu commented Nov 13, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Nov 13, 2019

📌 Commit f4c59b3 has been approved by Amanieu

@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 Nov 13, 2019
@bors
Copy link
Contributor

bors commented Nov 13, 2019

⌛ Testing commit f4c59b3 with merge 3ee3c488084427b19d5091a186ac7304b1029a0a...

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 13, 2019
@JohnTitor
Rollup merge of rust-lang#65821 - SimonSapin:in-place, r=Amanieu
c309266 
Use `drop_in_place` in `array::IntoIter::drop`

This skips the loop when the element type is known not to have drop glue, even in debug mode.
@JohnTitor JohnTitor mentioned this pull request Nov 13, 2019
Closed
@JohnTitor
Copy link
Member

@bors retry rolled up

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, 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. 
2019-11-13T12:59:48.5966248Z [RUSTC-TIMING] tidy test:false 0.709
2019-11-13T12:59:48.5981317Z     Finished release [optimized] target(s) in 1m 28s
2019-11-13T12:59:48.6091992Z [TIMING] ToolBuild { compiler: Compiler { stage: 0, host: "x86_64-unknown-linux-gnu" }, target: "x86_64-unknown-linux-gnu", tool: "tidy", path: "src/tools/tidy", mode: ToolBootstrap, is_optional_tool: false, source_type: InTree, extra_features: [] } -- 88.083
2019-11-13T12:59:48.6092258Z tidy check
2019-11-13T12:59:49.4639579Z tidy error: /checkout/src/libcore/array/iter.rs:100: undocumented unsafe
2019-11-13T12:59:49.4639811Z tidy error: /checkout/src/libcore/array/iter.rs:196: undocumented unsafe
2019-11-13T12:59:51.3273113Z Found 485 error codes
2019-11-13T12:59:51.3273854Z Found 0 error codes with no tests
2019-11-13T12:59:51.3274072Z Done!
2019-11-13T12:59:51.3274298Z some tidy checks failed
2019-11-13T12:59:51.3274298Z some tidy checks failed
2019-11-13T12:59:51.3274789Z 
2019-11-13T12:59:51.3274960Z 
2019-11-13T12:59:51.3276182Z 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"
2019-11-13T12:59:51.3277454Z 
2019-11-13T12:59:51.3277593Z 
2019-11-13T12:59:51.3277876Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-11-13T12:59:51.3278146Z Build completed unsuccessfully in 0:01:32
2019-11-13T12:59:51.3278146Z Build completed unsuccessfully in 0:01:32
2019-11-13T12:59:51.3328903Z == clock drift check ==
2019-11-13T12:59:51.3338838Z   local time: Wed Nov 13 12:59:51 UTC 2019
2019-11-13T12:59:51.4176767Z   network time: Wed, 13 Nov 2019 12:59:51 GMT
2019-11-13T12:59:51.4177824Z == end clock drift check ==
2019-11-13T12:59:52.7778489Z 
2019-11-13T12:59:52.7886089Z ##[error]Bash exited with code '1'.
2019-11-13T12:59:52.7918766Z ##[section]Starting: Checkout
2019-11-13T12:59:52.7920716Z ==============================================================================
2019-11-13T12:59:52.7920832Z Task         : Get sources
2019-11-13T12:59:52.7920937Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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 13, 2019

⌛ Testing commit f4c59b3 with merge b6b6d352012f5c6c7da0adedfb493a0a4bc9c47d...

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, 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. 
2019-11-13T13:15:27.4676662Z [RUSTC-TIMING] tidy test:false 0.657
2019-11-13T13:15:27.4693864Z     Finished release [optimized] target(s) in 1m 26s
2019-11-13T13:15:27.4964476Z [TIMING] ToolBuild { compiler: Compiler { stage: 0, host: "x86_64-unknown-linux-gnu" }, target: "x86_64-unknown-linux-gnu", tool: "tidy", path: "src/tools/tidy", mode: ToolBootstrap, is_optional_tool: false, source_type: InTree, extra_features: [] } -- 86.122
2019-11-13T13:15:27.4968720Z tidy check
2019-11-13T13:15:28.3628288Z tidy error: /checkout/src/libcore/array/iter.rs:100: undocumented unsafe
2019-11-13T13:15:28.3628544Z tidy error: /checkout/src/libcore/array/iter.rs:196: undocumented unsafe
2019-11-13T13:15:30.1814600Z some tidy checks failed
2019-11-13T13:15:30.1814761Z Found 485 error codes
2019-11-13T13:15:30.1814859Z Found 0 error codes with no tests
2019-11-13T13:15:30.1814939Z Done!
2019-11-13T13:15:30.1814939Z Done!
2019-11-13T13:15:30.1814983Z 
2019-11-13T13:15:30.1815044Z 
2019-11-13T13:15:30.1816315Z 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"
2019-11-13T13:15:30.1816873Z 
2019-11-13T13:15:30.1816930Z 
2019-11-13T13:15:30.1821665Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-11-13T13:15:30.1821809Z Build completed unsuccessfully in 0:01:30
2019-11-13T13:15:30.1821809Z Build completed unsuccessfully in 0:01:30
2019-11-13T13:15:30.1873984Z == clock drift check ==
2019-11-13T13:15:30.1901375Z   local time: Wed Nov 13 13:15:30 UTC 2019
2019-11-13T13:15:30.2592628Z   network time: Wed, 13 Nov 2019 13:15:30 GMT
2019-11-13T13:15:30.2594298Z == end clock drift check ==
2019-11-13T13:15:31.5866540Z 
2019-11-13T13:15:31.5963682Z ##[error]Bash exited with code '1'.
2019-11-13T13:15:31.5996753Z ##[section]Starting: Checkout
2019-11-13T13:15:31.5998681Z ==============================================================================
2019-11-13T13:15:31.5998803Z Task         : Get sources
2019-11-13T13:15:31.5998900Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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 13, 2019

💔 Test failed - checks-azure

@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 Nov 13, 2019
@joelpalmer
Copy link

Ping from Triage: @Amanieu @JohnTitor @SimonSapin any updates?

@JohnTitor
Copy link
Member

tidy check is failed due to undocumented unsafe block

@JohnTitor JohnTitor 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 18, 2019
@SimonSapin
Use drop_in_place in array::IntoIter::drop
74b5714 
This skips the loop when the element type is known not to have drop glue, even in debug mode.
@SimonSapin
Copy link
Contributor Author

Rebased and added SAFETY comments so that tidy passes.

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

Let’s maybe not keep blocking this. One of the new safety comments is copied from an existing similar method, the other seems straight-forward enough that I’ll take the liberty of carrying the review of the comment-less diff:

@bors r=Amanieu

@bors
Copy link
Contributor

bors commented Nov 18, 2019

📌 Commit 74b5714 has been approved by Amanieu

@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 Nov 18, 2019
@bors
Copy link
Contributor

bors commented Nov 19, 2019

⌛ Testing commit 74b5714 with merge 5c5a120...

bors added a commit that referenced this pull request Nov 19, 2019
@bors
Auto merge of #65821 - SimonSapin:in-place, r=Amanieu
5c5a120 
Use `drop_in_place` in `array::IntoIter::drop`

This skips the loop when the element type is known not to have drop glue, even in debug mode.
@bors
Copy link
Contributor

bors commented Nov 19, 2019

☀️ Test successful - checks-azure
Approved by: Amanieu
Pushing 5c5a120 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 19, 2019
@bors bors merged commit 74b5714 into rust-lang:master Nov 19, 2019
@SimonSapin SimonSapin deleted the in-place branch November 28, 2019 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@SimonSapin @rust-highfive @JohnCSimon @Dylan-DPC-zz @Amanieu @bors @JohnTitor @joelpalmer @Kimundi