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

change the order of type arguments on ControlFlow #76614

Merged
merged 2 commits into from
Oct 25, 2020

Conversation

NoraCodes
Copy link
Contributor

@NoraCodes NoraCodes commented Sep 11, 2020

This allows ControlFlow<BreakType> which is much more ergonomic for common iterator combinator use cases.

Addresses one component of #75744

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 Sep 11, 2020
@NoraCodes
Copy link
Contributor Author

r? @scottmcm

Copy link
Contributor

@ecstatic-morse ecstatic-morse left a comment

Choose a reason for hiding this comment

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

LGTM besides the one nit. I'll let @scottmcm have a look though.

@@ -87,8 +87,7 @@ where
}

/// Allows searches to terminate early with a value.
// FIXME (#75744): remove the alias once the generics are in a better order and `C=()`.
pub type ControlFlow<T> = std::ops::ControlFlow<(), T>;
pub use std::ops::ControlFlow;
Copy link
Contributor

Choose a reason for hiding this comment

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

Re-exporting a standard library type is somewhat unusual, could you change the use sites to import std::ops::ControlFlow instead?

Copy link
Member

@scottmcm scottmcm left a comment

Choose a reason for hiding this comment

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

Thanks @NoraCodes! I agree with morse's nit, then LGTM after that.

@jyn514 jyn514 added requires-nightly This issue requires a nightly compiler in some way. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 15, 2020
@camelid camelid 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 7, 2020
@camelid
Copy link
Member

camelid commented Oct 7, 2020

@NoraCodes It looks like this is very close to being ready to merge! Could you address #76614 (comment)?

@NoraCodes
Copy link
Contributor Author

Will do, apologies for taking so long on this.

@camelid
Copy link
Member

camelid commented Oct 7, 2020

No worries :)

@NoraCodes
Copy link
Contributor Author

Finally actually got around to setting up a Rust dev environment on my personal machine. 😅

@scottmcm
Copy link
Member

Thanks, @NoraCodes!

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Oct 21, 2020

📌 Commit a41795650c73a412a79aef06e71d068e2b707d8a has been approved by scottmcm

@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 Oct 21, 2020
@LeSeulArtichaut
Copy link
Contributor

@NoraCodes This conflicted with #78200, you'll have to rebase to merge this PR.

@LeSeulArtichaut LeSeulArtichaut 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 Oct 22, 2020
This allows ControlFlow<BreakType> which is much more ergonomic for
common iterator combinator use cases.
@scottmcm scottmcm force-pushed the nora/control_flow_enum branch from a417956 to bc23179 Compare October 23, 2020 00:27
@scottmcm
Copy link
Member

Rebased.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 23, 2020

📌 Commit bc23179 has been approved by scottmcm

@bors
Copy link
Contributor

bors commented Oct 23, 2020

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@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 Oct 23, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2020
…as-schievink

Rollup of 12 pull requests

Successful merges:

 - rust-lang#75115 (`#[deny(unsafe_op_in_unsafe_fn)]` in sys/cloudabi)
 - rust-lang#76614 (change the order of type arguments on ControlFlow)
 - rust-lang#77610 (revise Hermit's mutex interface to support the behaviour of StaticMutex)
 - rust-lang#77830 (Simplify query proc-macros)
 - rust-lang#77930 (Do not ICE with TraitPredicates containing [type error])
 - rust-lang#78069 (Fix const core::panic!(non_literal_str).)
 - rust-lang#78072 (Cleanup constant matching in exhaustiveness checking)
 - rust-lang#78119 (Throw core::panic!("message") as &str instead of String.)
 - rust-lang#78191 (Introduce a temporary for discriminant value in MatchBranchSimplification)
 - rust-lang#78272 (const_evaluatable_checked: deal with unused nodes + div)
 - rust-lang#78318 (TyCtxt: generate single impl block with `slice_interners` macro)
 - rust-lang#78327 (resolve: Relax macro resolution consistency check to account for any errors)

Failed merges:

r? `@ghost`
@bors bors merged commit a547055 into rust-lang:master Oct 25, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 25, 2020
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Nov 15, 2020
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.49.0, 1.48.0 Nov 15, 2020
@Mark-Simulacrum Mark-Simulacrum added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Nov 15, 2020
@Mark-Simulacrum
Copy link
Member

This was backported into 1.48 (approved by me) in #79075 as a dependency of #78775.

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 15, 2020
…ulacrum

[beta] next

This backports:

* Avoid installing external LLVM dylibs rust-lang#78986
* Install CI llvm into the library directory rust-lang#79074
* Revert "Revert "resolve: Avoid "self-confirming" import resolutions in one more case"" rust-lang#78784
* Bump Rustfmt and RLS rust-lang#78775
* Enforce no-move rule of ReentrantMutex using Pin and fix UB in stdio rust-lang#77801

For RLS/rustfmt compilation to succeed:
* change the order of type arguments on ControlFlow rust-lang#76614
* Add ControlFlow::is_{break,continue} methods rust-lang#78200
* Replace run_compiler with RunCompiler builder pattern rust-lang#77649

As a dependency of rust-lang#77801:
*  Add Pin::static_ref, static_mut. rust-lang#77726
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. requires-nightly This issue requires a nightly compiler in some way. 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
Development

Successfully merging this pull request may close these issues.

10 participants