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

core::any: replace some generic types with impl Trait #98912

Merged
merged 1 commit into from
Jul 19, 2022
Merged

core::any: replace some generic types with impl Trait #98912

merged 1 commit into from
Jul 19, 2022

Conversation

nrc
Copy link
Member

@nrc nrc commented Jul 5, 2022

This gives a cleaner API since the caller only specifies the concrete type they usually want to.

r? @yaahc

@rustbot
Copy link
Collaborator

rustbot commented Jul 5, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 5, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 5, 2022
@nrc
Copy link
Member Author

nrc commented Jul 5, 2022

@rustbot label +T-libs-api -T-libs

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 5, 2022
@yaahc
Copy link
Member

yaahc commented Jul 5, 2022 •
edited
Loading

Did we land a change that makes it so that we can still turbofish functions that have an impl trait arg? I'm surprised this works.

@nrc
Copy link
Member Author

nrc commented Jul 6, 2022

Did we land a change that makes it so that we can still turbofish functions that have an impl trait arg? I'm surprised this works.

Yes, we stabilised the ability to specify generic parameters where impl Trait is also used, but nothing has changed around actually specifying the implicit parameter corresponding to the impl Trait itself.

@yaahc yaahc mentioned this pull request Jul 8, 2022
Open
7 tasks
@yaahc
Copy link
Member

yaahc commented Jul 8, 2022

Cool, sounds good, thank you for the update!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 8, 2022

📌 Commit da11ff3 has been approved by yaahc

It is now in the queue for this repository.

@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 Jul 8, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 9, 2022
@Dylan-DPC
Rollup merge of rust-lang#98912 - nrc:provider-it, r=yaahc
1e8af6b 
core::any: replace some generic types with impl Trait

This gives a cleaner API since the caller only specifies the concrete type they usually want to.

r? `@yaahc`
@Dylan-DPC Dylan-DPC mentioned this pull request Jul 9, 2022
Closed
@Dylan-DPC
Copy link
Member

Dylan-DPC commented Jul 9, 2022 •
edited
Loading

failed in rollup ci

@bors r-

@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 Jul 9, 2022
@nrc
Copy link
Member Author

nrc commented Jul 11, 2022

@Dylan-DPC the rollup seems to have pulled in an old version of the PR which was failing tests, rather than the current version which passes them? Could you r? and rollup please?

@Dylan-DPC
Copy link
Member

sure

@bors r=yaahc

@bors
Copy link
Contributor

bors commented Jul 11, 2022

📌 Commit da11ff3 has been approved by yaahc

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 11, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 11, 2022
@Dylan-DPC
Rollup merge of rust-lang#98912 - nrc:provider-it, r=yaahc
fcb879e 
core::any: replace some generic types with impl Trait

This gives a cleaner API since the caller only specifies the concrete type they usually want to.

r? `@yaahc`
@Dylan-DPC Dylan-DPC mentioned this pull request Jul 11, 2022
Closed
@Dylan-DPC
Copy link
Member

@nrc this seems to be happening again better to let it merge on its own

@bors rollup=never

@nrc
Copy link
Member Author

nrc commented Jul 18, 2022

@Dylan-DPC @yaahc could you re-approve this PR please? Bors seems to think it is unapproved

@Dylan-DPC
Copy link
Member

@bors r=yaahc

@bors
Copy link
Contributor

bors commented Jul 18, 2022

📌 Commit 0c72be3 has been approved by yaahc

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 18, 2022

⌛ Testing commit 0c72be3 with merge db924fdb2f045c62e5fb51a735ff4567e36cbe5f...

@bors
Copy link
Contributor

bors commented Jul 18, 2022

💔 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 Jul 18, 2022
@rust-log-analyzer
Copy link
Collaborator

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

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

---- [run-pass-valgrind] src/test/run-pass-valgrind/cleanup-stdin.rs stdout ----

error: compilation failed!
status: signal: 9 (SIGKILL)
command: "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage2/bin/rustc" "/Users/runner/work/rust/rust/src/test/run-pass-valgrind/cleanup-stdin.rs" "-Zthreads=1" "--target=x86_64-apple-darwin" "-O" "-C" "prefer-dynamic" "-o" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/test/run-pass-valgrind/cleanup-stdin/a" "-Crpath" "-Cdebuginfo=0" "-Lnative=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/native/rust-test-helpers" "-L" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/test/run-pass-valgrind/cleanup-stdin/auxiliary"
stdout: none
stderr: none


failures:
    [run-pass-valgrind] src/test/run-pass-valgrind/cleanup-stdin.rs

@Mark-Simulacrum
Copy link
Member

@bors retry apple valgrind signal 9

@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 Jul 18, 2022
@bors
Copy link
Contributor

bors commented Jul 19, 2022

⌛ Testing commit 0c72be3 with merge 8bd12e8...

@bors
Copy link
Contributor

bors commented Jul 19, 2022

☀️ Test successful - checks-actions
Approved by: yaahc
Pushing 8bd12e8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 19, 2022
@bors bors merged commit 8bd12e8 into rust-lang:master Jul 19, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 19, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8bd12e8): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 6.2% 6.3% 2
Improvements 🎉
(primary)		 -2.9% -2.9% 1
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) -2.9% -2.9% 1

Cycles

This benchmark run did not return any relevant results for this metric.

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

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩

  2. number of relevant changes ↩

This was referenced Jul 28, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@yaahc yaahc

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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.
9 participants
@nrc @rustbot @yaahc @bors @Dylan-DPC @rust-log-analyzer @Mark-Simulacrum @rust-timer @rust-highfive