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

convert higher ranked Predicates to PredicateKind::ForAll #73503

Merged
merged 28 commits into from
Jul 27, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jun 19, 2020

implements step 2 of rust-lang/compiler-team#285
r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 19, 2020
@lcnr lcnr marked this pull request as draft June 19, 2020 12:07
src/librustc_traits/chalk/lowering.rs Show resolved Hide resolved
src/librustc_traits/chalk/lowering.rs Show resolved Hide resolved
src/librustc_traits/chalk/lowering.rs Show resolved Hide resolved
src/librustc_traits/chalk/lowering.rs Outdated Show resolved Hide resolved
src/librustc_infer/traits/util.rs Outdated Show resolved Hide resolved
src/librustc_middle/ty/mod.rs Outdated Show resolved Hide resolved
src/librustc_middle/ty/mod.rs Outdated Show resolved Hide resolved
src/librustc_middle/ty/mod.rs Show resolved Hide resolved
@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch from 66c7be0 to 9468dfb Compare June 21, 2020 09:36
@lcnr lcnr marked this pull request as ready for review June 21, 2020 11:27
@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch from 641480c to f2246c7 Compare June 21, 2020 11:54
@bors
Copy link
Contributor

bors commented Jun 21, 2020

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

@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch from 8514396 to eaa2952 Compare June 22, 2020 06:55
@bors
Copy link
Contributor

bors commented Jun 23, 2020

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

@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch 3 times, most recently from 4403278 to a4f1fcd Compare June 23, 2020 17:44
@nikomatsakis
Copy link
Contributor

@bors try

(I want to do a perf run)

@bors
Copy link
Contributor

bors commented Jun 23, 2020

⌛ Trying commit a4f1fcd with merge 33ea641dd70d10df2f81ff66863ce09f631b8195...

@nikomatsakis
Copy link
Contributor

@rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Jun 24, 2020

☀️ Try build successful - checks-azure
Build commit: 33ea641dd70d10df2f81ff66863ce09f631b8195 (33ea641dd70d10df2f81ff66863ce09f631b8195)

@rust-timer
Copy link
Collaborator

Queued 33ea641dd70d10df2f81ff66863ce09f631b8195 with parent ff5b446, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (33ea641dd70d10df2f81ff66863ce09f631b8195): comparison url.

@nikomatsakis
Copy link
Contributor

OK, perf impact is basically neutral (maybe 1% slower on stress tests...), that's great.

@bors
Copy link
Contributor

bors commented Jun 26, 2020

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

@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch 2 times, most recently from 4104127 to 6d55dfa Compare July 4, 2020 14:41
@bors
Copy link
Contributor

bors commented Jul 7, 2020

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

}
}
ty::PredicateAtom::Projection(projection) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that Projections no longer need special casing (they no longer run the leak check at the top level).

Copy link
Contributor Author

@lcnr lcnr Jul 18, 2020

Choose a reason for hiding this comment

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

It seems like we do still need this.

Removing the specialcase for projections results in

Building stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
   Compiling compiler_builtins v0.1.32
   Compiling core v0.0.0 (/home/lcnr/rust2/src/libcore)
   Compiling cc v1.0.57
   Compiling libc v0.2.71
   Compiling autocfg v0.1.7
   Compiling std v0.0.0 (/home/lcnr/rust2/src/libstd)
   Compiling hashbrown v0.6.2
   Compiling backtrace-sys v0.1.37
   Compiling unwind v0.0.0 (/home/lcnr/rust2/src/libunwind)
error[E0599]: no method named `count` found for struct `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u32>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>` in the current scope
    --> src/libcore/num/bignum.rs:173:74
     |
100  | / macro_rules! define_bignum {
101  | |     ($name:ident: type=$ty:ty, n=$n:expr) => {
102  | |         /// Stack-allocated arbitrary-precision (up to certain limit) integer.
103  | |         ///
...    |
173  | |                 let zeros = digits.iter().rev().take_while(|&&x| x == 0).count();
     | |                                                            ------------  ^^^^^ method not found in `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u32>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>`
     | |                                                            |
     | |                                                            doesn't satisfy `<_ as ops::function::FnOnce<(&&u32,)>>::Output = bool`
     | |                                                            doesn't satisfy `_: ops::function::FnMut<(&&u32,)>`
...    |
477  | |     };
478  | | }
     | |_- in this expansion of `define_bignum!`
...
483  |   define_bignum!(Big32x40: type=Digit32, n=40);
     |   --------------------------------------------- in this macro invocation
     | 
    ::: src/libcore/iter/adapters/mod.rs:1803:1
     |
1803 |   pub struct TakeWhile<I, P> {
     |   --------------------------
     |   |
     |   method `count` not found for this
     |   doesn't satisfy `_: iter::traits::iterator::Iterator`
     |
     = note: the method `count` exists but the following trait bounds were not satisfied:
             `<[closure@src/libcore/num/bignum.rs:173:60: 173:72] as ops::function::FnOnce<(&&u32,)>>::Output = bool`
             which is required by `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u32>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`
             `[closure@src/libcore/num/bignum.rs:173:60: 173:72]: ops::function::FnMut<(&&u32,)>`
             which is required by `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u32>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`
             `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u32>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`
             which is required by `&mut iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u32>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`

error[E0599]: no method named `count` found for struct `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u8>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>` in the current scope
    --> src/libcore/num/bignum.rs:173:74
     |
100  | / macro_rules! define_bignum {
101  | |     ($name:ident: type=$ty:ty, n=$n:expr) => {
102  | |         /// Stack-allocated arbitrary-precision (up to certain limit) integer.
103  | |         ///
...    |
173  | |                 let zeros = digits.iter().rev().take_while(|&&x| x == 0).count();
     | |                                                            ------------  ^^^^^ method not found in `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u8>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>`
     | |                                                            |
     | |                                                            doesn't satisfy `<_ as ops::function::FnOnce<(&&u8,)>>::Output = bool`
     | |                                                            doesn't satisfy `_: ops::function::FnMut<(&&u8,)>`
...    |
477  | |     };
478  | | }
     | |_- in this expansion of `define_bignum!`
...
488  |       define_bignum!(Big8x3: type=u8, n=3);
     |       ------------------------------------- in this macro invocation
     | 
    ::: src/libcore/iter/adapters/mod.rs:1803:1
     |
1803 |   pub struct TakeWhile<I, P> {
     |   --------------------------
     |   |
     |   method `count` not found for this
     |   doesn't satisfy `_: iter::traits::iterator::Iterator`
     |
     = note: the method `count` exists but the following trait bounds were not satisfied:
             `<[closure@src/libcore/num/bignum.rs:173:60: 173:72] as ops::function::FnOnce<(&&u8,)>>::Output = bool`
             which is required by `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u8>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`
             `[closure@src/libcore/num/bignum.rs:173:60: 173:72]: ops::function::FnMut<(&&u8,)>`
             which is required by `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u8>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`
             `iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u8>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`
             which is required by `&mut iter::adapters::TakeWhile<iter::adapters::Rev<slice::Iter<'_, u8>>, [closure@src/libcore/num/bignum.rs:173:60: 173:72]>: iter::traits::iterator::Iterator`

error: internal compiler error: src/librustc_middle/ich/impls_ty.rs:94:17: StableHasher: unexpected region RePlaceholder(Placeholder { universe: U1, name: BrAnon(0) })

@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch from cb12a23 to 105856c Compare July 9, 2020 08:11
@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch from 6553ad6 to 1859a2b Compare July 27, 2020 19:13
@lcnr lcnr force-pushed the forall-predicate-what-and-why-2 branch from 1859a2b to 602ef6b Compare July 27, 2020 19:17
@lcnr
Copy link
Contributor Author

lcnr commented Jul 27, 2020

@bors r=nikomatsakis rollup=never

@bors
Copy link
Contributor

bors commented Jul 27, 2020

📌 Commit 602ef6b 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 Jul 27, 2020
@bors
Copy link
Contributor

bors commented Jul 27, 2020

⌛ Testing commit 602ef6b with merge 76e8333...

@bors
Copy link
Contributor

bors commented Jul 27, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: nikomatsakis
Pushing 76e8333 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 27, 2020
@bors bors merged commit 76e8333 into rust-lang:master Jul 27, 2020
@lcnr lcnr deleted the forall-predicate-what-and-why-2 branch July 28, 2020 06:06
@Mark-Simulacrum
Copy link
Member

Perf triage:

This was a 2.5% regression on stress tests and a 1-2% loss on serde and futures (real world tests). This was expected.

Xanewok added a commit to Xanewok/rust-semverver that referenced this pull request Nov 17, 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 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.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants