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

Avoid uneccessary clone of Annotatable #91324

Merged

Conversation

eggyal
Copy link
Contributor

@eggyal eggyal commented Nov 28, 2021

Addresses FIXME comment created in #82608

r? @Aaron1011

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2021
@Aaron1011
Copy link
Member

Very clever!

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 28, 2021
@bors
Copy link
Contributor

bors commented Nov 28, 2021

⌛ Trying commit d05e4d2 with merge 753a94276a9dacd5bd9033012456ea133392d9da...

@bors
Copy link
Contributor

bors commented Nov 28, 2021

☀️ Try build successful - checks-actions
Build commit: 753a94276a9dacd5bd9033012456ea133392d9da (753a94276a9dacd5bd9033012456ea133392d9da)

@rust-timer
Copy link
Collaborator

Queued 753a94276a9dacd5bd9033012456ea133392d9da with parent db9d361, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (753a94276a9dacd5bd9033012456ea133392d9da): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 29, 2021
@Aaron1011
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 29, 2021

📌 Commit d05e4d2 has been approved by Aaron1011

@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 29, 2021
@bors
Copy link
Contributor

bors commented Nov 30, 2021

⌛ Testing commit d05e4d2 with merge cd3e911336b2811285fa6f1c6808b9e533b3aada...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Processing message: Object({"id": Number(0), "jsonrpc": String("2.0"), "result": Object({"capabilities": Object({"codeActionProvider": Bool(true), "codeLensProvider": Object({"resolveProvider": Bool(false)}), "completionProvider": Object({"resolveProvider": Bool(true), "triggerCharacters": Array([String("."), String(":")])}), "definitionProvider": Bool(true), "documentFormattingProvider": Bool(true), "documentHighlightProvider": Bool(true), "documentRangeFormattingProvider": Bool(false), "documentSymbolProvider": Bool(true), "executeCommandProvider": Object({"commands": Array([String("rls.applySuggestion-5805"), String("rls.deglobImports-5805")])}), "hoverProvider": Bool(true), "implementationProvider": Bool(true), "referencesProvider": Bool(true), "renameProvider": Bool(true), "textDocumentSync": Number(2), "workspaceSymbolProvider": Bool(true)})})})
Processing message: Object({"jsonrpc": String("2.0"), "method": String("window/progress"), "params": Object({"done": Null, "id": String("progress_1"), "message": Null, "percentage": Null, "title": String("Building")})})
Processing message: Object({"jsonrpc": String("2.0"), "method": String("window/progress"), "params": Object({"done": Bool(true), "id": String("progress_1"), "message": Null, "percentage": Null, "title": String("Building")})})
Processing message: Object({"jsonrpc": String("2.0"), "method": String("window/progress"), "params": Object({"done": Null, "id": String("progress_0"), "message": Null, "percentage": Null, "title": String("Indexing")})})
Processing message: Object({"jsonrpc": String("2.0"), "method": String("textDocument/publishDiagnostics"), "params": Object({"diagnostics": Array([Object({"message": String("failed to load manifest for dependency `member_a`"), "range": Object({"end": Object({"character": Number(0), "line": Number(9999)}), "start": Object({"character": Number(0), "line": Number(0)})}), "severity": Number(1)})]), "uri": String("file:///checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/rlsit/t26/invalid_member_toml/member_a/dodgy_member/Cargo.toml")})})
error: test failed, to rerun pass '--test client'
thread 'client_invalid_member_toml_manifest' panicked at 'called `Result::unwrap()` on an `Err` value: Elapsed(())', src/tools/rls/tests/support/client/mod.rs:267:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Sending: Object({"id": Number(99999), "jsonrpc": String("2.0"), "method": String("shutdown"), "params": Null})
---
Verifying status of rls...
Verifying status of miri...
Verifying status of embedded-book...
Cloning into 'rust-toolstate'...
error: Tool `rls` has regressed from test-pass to test-fail during beta week.
{"cargo-miri":"test-fail","rls":"test-fail","book":"test-pass","embedded-book":"test-pass","nomicon":"test-pass","reference":"test-pass","rust-by-example":"test-pass","edition-guide":"test-pass","miri":"test-pass","rustbook":"test-fail"}Build completed unsuccessfully in 0:00:00

@bors
Copy link
Contributor

bors commented Nov 30, 2021

💔 Test failed - checks-actions

@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 30, 2021
@eggyal
Copy link
Contributor Author

eggyal commented Nov 30, 2021

Well that's... interesting ?!

Spurious failure? Bug in MIRI? UB? Not sure what's going on here, but seems pretty unlikely for this PR to be at fault!

@Aaron1011
Copy link
Member

@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 Nov 30, 2021
@bors
Copy link
Contributor

bors commented Nov 30, 2021

⌛ Testing commit d05e4d2 with merge 2a8a133a2ed4e53b278c343d15d35579efeac881...

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Nov 30, 2021

💔 Test failed - checks-actions

@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 30, 2021
@Aaron1011
Copy link
Member

@bors try

@bors
Copy link
Contributor

bors commented Nov 30, 2021

⌛ Trying commit d05e4d2 with merge 7b97bbc7df1cd184854ae0fc18f6a22f944800f6...

@bors
Copy link
Contributor

bors commented Nov 30, 2021

☀️ Try build successful - checks-actions
Build commit: 7b97bbc7df1cd184854ae0fc18f6a22f944800f6 (7b97bbc7df1cd184854ae0fc18f6a22f944800f6)

@Aaron1011
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Nov 30, 2021

⌛ Trying commit d05e4d2 with merge f049e60b9fedcaf4d463dd92f0ffe7eb9b62f029...

@bors
Copy link
Contributor

bors commented Nov 30, 2021

☀️ Try build successful - checks-actions
Build commit: f049e60b9fedcaf4d463dd92f0ffe7eb9b62f029 (f049e60b9fedcaf4d463dd92f0ffe7eb9b62f029)

@Aaron1011
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 30, 2021

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Nov 30, 2021

📌 Commit d05e4d2 has been approved by Aaron1011

@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 30, 2021
@bors
Copy link
Contributor

bors commented Dec 1, 2021

⌛ Testing commit d05e4d2 with merge 2446a21...

@bors
Copy link
Contributor

bors commented Dec 1, 2021

☀️ Test successful - checks-actions
Approved by: Aaron1011
Pushing 2446a21 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 1, 2021
@bors bors merged commit 2446a21 into rust-lang:master Dec 1, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 1, 2021
@eggyal
Copy link
Contributor Author

eggyal commented Dec 1, 2021

Hooray! But still... all rather suspicious. Is this a known issue?

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2446a21): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@eggyal eggyal deleted the avoid-uneccesary-clone-of-annotatable branch December 1, 2021 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

7 participants