Skip to content

/rust

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

The Great Generics Generalisation: Ty Edition #48523

Merged
merged 40 commits into from May 16, 2018
+966 −806

Conversation

Reviewers

@eddyb eddyb

@nikomatsakis nikomatsakis

Assignees

@eddyb eddyb

Labels
S-waiting-on-bors T-compiler
Projects
None yet
Milestone
No milestone
9 participants
@varkor @bors @shepmaster @pietroalbini @nikomatsakis @rust-highfive @bombless @eddyb @kennytm
@varkor
Copy link
Member

varkor commented Feb 25, 2018

Part of the generic parameter refactoring effort, split off from #48149. Contains the ty-relative refactoring.

r? @eddyb

@rust-highfive rust-highfive assigned eddyb Feb 25, 2018

@rust-highfive rust-highfive added the S-waiting-on-review label Feb 25, 2018

@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Feb 25, 2018

☔️ The latest upstream changes (presumably #48520) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author and removed S-waiting-on-review labels Feb 25, 2018

@varkor varkor force-pushed the varkor:generics-ty-generalisations branch 5 times, most recently from 2a0b89e to b4dbc02 Feb 25, 2018

@shepmaster

This comment has been minimized.

Sign in to view
Copy link
Member

shepmaster commented Mar 2, 2018

Ping from triage, @varkor ! You have some merge conflicts you need to address, will you be able to get to those soon?

@varkor varkor referenced this pull request Mar 2, 2018

Merged

The Great Generics Generalisation: HIR Edition #48149

@varkor

This comment has been minimized.

Sign in to view
Copy link
Member Author

varkor commented Mar 2, 2018

@shepmaster: the merge conflicts should all be fairly routine (this code has a reasonably high bitrot rate) — I should be able to fix them without issue after a review, so I'll wait for now to avoid repeatedly fixing new conflicts as they arise.

@shepmaster shepmaster added S-waiting-on-review and removed S-waiting-on-author labels Mar 3, 2018

@shepmaster

This comment has been minimized.

Sign in to view
Copy link
Member

shepmaster commented Mar 3, 2018

Ping from triage, @eddyb !
@eddyb

eddyb reviewed Mar 6, 2018
View changes

src/librustc/ty/mod.rs Outdated
pub fn own_count(&self) -> usize {
self.regions.len() + self.types.len()
self.params.len()
}

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Mar 6, 2018

Member

This method can be inlined everywhere it's used.
@eddyb

eddyb reviewed Mar 6, 2018
View changes

src/librustc_typeck/collect.rs Outdated
regions.into_iter().map(|lt| ty::GenericParam::Lifetime(lt)).collect();
let types: Vec<ty::GenericParam> =
types.into_iter().map(|ty| ty::GenericParam::Type(ty)).collect();
let params = lifetimes.into_iter().chain(types.into_iter()).collect();

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Mar 6, 2018

Member

This should ideally not allocate for "lifetimes" / "types".
@eddyb

eddyb reviewed Mar 6, 2018
View changes

src/librustc/ty/mod.rs Outdated
if let Some(index) = param.index.checked_sub(self.parent_count() as u32) {
&self.regions[index as usize - self.has_self as usize]
if let Some(index) = param.index.checked_sub(self.parent_count as u32) {
// We're currently assuming that lifetimes precede other generic parameters.

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Mar 6, 2018

Member

Self should come before lifetimes, such that indexing is trivial.
@eddyb

eddyb reviewed Mar 6, 2018
View changes

src/librustc/ty/subst.rs Outdated
}
};
assert_eq!(def.index() as usize, substs.len());
substs.push(param.pack());

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Mar 6, 2018

Member

Please do not call pack outside of the From impls.
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Mar 6, 2018

☔️ The latest upstream changes (presumably #48611) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author and removed S-waiting-on-review labels Mar 6, 2018

@varkor varkor force-pushed the varkor:generics-ty-generalisations branch from b4dbc02 to d0b0eb7 Mar 8, 2018

@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Mar 8, 2018

☔️ The latest upstream changes (presumably #46882) made this pull request unmergeable. Please resolve the merge conflicts.

@varkor varkor force-pushed the varkor:generics-ty-generalisations branch 2 times, most recently from 937afe9 to 689b927 Mar 8, 2018

@pietroalbini pietroalbini added S-waiting-on-review and removed S-waiting-on-author labels Mar 12, 2018

@pietroalbini

This comment has been minimized.

Sign in to view
Copy link
Member

pietroalbini commented Mar 12, 2018

@eddyb ping from triage! This PR has new commits that needs to be reviewed.

@varkor varkor force-pushed the varkor:generics-ty-generalisations branch from 8c72a03 to 5ea91ac May 15, 2018

@rust-highfive

This comment was marked as resolved.

Sign in to view
Copy link
Collaborator

rust-highfive commented May 15, 2018

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 

[00:04:47] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:48] tidy error: /checkout/src/librustc/infer/mod.rs:931: line longer than 100 chars
[00:04:49] some tidy checks failed
[00:04:49] 
[00:04:49] 
[00:04:49] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:49] 
[00:04:49] 
[00:04:49] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:49] Build completed unsuccessfully in 0:01:50
[00:04:49] Build completed unsuccessfully in 0:01:50
[00:04:49] Makefile:79: recipe for target 'tidy' failed
[00:04:49] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:22df26d0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@kennytm kennytm added S-waiting-on-review and removed S-waiting-on-author labels May 15, 2018

@nikomatsakis

nikomatsakis approved these changes May 15, 2018
View changes

@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented May 15, 2018

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented May 15, 2018

📌 Commit 5be2bdb has been approved by nikomatsakis

@bors bors added S-waiting-on-bors and removed S-waiting-on-review labels May 15, 2018

@kennytm

This comment has been minimized.

Sign in to view
Copy link
Member

kennytm commented May 15, 2018

@bors p=1

Unblocks #48149.
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented May 15, 2018

⌛️ Testing commit 5be2bdb with merge e44fc6c...

bors added a commit that referenced this pull request May 15, 2018

@bors
Auto merge of #48523 - varkor:generics-ty-generalisations, r=nikomats… 
…akis

The Great Generics Generalisation: Ty Edition

Part of the generic parameter refactoring effort, split off from #48149. Contains the `ty`-relative refactoring.

r? @eddyb
Loading status checks…
e44fc6c
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented May 16, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing e44fc6c to master...

@bors bors merged commit 5be2bdb into rust-lang:master May 16, 2018
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented May 16, 2018

📣 Toolstate changed by #48523!

Tested on commit e44fc6c.
Direct link to PR: #48523

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 16, 2018

@rust-highfive
📣 Toolstate changed by rust-lang/rust#48523! 
Tested on commit rust-lang/rust@e44fc6c.
Direct link to PR: <rust-lang/rust#48523>

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
Loading status checks…
e842d03

This was referenced May 16, 2018

Merged
Merged
Merged
Closed
Merged

@phansch phansch referenced this pull request May 16, 2018

Closed

Incoming clippy breakage #2766

@mati865 mati865 referenced this pull request May 17, 2018

Merged

Rustup to 2018-05-16 #2770

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.