Stream: wg-traits

Topic: alexreg - nmatsakis sync


nikomatsakis (Jan 14 2019 at 16:34, on Zulip):

Hey @Alexander Regueiro — so first thing I wanted to ask is what's on your mind. Topics I associate with you include: =)

Alexander Regueiro (Jan 14 2019 at 16:34, on Zulip):

yep

Alexander Regueiro (Jan 14 2019 at 16:35, on Zulip):

definitely those

nikomatsakis (Jan 14 2019 at 16:35, on Zulip):

i'm game to talk about any of those but maybe the trait alias PR would be good to start just because it'd be good to see that finished up

Alexander Regueiro (Jan 14 2019 at 16:35, on Zulip):

also, associated_types_in_bindings briefly

nikomatsakis (Jan 14 2019 at 16:35, on Zulip):

in particular the remaining bit had to do with trait objects

Alexander Regueiro (Jan 14 2019 at 16:35, on Zulip):

and possibly impl Trait lifetime constraints if we have time

nikomatsakis (Jan 14 2019 at 16:35, on Zulip):

(and trait aliases)

Alexander Regueiro (Jan 14 2019 at 16:35, on Zulip):

okay

centril (Jan 14 2019 at 16:35, on Zulip):

mind if I jump in?

nikomatsakis (Jan 14 2019 at 16:35, on Zulip):

my rebase PR landed, so the cross-crate stuff landed

nikomatsakis (Jan 14 2019 at 16:35, on Zulip):

@centril maybe?

nikomatsakis (Jan 14 2019 at 16:35, on Zulip):

depends what you want to say :P

centril (Jan 14 2019 at 16:36, on Zulip):

:P

nikomatsakis (Jan 14 2019 at 16:36, on Zulip):

(go ahead)

nikomatsakis (Jan 14 2019 at 16:36, on Zulip):

(I'd just like to make sure we stay focused, is all)

Alexander Regueiro (Jan 14 2019 at 16:36, on Zulip):

@nikomatsakis I saw, that's good. you're still working on rebasing the duplicate non-auto trait detection though right? e.g. TraitAliasExpander, etc.?

nikomatsakis (Jan 14 2019 at 16:37, on Zulip):

but if it's germane to the topic, go for it :)

Alexander Regueiro (Jan 14 2019 at 16:37, on Zulip):

@centril sure, although maybe semi-passively, just because I have limited time with Niko? ;-)

nikomatsakis (Jan 14 2019 at 16:37, on Zulip):

@Alexander Regueiro yeah, the trait alias expander itself seems fine and rebases easily enough, the question is more the code that applies it and how to work on that

nikomatsakis (Jan 14 2019 at 16:37, on Zulip):

@centril what was on your mind?

centril (Jan 14 2019 at 16:37, on Zulip):

I would said in terms of prioritization of work, trait aliases are not the best thing to work on if you want to ship

centril (Jan 14 2019 at 16:37, on Zulip):

re. associated_type_bounds, I'm going to write up some tests for @Alexander Regueiro

Alexander Regueiro (Jan 14 2019 at 16:37, on Zulip):

I didn't look at arielby's PR that removed principal traits to be honest.

centril (Jan 14 2019 at 16:37, on Zulip):

but I've been swamped, but I'll get to it (I promise)

nikomatsakis (Jan 14 2019 at 16:38, on Zulip):

I would said in terms of prioritization of work, trait aliases are not the best thing to work on if you want to ship

Because of some pending concerns re: stabilization? I am mostly concerned with having broken, inconsistent stuff in nightly. I'd like to get some variant of the work that @Alexander Regueiro already did landed.

Alexander Regueiro (Jan 14 2019 at 16:38, on Zulip):

sure

centril (Jan 14 2019 at 16:38, on Zulip):

associated_types_in_bindings <-- which one was this?

nikomatsakis (Jan 14 2019 at 16:38, on Zulip):

That said, the problem is tricky, it might make sense to hold off if we want

centril (Jan 14 2019 at 16:39, on Zulip):

Because of some pending concerns re: stabilization?

Right, if the purpose is to ship, then energy is better spent elsewhere.

I am mostly concerned with having broken, inconsistent stuff in nightly. I'd like to get some variant of the work that @Alexander Regueiro already did landed.

If the purpose is this, fire on all engines away! =)

Alexander Regueiro (Jan 14 2019 at 16:39, on Zulip):

to be fair, trait aliases are working nicely right now, they just unfortunately permit multiple-trait objects, which is bad

Alexander Regueiro (Jan 14 2019 at 16:39, on Zulip):

I leave stabilisation concerns to others to be honest (even if I sometimes do stabilisation PRs). I'm more worried about getting things working on nightly.

nikomatsakis (Jan 14 2019 at 16:40, on Zulip):

So, what @Ariel Ben-Yehuda's PR did is not that important

nikomatsakis (Jan 14 2019 at 16:40, on Zulip):

That is, I don't think it creates the challenge

Alexander Regueiro (Jan 14 2019 at 16:40, on Zulip):

okay good

nikomatsakis (Jan 14 2019 at 16:40, on Zulip):

The basic idea was that a trait object can have no principal

Alexander Regueiro (Jan 14 2019 at 16:41, on Zulip):

which is great

Alexander Regueiro (Jan 14 2019 at 16:41, on Zulip):

kind of segues into multi-trait objects eh?

nikomatsakis (Jan 14 2019 at 16:41, on Zulip):

And auto traits are never principal

nikomatsakis (Jan 14 2019 at 16:41, on Zulip):

right

nikomatsakis (Jan 14 2019 at 16:41, on Zulip):

So, what I was a bit unsure about

nikomatsakis (Jan 14 2019 at 16:41, on Zulip):

Yes

nikomatsakis (Jan 14 2019 at 16:41, on Zulip):

So if you have dyn Send + Sync that has no principal trait

nikomatsakis (Jan 14 2019 at 16:41, on Zulip):

So that is one ordering question

nikomatsakis (Jan 14 2019 at 16:42, on Zulip):

In particular we may find it makes sense to pursue multi-trait objects a bit first

centril (Jan 14 2019 at 16:42, on Zulip):

(zulip is having sync problems on my end)

centril (Jan 14 2019 at 16:42, on Zulip):

(delays)

Alexander Regueiro (Jan 14 2019 at 16:42, on Zulip):

yes, which is solved by arielyby's PR I think

centril (Jan 14 2019 at 16:42, on Zulip):

In particular we may find it makes sense to pursue multi-trait objects a bit first

:100:

nikomatsakis (Jan 14 2019 at 16:42, on Zulip):

What is solved?

Alexander Regueiro (Jan 14 2019 at 16:42, on Zulip):

ordering of auto traits

nikomatsakis (Jan 14 2019 at 16:42, on Zulip):

Ah

nikomatsakis (Jan 14 2019 at 16:42, on Zulip):

Yes

Alexander Regueiro (Jan 14 2019 at 16:42, on Zulip):

at least, once it becomes a hard error, it is

Alexander Regueiro (Jan 14 2019 at 16:42, on Zulip):

which I think it can in the next cycle

nikomatsakis (Jan 14 2019 at 16:42, on Zulip):

Well, the whole problem was that sometimes we would consider an auto trait to be principal

nikomatsakis (Jan 14 2019 at 16:43, on Zulip):

and we only detected conflicts within the auto traits

nikomatsakis (Jan 14 2019 at 16:43, on Zulip):

Actually I think it is a hard erorr now

Alexander Regueiro (Jan 14 2019 at 16:43, on Zulip):

we really want to remove the concept of principal traits entirely... but that will probably only be done with multi-trait objects

nikomatsakis (Jan 14 2019 at 16:43, on Zulip):

but we special-cased the coherence conflict

Alexander Regueiro (Jan 14 2019 at 16:43, on Zulip):

yes that's what I mean.

nikomatsakis (Jan 14 2019 at 16:43, on Zulip):

and that we can make a hard error at our liesure

Alexander Regueiro (Jan 14 2019 at 16:43, on Zulip):

mhm

nikomatsakis (Jan 14 2019 at 16:43, on Zulip):

So, the tricky bit that I was trying to think through.

nikomatsakis (Jan 14 2019 at 16:44, on Zulip):

The current logic takes the first thing in the list and processes it. This processes produces both a trait reference and a list of "associated type bindings"

Alexander Regueiro (Jan 14 2019 at 16:44, on Zulip):

right

nikomatsakis (Jan 14 2019 at 16:44, on Zulip):

It then processes the remaining items: the list of associated type bindings is thrown away, and we check that each of those remaining items are only auto traits. If that is true, it implies that there were no associated type bindings (or there is a separate error)

Alexander Regueiro (Jan 14 2019 at 16:44, on Zulip):

i.e. projections

nikomatsakis (Jan 14 2019 at 16:44, on Zulip):

Right

nikomatsakis (Jan 14 2019 at 16:45, on Zulip):

If that is true, it implies that there were no associated type bindings (or there is a separate error)

(because auto traits cannot have associated types)

Alexander Regueiro (Jan 14 2019 at 16:45, on Zulip):

yeah... the way I did this in my former PR was to expand trait aliases before checking for extra non-auto traits.

Alexander Regueiro (Jan 14 2019 at 16:45, on Zulip):

but I was still taking the future (unexpanded) trait as principal

nikomatsakis (Jan 14 2019 at 16:45, on Zulip):

Right, but that gets a bit tricky now around the associated type bindings

Alexander Regueiro (Jan 14 2019 at 16:45, on Zulip):

which may be non-ideal

Alexander Regueiro (Jan 14 2019 at 16:46, on Zulip):

yes...

nikomatsakis (Jan 14 2019 at 16:46, on Zulip):

so I was debating a few options

nikomatsakis (Jan 14 2019 at 16:46, on Zulip):

I'm actually not sure how tricky said bindings are

nikomatsakis (Jan 14 2019 at 16:46, on Zulip):

that is, we could just expand all the things

Alexander Regueiro (Jan 14 2019 at 16:46, on Zulip):

although I think we already handle associated type bindings from trait aliases correctly?

nikomatsakis (Jan 14 2019 at 16:46, on Zulip):

sort if out

Alexander Regueiro (Jan 14 2019 at 16:46, on Zulip):

I remember fretting over that and got it working in the end

nikomatsakis (Jan 14 2019 at 16:46, on Zulip):

there should be at most one non-auto trait

nikomatsakis (Jan 14 2019 at 16:46, on Zulip):

and then any number of auto traits

nikomatsakis (Jan 14 2019 at 16:46, on Zulip):

and all the bindings must therefore (by the same logic) come from that non-auto-trait

nikomatsakis (Jan 14 2019 at 16:47, on Zulip):

I remember fretting over that and got it working in the end

I think it should work, yes.

nikomatsakis (Jan 14 2019 at 16:47, on Zulip):

That is, I think it should just fall out from the existing supertrait handling

nikomatsakis (Jan 14 2019 at 16:47, on Zulip):

Hm maybe this is not so tricky

Alexander Regueiro (Jan 14 2019 at 16:47, on Zulip):

yes

nikomatsakis (Jan 14 2019 at 16:47, on Zulip):

It doesn't seem to me that there is much reason to special case that "first predicate"

Alexander Regueiro (Jan 14 2019 at 16:47, on Zulip):

no indeed

nikomatsakis (Jan 14 2019 at 16:47, on Zulip):

you could basically just concatenate them all

nikomatsakis (Jan 14 2019 at 16:47, on Zulip):

(and all the resulting bindings too)

nikomatsakis (Jan 14 2019 at 16:48, on Zulip):

(there are some special-case diagnostics that may want to be careful about)

Alexander Regueiro (Jan 14 2019 at 16:48, on Zulip):

I think that's the way to go actually.

Alexander Regueiro (Jan 14 2019 at 16:48, on Zulip):

yes

nikomatsakis (Jan 14 2019 at 16:48, on Zulip):

I agree

Alexander Regueiro (Jan 14 2019 at 16:48, on Zulip):

for duplicate bindings

Alexander Regueiro (Jan 14 2019 at 16:48, on Zulip):

we want proper error messages

Alexander Regueiro (Jan 14 2019 at 16:48, on Zulip):

no silent overridings

nikomatsakis (Jan 14 2019 at 16:48, on Zulip):

let me see what errors I was thinking of

Alexander Regueiro (Jan 14 2019 at 16:48, on Zulip):

should we move on though? I'm kind of worried about time heh.

Alexander Regueiro (Jan 14 2019 at 16:48, on Zulip):

(sorry)

nikomatsakis (Jan 14 2019 at 16:49, on Zulip):

this stuff

nikomatsakis (Jan 14 2019 at 16:50, on Zulip):

we can move on. The question that remains for me is whether you want to implement this particular change or if you'd prefer for me to finish that rebase. If you're just sick of it I can do so =)

nikomatsakis (Jan 14 2019 at 16:50, on Zulip):

What would you like to discuss next? :)

Alexander Regueiro (Jan 14 2019 at 16:50, on Zulip):

they're both pretty closely related no? you could do them in the same PR? unless I'm thinking of something else

Alexander Regueiro (Jan 14 2019 at 16:50, on Zulip):

maybe briefly associated_type_bindings...

nikomatsakis (Jan 14 2019 at 16:51, on Zulip):

OK

nikomatsakis (Jan 14 2019 at 16:51, on Zulip):

Remind me the PR?

Alexander Regueiro (Jan 14 2019 at 16:52, on Zulip):

so, I get a cyclic dependency error

Alexander Regueiro (Jan 14 2019 at 16:52, on Zulip):

https://github.com/rust-lang/rust/pull/57428

Alexander Regueiro (Jan 14 2019 at 16:52, on Zulip):

I've done a lot more debugging since

nikomatsakis (Jan 14 2019 at 16:52, on Zulip):

(is the current state of the PR still "up to date"?)

nikomatsakis (Jan 14 2019 at 16:52, on Zulip):

i.e., if I were to pull it and build it

Alexander Regueiro (Jan 14 2019 at 16:53, on Zulip):

yeah I think so

Alexander Regueiro (Jan 14 2019 at 16:53, on Zulip):

or very nearly

Alexander Regueiro (Jan 14 2019 at 16:53, on Zulip):

https://gist.github.com/b7f24166722b4e6a32f4aecf1cd4e6dc

Alexander Regueiro (Jan 14 2019 at 16:53, on Zulip):

so, I thought this was due to just debug statements initially

Alexander Regueiro (Jan 14 2019 at 16:53, on Zulip):

it still may be...

nikomatsakis (Jan 14 2019 at 16:54, on Zulip):

hmm OK

Alexander Regueiro (Jan 14 2019 at 16:54, on Zulip):

but fundamentally I've determined the problem is find_existential_constraints isn't be called on the generated existential type HIR item

Alexander Regueiro (Jan 14 2019 at 16:54, on Zulip):

because convert_item isn't even being called on it

Alexander Regueiro (Jan 14 2019 at 16:55, on Zulip):

I'm not sure if the issue is really because we the type definition for the generated existential type gets embedded in the user-written one.

Alexander Regueiro (Jan 14 2019 at 16:55, on Zulip):

or if that can be worked around.

Alexander Regueiro (Jan 14 2019 at 16:56, on Zulip):

the problem may be:

        // Desugared from `impl Trait`, so visited by the function's return type.
        hir::ItemKind::Existential(hir::ExistTy {
            impl_trait_fn: Some(_),
            ..
        }) => {}
nikomatsakis (Jan 14 2019 at 16:56, on Zulip):

is this somehow specific to existential type?

Alexander Regueiro (Jan 14 2019 at 16:56, on Zulip):

that I'm reusing this functionality for something other than RPIT

centril (Jan 14 2019 at 16:57, on Zulip):

(RPIT = return position impl trait)

Alexander Regueiro (Jan 14 2019 at 16:57, on Zulip):

i.e. you don't just get this when desugaring from RPIT

centril (Jan 14 2019 at 16:57, on Zulip):

(for those who don't have Varkor Initialism Syndrome...)

nikomatsakis (Jan 14 2019 at 16:57, on Zulip):

Yes, OK, I'm seeing now

nikomatsakis (Jan 14 2019 at 16:58, on Zulip):

If you write it as Foo = impl Trait, though, it works?

Alexander Regueiro (Jan 14 2019 at 16:58, on Zulip):

but maybe not? since impl_trait_fn should be None there... hmm

Alexander Regueiro (Jan 14 2019 at 16:58, on Zulip):

let me see

nikomatsakis (Jan 14 2019 at 16:58, on Zulip):

Also, which is the example that is failing?

Alexander Regueiro (Jan 14 2019 at 16:58, on Zulip):

existential type Bar: Debug + TraitB<AssocB = impl Send>; or equivalently existential type Bar: Debug + TraitB<AssocB: Send>;

nikomatsakis (Jan 14 2019 at 16:59, on Zulip):

do you have a test case committed?

nikomatsakis (Jan 14 2019 at 17:00, on Zulip):

if not, can you do so?

nikomatsakis (Jan 14 2019 at 17:00, on Zulip):

also, does this feature work in some other context?

nikomatsakis (Jan 14 2019 at 17:00, on Zulip):

e.g., does this example from the RFC work

fn print_all<T: Iterator<Item: Display>>(printables: T) {
    for p in printables {
        println!("{}", p);
    }
}
Alexander Regueiro (Jan 14 2019 at 17:00, on Zulip):

yeah, kind of... but it's only vaguely related (since this desugars to APIT, not RPIT):

fn print_all<T: IntoIterator<Item: Display>>(printables: T) {
    for p in printables {
        println!("{}", p);
    }
}
Alexander Regueiro (Jan 14 2019 at 17:01, on Zulip):

^^

Alexander Regueiro (Jan 14 2019 at 17:01, on Zulip):

that works yep

nikomatsakis (Jan 14 2019 at 17:01, on Zulip):

and something like fn get_items() -> impl IntoIterator<Item: Display> ?

nikomatsakis (Jan 14 2019 at 17:01, on Zulip):

incidentally my attempts to build the branch fail:

error[E0425]: cannot find value `DUMMY_SP` in this scope
    --> src/librustc/hir/lowering.rs:1126:21
     |
1126 |                     DUMMY_SP);
     |                     ^^^^^^^^ not found in this scope
help: possible candidates are found in other modules, you can import them into scope
     |
33   | use syntax::ext::quote::rt::DUMMY_SP;
     |
33   | use syntax::source_map::DUMMY_SP;
     |
33   | use syntax_pos::DUMMY_SP;
Alexander Regueiro (Jan 14 2019 at 17:02, on Zulip):

hmm

Alexander Regueiro (Jan 14 2019 at 17:02, on Zulip):

probably a bad rebase sorry

nikomatsakis (Jan 14 2019 at 17:02, on Zulip):

(obviously I can fix that particular problem)

Alexander Regueiro (Jan 14 2019 at 17:02, on Zulip):

just missing the import I suspect

nikomatsakis (Jan 14 2019 at 17:02, on Zulip):

right, I already added it

nikomatsakis (Jan 14 2019 at 17:02, on Zulip):

I was just saying in case there are missing commits :)

Alexander Regueiro (Jan 14 2019 at 17:03, on Zulip):

sure :-)

nikomatsakis (Jan 14 2019 at 17:04, on Zulip):

OK, so, I have to do another call now, but I've got a build going, and I'll get back to you in a bit

nikomatsakis (Jan 14 2019 at 17:04, on Zulip):

=)

nikomatsakis (Jan 14 2019 at 17:04, on Zulip):

next problem:

error: unused variable: `owner`
    --> src/librustc/hir/lowering.rs:1356:13
     |
1356 |         let owner = explicit_owner.unwrap_or(exist_ty_node_id);
     |             ^^^^^ help: consider using `_owner` instead
     |
     = note: `-D unused-variables` implied by `-D warnings`
Alexander Regueiro (Jan 14 2019 at 17:04, on Zulip):

okay no problem

Alexander Regueiro (Jan 14 2019 at 17:04, on Zulip):

@nikomatsakis can I ping you later with some other things?

nikomatsakis (Jan 14 2019 at 17:04, on Zulip):

of course

Alexander Regueiro (Jan 14 2019 at 17:04, on Zulip):

ta

Alexander Regueiro (Jan 14 2019 at 17:05, on Zulip):

@nikomatsakis multi-trait objects can wait, but I'd really like to set up a meeting with you, dhardy, and the other guy who expressed an interest in them on GH (I forget who)

Alexander Regueiro (Jan 14 2019 at 17:06, on Zulip):

@nikomatsakis I will get back to work on impl-trait-in-bindings when I can too, but obviously once you get around to collating all your notes from all over the place, that will be super helpful.

Alexander Regueiro (Jan 14 2019 at 17:08, on Zulip):

anyway, thanks for your time.

centril (Jan 14 2019 at 17:16, on Zulip):

@nikomatsakis slightly unrelated... one thing I discussed recently with Taylor was that we might want to require a set of tests upfront for lang RFCs

centril (Jan 14 2019 at 17:17, on Zulip):

so it is clearer what the semantics are for the team and for implementors

centril (Jan 14 2019 at 17:17, on Zulip):

for now, I'll try to write up some tests today; I'm also working on the associated type defaults RFC and simplifying it

Alexander Regueiro (Jan 14 2019 at 18:06, on Zulip):

So, I think that embedding the generated type with the binding for the existing existential type is going to always lead to problems... unless I'm mistaken?

Alexander Regueiro (Jan 14 2019 at 18:30, on Zulip):

@nikomatsakis ^

Alexander Regueiro (Jan 14 2019 at 18:31, on Zulip):

alternatively, I generate the type elsewhere in the HIR tree, and refer to it by Path

Alexander Regueiro (Jan 14 2019 at 18:41, on Zulip):

@nikomatsakis there seems to be an error before defining uses are checked, in fact: https://gist.github.com/6549d576e7dc98c4520bf519f32e6a9e

Alexander Regueiro (Jan 17 2019 at 17:52, on Zulip):

@nikomatsakis around now by chance?

Alexander Regueiro (Jan 21 2019 at 16:33, on Zulip):

@nikomatsakis hi

Alexander Regueiro (Jan 21 2019 at 16:42, on Zulip):

you there?

Alexander Regueiro (Jan 21 2019 at 16:52, on Zulip):

@nikomatsakis okay, this will have to be another time!

Taylor Cramer (Jan 21 2019 at 19:07, on Zulip):

@Alexander Regueiro It's a holiday in the US (MLK day)

nikomatsakis (Jan 22 2019 at 17:33, on Zulip):

sorry @Alexander Regueiro was AFK yeah

nikomatsakis (Jan 22 2019 at 17:33, on Zulip):

should have mentioned it to you

Alexander Regueiro (Mar 08 2019 at 21:07, on Zulip):

@nikomatsakis BTW, let's find time soon (first half of next week if possible?) to go over.
1. banning multi-trait objects via trait aliases, which is already implemented in my PR, but you were going to factor out...
2. extending the import system to get make methods on trait aliases and enum type aliases available. (feel free to point me to another compiler team member for this, if they're better-suited.)
3. talk about the existential lifetime issues and how to fix them (I may have a go at that anyway over the weekend if I have time, but definitely worth a chat).

Alexander Regueiro (Mar 08 2019 at 22:54, on Zulip):

4. collate notes/plans on impl-trait-in-bindings (but this can wait a bit still)

Last update: Nov 12 2019 at 15:35UTC