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

Refactor ops.rs #42523

Merged
merged 11 commits into from
Jun 14, 2017
Merged

Refactor ops.rs #42523

merged 11 commits into from
Jun 14, 2017

Conversation

clarfonthey
Copy link
Contributor

@clarfonthey clarfonthey commented Jun 8, 2017

This refactors ops.rs into several different modules internally, as the file has gotten quite big. None of these modules are actually exported, but this should make maintaining it much easier. I've avoided the ambition of exporting the modules because they can more easily be rearranged after this commit goes through, even though it'd be cool to potentially export the modules in the future.

I've separated the creation of each file into a separate commit so that this is easier to read.

Redone version of #42269 with the movement of RangeArgument moved.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@kennytm
Copy link
Member

kennytm commented Jun 8, 2017

Codegen failed.

[00:49:17] ---- [codegen-units] codegen-units/item-collection/function-as-argument.rs stdout ----
[00:49:17] 	
[00:49:17] These items should have been contained but were not:
[00:49:17] 
[00:49:17] TRANS_ITEM fn core::ops[0]::FnOnce[0]::call_once[0]<fn(char, f64), (char, f64)>
[00:49:17] TRANS_ITEM fn core::ops[0]::FnOnce[0]::call_once[0]<fn(u32, &str), (u32, &str)>
[00:49:17] 
[00:49:17] 
[00:49:17] 
[00:49:17] These items were contained but should not have been:
[00:49:17] 
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::FnOnce[0]::call_once[0]<fn(char, f64), (char, f64)> @@ function_as_argument.cgu-0[Internal]
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::FnOnce[0]::call_once[0]<fn(u32, &str), (u32, &str)> @@ function_as_argument.cgu-0[Internal]
[00:49:17] 
[00:49:17] 
[00:49:17] thread '[codegen-units] codegen-units/item-collection/function-as-argument.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:1979
[00:49:17] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:49:17] 
[00:49:17] ---- [codegen-units] codegen-units/item-collection/trait-method-as-argument.rs stdout ----
[00:49:17] 	
[00:49:17] These items should have been contained but were not:
[00:49:17] 
[00:49:17] TRANS_ITEM fn core::ops[0]::FnMut[0]::call_mut[0]<fn(char) -> char, (char)>
[00:49:17] TRANS_ITEM fn core::ops[0]::FnMut[0]::call_mut[0]<fn(u32) -> u32, (u32)>
[00:49:17] TRANS_ITEM fn core::ops[0]::FnOnce[0]::call_once[0]<fn(char) -> char, (char)>
[00:49:17] TRANS_ITEM fn core::ops[0]::FnOnce[0]::call_once[0]<fn(u32) -> u32, (u32)>
[00:49:17] TRANS_ITEM fn core::ops[0]::Fn[0]::call[0]<fn(char) -> char, (char)>
[00:49:17] TRANS_ITEM fn core::ops[0]::Fn[0]::call[0]<fn(u32) -> u32, (u32)>
[00:49:17] 
[00:49:17] 
[00:49:17] 
[00:49:17] These items were contained but should not have been:
[00:49:17] 
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::FnMut[0]::call_mut[0]<fn(char) -> char, (char)> @@ trait_method_as_argument.cgu-0[Internal]
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::FnMut[0]::call_mut[0]<fn(u32) -> u32, (u32)> @@ trait_method_as_argument.cgu-0[Internal]
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::FnOnce[0]::call_once[0]<fn(char) -> char, (char)> @@ trait_method_as_argument.cgu-0[Internal]
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::FnOnce[0]::call_once[0]<fn(u32) -> u32, (u32)> @@ trait_method_as_argument.cgu-0[Internal]
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::Fn[0]::call[0]<fn(char) -> char, (char)> @@ trait_method_as_argument.cgu-0[Internal]
[00:49:17] TRANS_ITEM fn core::ops[0]::function[0]::Fn[0]::call[0]<fn(u32) -> u32, (u32)> @@ trait_method_as_argument.cgu-0[Internal]
[00:49:17] 
[00:49:17] 
[00:49:17] thread '[codegen-units] codegen-units/item-collection/trait-method-as-argument.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:1979
[00:49:17] 
[00:49:17] 
[00:49:17] failures:
[00:49:17]     [codegen-units] codegen-units/item-collection/function-as-argument.rs
[00:49:17]     [codegen-units] codegen-units/item-collection/trait-method-as-argument.rs

@alexcrichton alexcrichton added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 8, 2017
@clarfonthey
Copy link
Contributor Author

I updated the tests, so, it should work now.

@frewsxcv
Copy link
Member

frewsxcv commented Jun 9, 2017

Latest Travis run has more codegen errors.

take_foo_once(Trait::foo, 0u32);

//~ TRANS_ITEM fn trait_method_as_argument::take_foo_once[0]<char, fn(char) -> char>
//~ TRANS_ITEM fn trait_method_as_argument::take_foo_once[0]<char, fn(char) -> chr>
Copy link
Member

Choose a reason for hiding this comment

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

Typo 😅

Copy link
Contributor Author

@clarfonthey clarfonthey Jun 9, 2017

Choose a reason for hiding this comment

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

><

@clarfonthey
Copy link
Contributor Author

I got rid of the typo, so, it should work now.

@Mark-Simulacrum Mark-Simulacrum 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 Jun 11, 2017
@brson
Copy link
Contributor

brson commented Jun 13, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Jun 13, 2017

📌 Commit f8d5f90 has been approved by brson

@bors
Copy link
Contributor

bors commented Jun 14, 2017

⌛ Testing commit f8d5f90 with merge f7111f9...

@bors
Copy link
Contributor

bors commented Jun 14, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Jun 14, 2017

⌛ Testing commit f8d5f90 with merge 554c685...

bors added a commit that referenced this pull request Jun 14, 2017
Refactor ops.rs

This refactors ops.rs into several different modules internally, as the file has gotten quite big. None of these modules are actually exported, but this should make maintaining it much easier. I've avoided the ambition of exporting the modules because they can more easily be rearranged after this commit goes through, even though it'd be cool to potentially export the modules in the future.

I've separated the creation of each file into a separate commit so that this is easier to read.

Redone version of #42269 with the movement of `RangeArgument` moved.
@bors
Copy link
Contributor

bors commented Jun 14, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: brson
Pushing 554c685 to master...

@bors bors merged commit f8d5f90 into rust-lang:master Jun 14, 2017
@clarfonthey clarfonthey deleted the refactor_ops branch May 5, 2018 04:33
bors added a commit that referenced this pull request Jan 27, 2019
Refactor core::iter module

A while back, I refactored `core::ops` in #42523 because the module had become a giant mess and was difficult to modify. Now, I'm doing the same with the `core::iter` module.

Like the `core::ops` refactor, things have been split up into multiple commits to make rebasing easier, and so that you can follow changes. Although the diffs are hard to decipher, the only actual code changes I've made in the first few commits are to modify exports and imports. I save all of the actual code refactoring, e.g. modifying what methods are called, for the end.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants