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

stop taking references in Relate #73705

Merged
merged 4 commits into from
Jul 1, 2020
Merged

stop taking references in Relate #73705

merged 4 commits into from
Jul 1, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jun 24, 2020

Adds a Copy bound to Relate and changes the type signatures to T from &T. While the Copy bound is not strictly necessary (i.e. the Clone bound of TypeRelation would be good enough), we don't need non Copy types and it simplifies the implementation.

Removes the afaict unused impls for Vec<ty::PolyExistentialProjection<'tcx>>, Rc<T> and Box<T>. If they end up being relevant again the bound of Relate can be reduced to T: Clone.

This also changes signature of Binder::skip_binder to fn skip_binder(self) -> T.

TypeError::ProjectionBoundsLength was never used and is also removed in this PR.

r? @nikomatsakis maybe 🤔 feel free to reassign

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 24, 2020
@nikomatsakis
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Jun 25, 2020

⌛ Trying commit 863ff8a61175b286206eace70e1cd174190cc520 with merge 962b50900727f0bf054bce5b61be7fd6dd43ab1b...

@bors
Copy link
Contributor

bors commented Jun 25, 2020

☀️ Try build successful - checks-azure
Build commit: 962b50900727f0bf054bce5b61be7fd6dd43ab1b (962b50900727f0bf054bce5b61be7fd6dd43ab1b)

@rust-timer
Copy link
Collaborator

Queued 962b50900727f0bf054bce5b61be7fd6dd43ab1b with parent 67100f6, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (962b50900727f0bf054bce5b61be7fd6dd43ab1b): comparison url.

@lcnr
Copy link
Contributor Author

lcnr commented Jun 25, 2020

looks like a small perf improvement

@nikomatsakis
Copy link
Contributor

Yeah, or neutral. Which is fine. The compelling case here is more ergonomic (which is what I would expect, to be honest).

@lcnr
Copy link
Contributor Author

lcnr commented Jun 27, 2020

@nikomatsakis fixed the merge conflict. Do you want to go ahead with merging this or is there still something I should do?

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 29, 2020

📌 Commit acd4818 has been approved by nikomatsakis

@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 Jun 29, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 29, 2020
stop taking references in Relate

Adds a `Copy` bound to `Relate` and changes the type signatures to `T` from `&T`. While the `Copy` bound is not strictly necessary (i.e. the `Clone` bound of `TypeRelation` would be good enough), we don't need non `Copy` types and it simplifies the implementation.

Removes the afaict unused impls for `Vec<ty::PolyExistentialProjection<'tcx>>`, `Rc<T>` and `Box<T>`. If they end up being relevant again the bound of `Relate` can be reduced to `T: Clone`.

This also changes signature of `Binder::skip_binder` to `fn skip_binder(self) -> T`.

`TypeError::ProjectionBoundsLength` was never used and is also removed in this PR.

r? @nikomatsakis maybe 🤔 feel free to reassign
@Manishearth
Copy link
Member

@bors r-

#73876 (comment)

@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 Jun 30, 2020
@lcnr
Copy link
Contributor Author

lcnr commented Jun 30, 2020

rebased, should be ready for merge again

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 30, 2020

📌 Commit 69e4990 has been approved by nikomatsakis

@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 Jun 30, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 1, 2020
stop taking references in Relate

Adds a `Copy` bound to `Relate` and changes the type signatures to `T` from `&T`. While the `Copy` bound is not strictly necessary (i.e. the `Clone` bound of `TypeRelation` would be good enough), we don't need non `Copy` types and it simplifies the implementation.

Removes the afaict unused impls for `Vec<ty::PolyExistentialProjection<'tcx>>`, `Rc<T>` and `Box<T>`. If they end up being relevant again the bound of `Relate` can be reduced to `T: Clone`.

This also changes signature of `Binder::skip_binder` to `fn skip_binder(self) -> T`.

`TypeError::ProjectionBoundsLength` was never used and is also removed in this PR.

r? @nikomatsakis maybe 🤔 feel free to reassign
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jul 1, 2020
stop taking references in Relate

Adds a `Copy` bound to `Relate` and changes the type signatures to `T` from `&T`. While the `Copy` bound is not strictly necessary (i.e. the `Clone` bound of `TypeRelation` would be good enough), we don't need non `Copy` types and it simplifies the implementation.

Removes the afaict unused impls for `Vec<ty::PolyExistentialProjection<'tcx>>`, `Rc<T>` and `Box<T>`. If they end up being relevant again the bound of `Relate` can be reduced to `T: Clone`.

This also changes signature of `Binder::skip_binder` to `fn skip_binder(self) -> T`.

`TypeError::ProjectionBoundsLength` was never used and is also removed in this PR.

r? @nikomatsakis maybe 🤔 feel free to reassign
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 1, 2020
…arth

Rollup of 17 pull requests

Successful merges:

 - rust-lang#72071 (Added detailed error code explanation for issue E0687 in Rust compiler.)
 - rust-lang#72369 (Bring net/parser.rs up to modern up to date with modern rust patterns)
 - rust-lang#72445 (Stabilize `#[track_caller]`.)
 - rust-lang#73466 (impl From<char> for String)
 - rust-lang#73548 (remove rustdoc warnings)
 - rust-lang#73649 (Fix sentence structure)
 - rust-lang#73678 (Update Box::from_raw example to generalize better)
 - rust-lang#73705 (stop taking references in Relate)
 - rust-lang#73716 (Document the static keyword)
 - rust-lang#73752 (Remap Windows ERROR_INVALID_PARAMETER to ErrorKind::InvalidInput from Other)
 - rust-lang#73776 (Move terminator to new module)
 - rust-lang#73778 (Make `likely` and `unlikely` const, gated by feature `const_unlikely`)
 - rust-lang#73805 (Document the type keyword)
 - rust-lang#73806 (Use an 'approximate' universal upper bound when reporting region errors)
 - rust-lang#73828 (Fix wording for anonymous parameter name help)
 - rust-lang#73846 (Fix comma in debug_assert! docs)
 - rust-lang#73847 (Edit cursor.prev() method docs in lexer)

Failed merges:

r? @ghost
@bors bors merged commit 9c65486 into rust-lang:master Jul 1, 2020
@lcnr lcnr deleted the skip_binder branch July 1, 2020 19:00
Xanewok added a commit to Xanewok/rust-semverver that referenced this pull request Nov 18, 2020
Xanewok added a commit to Xanewok/rust-semverver that referenced this pull request Nov 18, 2020
Xanewok added a commit to Xanewok/rust-semverver that referenced this pull request Nov 18, 2020
Xanewok added a commit to Xanewok/rust-semverver that referenced this pull request Nov 19, 2020
Xanewok added a commit to Xanewok/rust-semverver that referenced this pull request Nov 19, 2020
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
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.

7 participants