Stream: wg-traits

Topic: #41517 trait aliases


Alexander Regueiro (Oct 15 2018 at 18:39, on Zulip):

@nikomatsakis sure

nikomatsakis (Oct 15 2018 at 18:40, on Zulip):

so what overall approach have you been taking?

Alexander Regueiro (Oct 15 2018 at 18:41, on Zulip):

so, I'll be honest, I'm just getting into traits in the compiler lately, and it's a little overwhelming. but I'm wading through slowly thanks to the rustc-guide, @durka's PR, and your notes on that PR

Alexander Regueiro (Oct 15 2018 at 18:41, on Zulip):

modifying how selection is done

Alexander Regueiro (Oct 15 2018 at 18:41, on Zulip):

adding a new TraitAliasCandidate (with your suggested definition)

Alexander Regueiro (Oct 15 2018 at 18:42, on Zulip):

vtable tomfoolery

Alexander Regueiro (Oct 15 2018 at 18:42, on Zulip):

I was wondering if it would make sense to base candidate assembly & confirm for trait alias off existing code?

Alexander Regueiro (Oct 15 2018 at 18:42, on Zulip):

can I reuse anything?

nikomatsakis (Oct 15 2018 at 18:43, on Zulip):

let me review what I wrote before :)

Alexander Regueiro (Oct 15 2018 at 18:46, on Zulip):

sure

Alexander Regueiro (Oct 15 2018 at 18:46, on Zulip):

want a link?

Alexander Regueiro (Oct 15 2018 at 18:46, on Zulip):

you suggested two different approaches, and I picked the one that sounded lightly cleaner/easier to me, even though I think you slightly preferred the other (sorry)

nikomatsakis (Oct 15 2018 at 18:49, on Zulip):

I just re-read the PR

nikomatsakis (Oct 15 2018 at 18:49, on Zulip):

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

Alexander Regueiro (Oct 15 2018 at 18:50, on Zulip):

yep that's it

nikomatsakis (Oct 15 2018 at 18:50, on Zulip):

I am torn here because I would definitely like to see progress on trait aliases

nikomatsakis (Oct 15 2018 at 18:50, on Zulip):

but I can't help but think this would be so much easier to do via chalk

Alexander Regueiro (Oct 15 2018 at 18:50, on Zulip):

I linked in the meeting notes, I just forgot ha

Alexander Regueiro (Oct 15 2018 at 18:51, on Zulip):

I agree.

Alexander Regueiro (Oct 15 2018 at 18:51, on Zulip):

I don't mind this being stop-gap

nikomatsakis (Oct 15 2018 at 18:51, on Zulip):

in any case we could adapt select.rs with relative ease, I think

nikomatsakis (Oct 15 2018 at 18:51, on Zulip):

i'm not sure what is the best template but I think

nikomatsakis (Oct 15 2018 at 18:52, on Zulip):

well it's sort of a hybrid between impls + where-clauses

nikomatsakis (Oct 15 2018 at 18:52, on Zulip):

I guess a bit diferent from both too;)

nikomatsakis (Oct 15 2018 at 18:53, on Zulip):

let's worry first about T: TraitAlias as a "goal" — that is, something we have to prove

nikomatsakis (Oct 15 2018 at 18:53, on Zulip):

in that case, you would basically always have a candidate that is successful

nikomatsakis (Oct 15 2018 at 18:53, on Zulip):

it doesn't need to do any unification etc

nikomatsakis (Oct 15 2018 at 18:53, on Zulip):

but it would produce a vtable with derived goals that are equal to the predicates_of instantiated

nikomatsakis (Oct 15 2018 at 18:53, on Zulip):

with the substitution

nikomatsakis (Oct 15 2018 at 18:53, on Zulip):

pretty straight-forward really

nikomatsakis (Oct 15 2018 at 18:54, on Zulip):

the reverse direction would be handled by extending elaboration

Alexander Regueiro (Oct 15 2018 at 18:54, on Zulip):

hmm

nikomatsakis (Oct 15 2018 at 18:54, on Zulip):

I guess.. now that I think about it.. it would be nicer with chalk but it should also be a fairly self-contained edit to the existing system

nikomatsakis (Oct 15 2018 at 18:54, on Zulip):

well, so, I'm not sure if any of that made sense :P

nikomatsakis (Oct 15 2018 at 18:54, on Zulip):

I can try to write some mentoring instructions somewhere

nikomatsakis (Oct 15 2018 at 18:54, on Zulip):

do you have an open PR?

nikomatsakis (Oct 15 2018 at 18:54, on Zulip):

I've got to go now for a meeting though

Alexander Regueiro (Oct 15 2018 at 18:54, on Zulip):

I don't, but let me create one now

Alexander Regueiro (Oct 15 2018 at 18:55, on Zulip):

please do write some mentoring instructions, I'd much appreciate it :-)

Alexander Regueiro (Oct 15 2018 at 18:55, on Zulip):

also, what would TraitAliasCandidate look like?

nikomatsakis (Oct 15 2018 at 18:55, on Zulip):

ok, it seems like the tracking issue sort of dominated by unrelated bikeshedding

Alexander Regueiro (Oct 15 2018 at 18:55, on Zulip):

yeah heh

nikomatsakis (Oct 15 2018 at 18:56, on Zulip):

ping me with the PR # once you open it

nikomatsakis (Oct 15 2018 at 18:56, on Zulip):

and we can use this thread to communicate too

nikomatsakis (Oct 15 2018 at 18:57, on Zulip):

this should be mildly easier to write some notes on than the impl trait stuff ;)

Alexander Regueiro (Oct 15 2018 at 18:57, on Zulip):

https://github.com/rust-lang/rust/pull/55101
here you go

Alexander Regueiro (Oct 15 2018 at 18:57, on Zulip):

yeah ha

Alexander Regueiro (Oct 15 2018 at 18:57, on Zulip):

though I think I understand where you want to go with the impl Trait stuff better at this point ;-)

Alexander Regueiro (Oct 15 2018 at 18:58, on Zulip):

anyway I'm off to eat now, but may give this a go in a few hours if you've written notes by then. no worries if not.

nikomatsakis (Oct 15 2018 at 18:58, on Zulip):

great, I have a meeting for next hour so not before then :)

Alexander Regueiro (Oct 15 2018 at 18:58, on Zulip):

:wave:🏼

Alexander Regueiro (Oct 15 2018 at 18:58, on Zulip):

and thanks

varkor (Oct 16 2018 at 09:13, on Zulip):

cc me when aliases are in a usable state, @Alexander Regueiro; I'm keen not to lose track of the status, so that the feature name / keyword issues aren't overlooked

nikomatsakis (Oct 16 2018 at 13:40, on Zulip):

@Alexander Regueiro first round of comments is available

Alexander Regueiro (Oct 16 2018 at 17:28, on Zulip):

@Alexander Regueiro first round of comments is available

thanks!

Alexander Regueiro (Oct 16 2018 at 17:28, on Zulip):

cc me when aliases are in a usable state, @Alexander Regueiro; I'm keen not to lose track of the status, so that the feature name / keyword issues aren't overlooked

sure

nikomatsakis (Oct 18 2018 at 17:40, on Zulip):

@Alexander Regueiro wait, where did you leave the questions you wanted me to answer? were they in #55101 ?

nikomatsakis (Oct 18 2018 at 17:40, on Zulip):

if so, I don't see them :)

nikomatsakis (Oct 18 2018 at 17:40, on Zulip):

or I guess maybe you mean this:

Any advice on writing confirm_trait_alias_candidate in traits/project.rs, or the VtableTraitAlias arm for resolve_associated_item in ty/instance.rs?

Alexander Regueiro (Oct 18 2018 at 17:40, on Zulip):

Yeah

nikomatsakis (Oct 18 2018 at 17:40, on Zulip):

hmm

nikomatsakis (Oct 18 2018 at 17:40, on Zulip):

I don't think that should be possible

nikomatsakis (Oct 18 2018 at 17:40, on Zulip):

since they have no associated items

nikomatsakis (Oct 18 2018 at 17:41, on Zulip):

unless i'm mis-remembering something, I think you can just put unreachable!()

Alexander Regueiro (Oct 18 2018 at 17:41, on Zulip):

There are TODO comments in the relevant bits of code I think

nikomatsakis (Oct 18 2018 at 17:41, on Zulip):

or — better yet — span_bug!

Alexander Regueiro (Oct 18 2018 at 17:41, on Zulip):

Oh okay, cool.

nikomatsakis (Oct 18 2018 at 17:41, on Zulip):

is that pushed to the PR?

Alexander Regueiro (Oct 18 2018 at 17:41, on Zulip):

Yep

nikomatsakis (Oct 18 2018 at 17:41, on Zulip):

it didn't look different...

Alexander Regueiro (Oct 18 2018 at 17:41, on Zulip):

I think so

nikomatsakis (Oct 18 2018 at 17:41, on Zulip):

maybe I missed something

Alexander Regueiro (Oct 18 2018 at 17:41, on Zulip):

Hmm

nikomatsakis (Oct 18 2018 at 17:41, on Zulip):

alexreg committed 7 days ago

nikomatsakis (Oct 18 2018 at 17:42, on Zulip):

I think you didn't push :)

Alexander Regueiro (Oct 18 2018 at 17:59, on Zulip):

Try now. I’m on the move, so had to VPN in to push. ;-)

Alexander Regueiro (Oct 18 2018 at 18:03, on Zulip):

Anyway the test failures are just tests I need to bless I think. Though you can check.

Alexander Regueiro (Oct 18 2018 at 18:04, on Zulip):

More interested in those question points now. Maybe it’s as simple as stubbing out with span_bug as ypu said thpugh! And probably getting trait objects working after.

nikomatsakis (Oct 18 2018 at 18:07, on Zulip):

yep, I left a comment, but it just repeated that

nikomatsakis (Oct 18 2018 at 18:07, on Zulip):

as far as trait objects go...

nikomatsakis (Oct 18 2018 at 18:07, on Zulip):

I'll have to look about to remember

Alexander Regueiro (Oct 18 2018 at 18:44, on Zulip):

@nikomatsakis Okay thanks. Look forward to hearing about that! After which I can bug you about impl-trait-in-bindings again heh.

varkor (Oct 18 2018 at 19:05, on Zulip):

so "trait aliases" are really like "trait bound aliases"?

varkor (Oct 18 2018 at 19:06, on Zulip):

I had thought they were general bounds, but this is even stranger

Alexander Regueiro (Oct 18 2018 at 19:32, on Zulip):

General in what sense? They include lifetimes yes.

nikomatsakis (Oct 18 2018 at 19:33, on Zulip):

I had thought they were general bounds, but this is even stranger

they are general bounds?

nikomatsakis (Oct 18 2018 at 19:33, on Zulip):

I think perhaps you mean, can I express any sort of where clause?

nikomatsakis (Oct 18 2018 at 19:34, on Zulip):

I forget the exact syntax, but I think you can write something like (e.g.)

trait Foo<T> = where T: PartialEq<Self>;
nikomatsakis (Oct 18 2018 at 19:34, on Zulip):

they are obviously "biased" to work like the stuff that comes after a :, so that you can write (e.g.)

trait Printable = Debug + Display;

:heart_eyes:

Alexander Regueiro (Oct 18 2018 at 19:34, on Zulip):

Yeah you can. :-)

Alexander Regueiro (Oct 18 2018 at 19:35, on Zulip):

It’s in the RFC and I believe @Alex Burka already implemented the parsing fully.

varkor (Oct 18 2018 at 20:45, on Zulip):

isn't trait X = 'static; a valid alias?

Alexander Regueiro (Oct 18 2018 at 21:44, on Zulip):

It is yes.

Alexander Regueiro (Oct 19 2018 at 15:38, on Zulip):

Where are trait object types created in the codebase?

nikomatsakis (Oct 19 2018 at 19:56, on Zulip):

@Alexander Regueiro

Where are trait object types created in the codebase?

I'm taking the liberty of replying in this topic =) Are you looking for where the "trait safety" restrictions are checked? Trait alias types are created in astconv.rs, I think

nikomatsakis (Oct 19 2018 at 19:58, on Zulip):

specifically, on this line

Alexander Regueiro (Oct 19 2018 at 20:03, on Zulip):

Oh. I sent the message in this topic, didn't I? (as well as on Discord)

Alexander Regueiro (Oct 19 2018 at 20:03, on Zulip):

anyway, thanks

Alexander Regueiro (Oct 19 2018 at 20:07, on Zulip):

although I think I need to go up the call tree from there... since I'm more interested in the point at which it's actually decided it's a trait object. I'm also interested in the decision point for &Trait. is this separate, @nikomatsakis ?

Alexander Regueiro (Oct 19 2018 at 20:45, on Zulip):

@nikomatsakis sorry, I'm an idiot. somehow ended up posting in the high-level docs sub-topic. not sure how that happened.

Alexander Regueiro (Oct 20 2018 at 02:03, on Zulip):

Okay, I think I have a good idea how to go about this... @Matthew Jasper gave me some good advice over on Discord.

Alexander Regueiro (Oct 20 2018 at 02:04, on Zulip):

The only question is, should I do the substitution of trait aliases for multiple trait bounds (a PoltytraitRef?) at HIR lowering or type resolution time.

Alexander Regueiro (Oct 21 2018 at 22:41, on Zulip):

@nikomatsakis or thirdly, at astconv time

Alexander Regueiro (Oct 21 2018 at 23:48, on Zulip):

@nikomatsakis Okay, I was trying to implement it in lowering, and to my utter surprise, it started working with a more-or-less trivial change!

Alexander Regueiro (Oct 21 2018 at 23:48, on Zulip):

Even transitive aliases

Alexander Regueiro (Oct 21 2018 at 23:50, on Zulip):

errors properly too (displaying the trait alias name rather than the "expanded" version... not sure if this is what is desirable; it's not obvious)

Alexander Regueiro (Oct 21 2018 at 23:51, on Zulip):

Quite incredible. I thought I knew what had to be done to make it work, but there we go. :-P

Alexander Regueiro (Oct 21 2018 at 23:51, on Zulip):

Anyway see my latest push on the PIR. WIP tests in src/test/run-pass/traits/trait-alias.rs right now

Alexander Regueiro (Oct 22 2018 at 02:34, on Zulip):

@nikomatsakis All tests passing now, except one. src/test/ui/trait-alias-fail.rs -- the first impl succeeds (but shouldn't), while the second impl fails (as it should). the RFC indicates both should fail, I believe. what would you expect in this scenario? and in whatever case, how can we remedy this? I'm hoping my simple solution to trait aliases as object types can remain this simple, and we can just work around this...

Alexander Regueiro (Oct 22 2018 at 14:12, on Zulip):

Which was one of the causes for bikeshedxing on the name.

nikomatsakis (Oct 22 2018 at 16:43, on Zulip):

@Alexander Regueiro will take a look hopefully today :)

nikomatsakis (Oct 22 2018 at 16:43, on Zulip):

exciting to see you made good progress

Alexander Regueiro (Oct 22 2018 at 21:43, on Zulip):

does anyone know what happened to the lints for dyn Trait? I don't get them any more on nightly

Alexander Regueiro (Oct 22 2018 at 23:18, on Zulip):

never mind, I found it.

Alexander Regueiro (Oct 22 2018 at 23:49, on Zulip):

@nikomatsakis So, the way I've implemented it, impl dyn TraitAlias works now, but I think that makes perfect sense, to be honest. (impl TraitAlias for Type doesn't compile, which also makes sense, I believe.)

Alexander Regueiro (Oct 22 2018 at 23:49, on Zulip):

anyway, I just pushed a version which should pass all tests now

nikomatsakis (Oct 23 2018 at 14:28, on Zulip):

Huh, impl dyn TraitAlias... I'll take a look

nikomatsakis (Oct 23 2018 at 14:28, on Zulip):

I didn't get to finish my review yesterday before I got pulled off

Alexander Regueiro (Oct 23 2018 at 15:53, on Zulip):

No worries.

Alexander Regueiro (Oct 23 2018 at 15:54, on Zulip):

@nikomatsakis My argument is, dyn Trait works, and we want dyn TraitAlias to work (as per the RFC), where dyn TraitAlias is a type... thus we should also want the inherent impl dyn TraitAlias to work

nikomatsakis (Oct 23 2018 at 16:21, on Zulip):

the thing that is confusing me is that impl X doesn't take a type

nikomatsakis (Oct 23 2018 at 16:21, on Zulip):

but rather trait bounds

nikomatsakis (Oct 23 2018 at 16:21, on Zulip):

we don't e.g. permit X: dyn Debug

nikomatsakis (Oct 23 2018 at 16:21, on Zulip):

(do we?)

kennytm (Oct 23 2018 at 16:52, on Zulip):

the thing that is confusing me is that impl X doesn't take a type

i think they means impl dyn TraitAlias { fn stuff() ... }

nikomatsakis (Oct 23 2018 at 17:24, on Zulip):

sorry I meant fn foo(x: &impl T)

nikomatsakis (Oct 23 2018 at 17:24, on Zulip):

but I see

nikomatsakis (Oct 23 2018 at 17:24, on Zulip):

ok ok

nikomatsakis (Oct 23 2018 at 17:25, on Zulip):

I was just misreading

nikomatsakis (Oct 23 2018 at 17:25, on Zulip):

well, in that case, seems fine

nikomatsakis (Oct 23 2018 at 17:25, on Zulip):

or at least potentially fine

nikomatsakis (Oct 23 2018 at 17:25, on Zulip):

can you do impl SomeTypeAlias { .. }... I think you can

nikomatsakis (Oct 23 2018 at 17:25, on Zulip):

(yes you can)

Alexander Regueiro (Oct 23 2018 at 17:57, on Zulip):

@nikomatsakis yep, right.

Alexander Regueiro (Oct 23 2018 at 17:57, on Zulip):

so we're on the same page now I trust? :-)

Alexander Regueiro (Oct 23 2018 at 17:58, on Zulip):

and yes, impl TraitAlias is always valid in 2015 edition, though in the 2018 edition with lints on, you need to do impl dyn TraitAlias

Alexander Regueiro (Oct 23 2018 at 17:58, on Zulip):

which I think is much clearer

Alexander Regueiro (Oct 23 2018 at 17:59, on Zulip):

and consistent, in any case, I reckon.

Alexander Regueiro (Oct 23 2018 at 17:59, on Zulip):

anyway, if you do get time to review and r+ today, that would be super!

Alexander Regueiro (Oct 23 2018 at 18:00, on Zulip):

would love to finally (finally!) start thinking more about upcasting & multi-trait objects... maybe would be a good idea to finish impl-trait-in-bindings though, once you leave the notes on that too (I know, sorry...)

nikomatsakis (Oct 23 2018 at 18:23, on Zulip):

@Alexander Regueiro I left a comment — I think the code looks great, but I'd like to make sure we are exhaustively testing All The Cases. As it my standard practice for new RFCs (well, sometimes I fail to do it, but I always like to do it), I went through the RFC and tried to write-up a kind of comprehensive list of all the things that seemed worth testing. Do you think you could compare what is in the PR and make sure it lines up? See comment here.

nikomatsakis (Oct 23 2018 at 18:23, on Zulip):

which I think is much clearer

yep, seems good

Alexander Regueiro (Oct 23 2018 at 19:07, on Zulip):

@nikomatsakis Okay great, thanks. I’ll try to add to the tests when I get back tonight, then ping you.

nikomatsakis (Oct 23 2018 at 19:10, on Zulip):

:tada:

Alexander Regueiro (Oct 23 2018 at 20:16, on Zulip):

@nikomatsakis When do you think you can write up the notes on impl trait in bindings btw... tomorrow?

nikomatsakis (Oct 23 2018 at 20:16, on Zulip):

maybe

nikomatsakis (Oct 23 2018 at 20:16, on Zulip):

I'll try

Alexander Regueiro (Oct 23 2018 at 20:17, on Zulip):

Thanks. :-)

Alexander Regueiro (Oct 25 2018 at 16:12, on Zulip):

@nikomatsakis Thanks for the notes on impl Trait in bindings!

Alexander Regueiro (Oct 25 2018 at 16:12, on Zulip):

Did you see my reply on the trait alias issue btw?

nikomatsakis (Oct 25 2018 at 16:12, on Zulip):

@Alexander Regueiro :+1: I'll try to post more later on, that was as far as I got :)

I did not see your reply, link?

Alexander Regueiro (Oct 25 2018 at 16:13, on Zulip):

okay great, thanks

Alexander Regueiro (Oct 25 2018 at 16:13, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/55101#issuecomment-432869298

Alexander Regueiro (Oct 25 2018 at 16:14, on Zulip):

I get the feeling we're not too far away, but yeah, those tests definitely exposed an issue.

nikomatsakis (Oct 25 2018 at 16:15, on Zulip):

great :)

nikomatsakis (Oct 25 2018 at 16:16, on Zulip):

I'll take a look

nikomatsakis (Oct 25 2018 at 16:17, on Zulip):

@Alexander Regueiro are you referring to trait-alias-object-type.rs? If so, how does it fail when you build locally? (The travis output doesn't show the error you get)

Alexander Regueiro (Oct 25 2018 at 16:19, on Zulip):

the Travis output is really unclear unfortunately

Alexander Regueiro (Oct 25 2018 at 16:19, on Zulip):

it stringifies stderr

Alexander Regueiro (Oct 25 2018 at 16:19, on Zulip):

quite annoying

Alexander Regueiro (Oct 25 2018 at 16:20, on Zulip):

just a min...

Alexander Regueiro (Oct 25 2018 at 17:08, on Zulip):

@nikomatsakis I commented with the error msgs

Alexander Regueiro (Oct 25 2018 at 17:08, on Zulip):

do you have any ideas about the FIXME too, btw? maybe it doesn't need to be fixed

Alexander Regueiro (Oct 25 2018 at 17:08, on Zulip):

also, I presume not supporting bounds on LHS type parameters is okay?

nikomatsakis (Oct 25 2018 at 17:31, on Zulip):

will take a look in a bit @Alexander Regueiro when I do reviews

Alexander Regueiro (Oct 25 2018 at 17:32, on Zulip):

@nikomatsakis sure :-)

Alexander Regueiro (Oct 25 2018 at 20:59, on Zulip):

@nikomatsakis around now by chance?

nikomatsakis (Oct 25 2018 at 21:05, on Zulip):

@Alexander Regueiro yes

Alexander Regueiro (Oct 25 2018 at 21:16, on Zulip):

So... if you've got a minute to go over some of these errors, that would be great

Alexander Regueiro (Oct 25 2018 at 21:17, on Zulip):

I find interactive discussion about this more helpful. :-)

Alexander Regueiro (Oct 25 2018 at 21:17, on Zulip):

Saves you writing a long post too.

Alexander Regueiro (Oct 25 2018 at 21:17, on Zulip):

Did you see the error logs I posted though, @nikomatsakis ?

nikomatsakis (Oct 25 2018 at 21:17, on Zulip):

I saw it

nikomatsakis (Oct 25 2018 at 21:17, on Zulip):

I just don't know the cause yet :)

nikomatsakis (Oct 25 2018 at 21:17, on Zulip):

the error had to do with impl TraitAlias, right?

nikomatsakis (Oct 25 2018 at 21:17, on Zulip):

(at least one of them)

nikomatsakis (Oct 25 2018 at 21:19, on Zulip):

this is why I kicked off a local build of your branch, so I could poke a bit more easily :)

nikomatsakis (Oct 25 2018 at 21:19, on Zulip):

actually, that build is done now...

Alexander Regueiro (Oct 25 2018 at 21:25, on Zulip):

okay cool

Alexander Regueiro (Oct 25 2018 at 21:25, on Zulip):

I have a build done here too

Alexander Regueiro (Oct 25 2018 at 21:25, on Zulip):

and yeah, one of them to do with impl TraitAlias

Alexander Regueiro (Oct 25 2018 at 21:25, on Zulip):

perhaps it's not generating the candidates properly?

nikomatsakis (Oct 25 2018 at 21:26, on Zulip):

btw, thanks for putting in the effort to square away the tests

nikomatsakis (Oct 25 2018 at 21:28, on Zulip):

hmm

nikomatsakis (Oct 25 2018 at 21:28, on Zulip):

I think that test may just be broken

Alexander Regueiro (Oct 25 2018 at 21:29, on Zulip):

no worries. it's the least I could do after you carefully enumerated the things to be tested!

Alexander Regueiro (Oct 25 2018 at 21:29, on Zulip):

yeah, I was considering that too

Alexander Regueiro (Oct 25 2018 at 21:29, on Zulip):

wasn't 100% sure on expected behaviour

Alexander Regueiro (Oct 25 2018 at 21:29, on Zulip):

are you okay with LHS type bounds not being allowed by the way?

nikomatsakis (Oct 25 2018 at 21:37, on Zulip):

are you okay with LHS type bounds not being allowed by the way?

not sure yet :)

nikomatsakis (Oct 25 2018 at 21:41, on Zulip):

I am not a big fan of these "mega tests"

nikomatsakis (Oct 25 2018 at 21:41, on Zulip):

they're easy to write, but a pain later...

nikomatsakis (Oct 25 2018 at 21:41, on Zulip):
trait SendEqAlias<T> = Send where T: PartialEq<Self>;

fn b(x: &impl SendEqAlias<i32>) -> bool {
    22_i32 == *x
}

fn main() { }
nikomatsakis (Oct 25 2018 at 21:41, on Zulip):

I think you can simplify the test in question to that

nikomatsakis (Oct 25 2018 at 21:41, on Zulip):

PartialEq::eq(&22_i32, x) also fails...

nikomatsakis (Oct 25 2018 at 21:42, on Zulip):

( I reversed the order incidentally of the lhs/rhs, which I think is correct )

nikomatsakis (Oct 25 2018 at 21:44, on Zulip):

ah...hmmm....

nikomatsakis (Oct 25 2018 at 21:45, on Zulip):

ok so I think I see the problem. I have to ponder it :)

Alexander Regueiro (Oct 25 2018 at 21:46, on Zulip):

I am not a big fan of these "mega tests"

sorry... I'm not very good writing lots of test cases. feel free to split them up, if you want. I'd be a bit lost (you have push permissions on the PR)

nikomatsakis (Oct 25 2018 at 21:46, on Zulip):

heh sorry no worries

nikomatsakis (Oct 25 2018 at 21:46, on Zulip):

I make them all the time too ;)

nikomatsakis (Oct 25 2018 at 21:46, on Zulip):

it's just hard to debug

nikomatsakis (Oct 25 2018 at 21:46, on Zulip):

because then when you run with RUST_LOG

Alexander Regueiro (Oct 25 2018 at 21:46, on Zulip):

@nikomatsakis also, I presume you saw the discussion about LHS bounds here: https://github.com/rust-lang/rfcs/blob/master/text/1733-trait-alias.md#what-about-bounds-on-type-variable-declaration-in-the-trait-alias -- I'll wait for your thoughts on that

nikomatsakis (Oct 25 2018 at 21:46, on Zulip):

you get lots of logs mixed in

Alexander Regueiro (Oct 25 2018 at 21:46, on Zulip):

yeah, you're not wrong

nikomatsakis (Oct 25 2018 at 21:47, on Zulip):

the problem has to do with elaboration

Alexander Regueiro (Oct 25 2018 at 21:47, on Zulip):

ah

nikomatsakis (Oct 25 2018 at 21:47, on Zulip):

(maybe we just forgot to do that?)

Alexander Regueiro (Oct 25 2018 at 21:47, on Zulip):

maybe?

Alexander Regueiro (Oct 25 2018 at 21:47, on Zulip):

where is that done in the compiler?

Alexander Regueiro (Oct 25 2018 at 21:47, on Zulip):

and what sort of elaboration are we talking about here?

nikomatsakis (Oct 25 2018 at 21:48, on Zulip):

src/librustc/traits/util.rs, the fn elaborate_predicates

nikomatsakis (Oct 25 2018 at 21:48, on Zulip):

it has the job of "expanding" things like T: Foo to include all the steps they imply

nikomatsakis (Oct 25 2018 at 21:48, on Zulip):

e.g. it would elaborate T: Eq to T: PartialEq + Eq

nikomatsakis (Oct 25 2018 at 21:49, on Zulip):

we presently only elaborate supertraits

nikomatsakis (Oct 25 2018 at 21:49, on Zulip):

but for trait aliases we .. probably .. want to alter that rule

Alexander Regueiro (Oct 25 2018 at 21:49, on Zulip):

ah

nikomatsakis (Oct 25 2018 at 21:50, on Zulip):

alternatively we could declare all the parts to be where clauses

nikomatsakis (Oct 25 2018 at 21:50, on Zulip):

@nikomatsakis also, I presume you saw the discussion about LHS bounds here: https://github.com/rust-lang/rfcs/blob/master/text/1733-trait-alias.md#what-about-bounds-on-type-variable-declaration-in-the-trait-alias -- I'll wait for your thoughts on that

yes, I remember, i'll have to revisit and think it over

nikomatsakis (Oct 25 2018 at 21:50, on Zulip):

gotta run now :)

nikomatsakis (Oct 25 2018 at 21:50, on Zulip):

I can leave some more comments later

Alexander Regueiro (Oct 25 2018 at 21:50, on Zulip):

no worries

Alexander Regueiro (Oct 25 2018 at 21:50, on Zulip):

thanks for your time. look forward to the comments later!

Alexander Regueiro (Oct 25 2018 at 21:51, on Zulip):

also, on the FIXME in code and the associated type failure... but let's tackle this first

nikomatsakis (Oct 25 2018 at 22:02, on Zulip):

(afk but I just realized — we're gonna have some problems with -> impl TraitAlias if they use where clauses, but that's ok — chalk would help)

nikomatsakis (Oct 25 2018 at 22:02, on Zulip):

just leaving that comment as a note to myself :)

Alexander Regueiro (Oct 25 2018 at 22:15, on Zulip):

Yep... I think this probably won't stabilise until post-Chalk integration, right? So we should be okay.

Alexander Regueiro (Oct 25 2018 at 22:25, on Zulip):

okay, I think I figured out the associated type issue

Alexander Regueiro (Oct 25 2018 at 22:26, on Zulip):

for the PolyTraitRef in HIR for the TraitObject, bound_generic_params is always just initialised to an empty vector

Alexander Regueiro (Oct 25 2018 at 22:48, on Zulip):

might be the cause of the other failure too, in fact

Alexander Regueiro (Oct 25 2018 at 22:49, on Zulip):

re. PartialEq<Self>

Alexander Regueiro (Oct 26 2018 at 00:10, on Zulip):

okay, it's an issue to do with elaboration of projected types

Alexander Regueiro (Oct 26 2018 at 00:18, on Zulip):

@nikomatsakis the key here is the create_substs_for_ast_path function I think. it needs to look at supertraits too (since supertraits of a trait alias will include all the trait aliases/traits/associated items/etc. up the chain

Alexander Regueiro (Oct 26 2018 at 16:04, on Zulip):

@nikomatsakis coming back to this just now. does the above make sense to you?

nikomatsakis (Oct 26 2018 at 18:39, on Zulip):

okay, I think I figured out the associated type issue

Ah yes, I think this is what @Alex Burka and I were talking about when we discussed how trait objects would need to "see through" aliases for this purpose. TBH, this is a more general bug with trait objects, and it'd be nice to fix it more generally.

nikomatsakis (Oct 26 2018 at 19:07, on Zulip):

@Alexander Regueiro left a comment here for the supertrait elaboration

nikomatsakis (Oct 26 2018 at 19:11, on Zulip):

now left a comment on the other half :) in short, I think this is #24010, and I'd prefer to solve both problems at once — perhaps in a second PR?

Alexander Regueiro (Oct 26 2018 at 19:29, on Zulip):

@nikomatsakis okay, sounds good, thanks

Alexander Regueiro (Oct 26 2018 at 19:38, on Zulip):

@nikomatsakis what about the existing FIXME?

Alexander Regueiro (Oct 26 2018 at 21:44, on Zulip):

@nikomatsakis so, I need to call ast_ty_to_ty (actually to_ty on the ItemCtxt which then calls that) in the case the type parameter is not Self. apparently this isn't called for Self, to avoid cycles. do I need to be careful to avoid cycles here too? maybe memoize the conversions... at what level, if so?

Alexander Regueiro (Oct 26 2018 at 22:06, on Zulip):

I've got it working here, but you can take a look at the code and tell me if it's the right approach. :-)

nikomatsakis (Oct 29 2018 at 17:45, on Zulip):

@Alexander Regueiro now is ok, I'm just wrapping up a few things

nikomatsakis (Oct 29 2018 at 17:45, on Zulip):

what's on your mind

nikomatsakis (Oct 29 2018 at 17:45, on Zulip):

er, I see you left me some questions I forgot about

nikomatsakis (Oct 29 2018 at 17:45, on Zulip):

is that the context? :)

Alexander Regueiro (Oct 29 2018 at 17:46, on Zulip):

yeah, basically

Alexander Regueiro (Oct 29 2018 at 17:46, on Zulip):

I an explain to you what I discussed with @centril last night if you like though

nikomatsakis (Oct 29 2018 at 17:46, on Zulip):

I forgot what the existing FIXME is about

Alexander Regueiro (Oct 29 2018 at 17:48, on Zulip):

basically just https://github.com/rust-lang/rust/pull/55101#issuecomment-433772151

Alexander Regueiro (Oct 29 2018 at 17:49, on Zulip):

sorry, I mean https://github.com/rust-lang/rust/pull/55101#issuecomment-433572568

Alexander Regueiro (Oct 29 2018 at 17:49, on Zulip):

the first was the other one

Alexander Regueiro (Oct 29 2018 at 17:58, on Zulip):

@nikomatsakis my proposal (which @centril thought would work too) was to disallow implication bounds, and only have pre-condition bounds, which fits in with the rest of Rust well.

trait Bar {}
trait Foo<T> = Bar<T>; // good
trait Foo<T: Send> = Bar<T>; // good
trait Foo<T> = Bar<T> where T: Send; // good
trait Foo<T> = Bar<T: Send>; // bad
nikomatsakis (Oct 29 2018 at 18:02, on Zulip):

I don't really understand that last example

nikomatsakis (Oct 29 2018 at 18:02, on Zulip):

what syntax is that?

nikomatsakis (Oct 29 2018 at 18:02, on Zulip):

I am still feeling a bit confused about how to think about the well-formedness conditions in this context

nikomatsakis (Oct 29 2018 at 18:02, on Zulip):

like, which things do "users" of the alias have to prove

Alexander Regueiro (Oct 29 2018 at 18:02, on Zulip):

it would be implied bounds, as mentioned in the RFC

Alexander Regueiro (Oct 29 2018 at 18:03, on Zulip):

i.e. "bundle" the bounds with the alias, so they don't have to be specified at use site

Alexander Regueiro (Oct 29 2018 at 18:03, on Zulip):

but I don't think we want that

Alexander Regueiro (Oct 29 2018 at 18:03, on Zulip):

I would rather make all bounds on the LHS

Alexander Regueiro (Oct 29 2018 at 18:03, on Zulip):

and have to specify them at use site

nikomatsakis (Oct 29 2018 at 18:03, on Zulip):

it would be implied bounds, as mentioned in the RFC

trait Foo<T> = Bar<T: Send> this syntax was proposed in the RFC?

Alexander Regueiro (Oct 29 2018 at 18:04, on Zulip):

yeah, I think so. in the final section

Alexander Regueiro (Oct 29 2018 at 18:04, on Zulip):

it was discussed

Alexander Regueiro (Oct 29 2018 at 18:04, on Zulip):

anyway, interpret it as an implication bound

Alexander Regueiro (Oct 29 2018 at 18:04, on Zulip):

whereas trait Foo<T: Send> = Bar<T> would be a pre-condition bound

Alexander Regueiro (Oct 29 2018 at 18:17, on Zulip):

@nikomatsakis make sense?

Alexander Regueiro (Oct 29 2018 at 18:41, on Zulip):

?

nikomatsakis (Oct 29 2018 at 18:41, on Zulip):

I see

nikomatsakis (Oct 29 2018 at 18:43, on Zulip):

That said, with the implied bounds plans that we had in mind, I believe (@scalexm can perhaps confirm) that

trait Foo<T: Send> { .. }

fn bar<T: Foo<U>, U>() { .. }

the function bar in this example would be able to assume that U: Send (because T: Foo<U> and Foo requires that U: Send)

nikomatsakis (Oct 29 2018 at 18:43, on Zulip):

so I am wondering how important this distinction is

scalexm (Oct 29 2018 at 18:43, on Zulip):

right

nikomatsakis (Oct 29 2018 at 18:44, on Zulip):

@scalexm here is a question though

nikomatsakis (Oct 29 2018 at 18:45, on Zulip):

if you have

trait Foo<T: Send> { .. }

trait Bar<U> : Foo<T> { .. }

is that an error? (Because the where-clause Self: Foo<T> is not well-formed)

nikomatsakis (Oct 29 2018 at 18:45, on Zulip):

I believe that

struct Foo<T: Send> { .. }

struct Bar<T> {
  f: Foo<T>
}

would be ill-formed

scalexm (Oct 29 2018 at 18:45, on Zulip):

yes it is an error

scalexm (Oct 29 2018 at 18:45, on Zulip):

in traits, we check that where clauses are "self-consistent"

nikomatsakis (Oct 29 2018 at 18:45, on Zulip):

yes

nikomatsakis (Oct 29 2018 at 18:46, on Zulip):

this is what I was imagining

nikomatsakis (Oct 29 2018 at 18:46, on Zulip):

so @Alexander Regueiro we might do a similar check for trait aliases; I'll have to revist what the RFC said but I imagine we left it as a kind of "open question"

nikomatsakis (Oct 29 2018 at 18:46, on Zulip):

I think modeling things fairly closely after a "trait known to have one impl" will not lead us astray

nikomatsakis (Oct 29 2018 at 18:48, on Zulip):

that is, trait Foo<P...> = where WC is equivalent to

trait Foo<P...> where WC { }
impl<Self, P...> Foo<P...> for Self where WC { .. }

actually, with implied bounds, it is probably exactly equivalent to that...

scalexm (Oct 29 2018 at 18:48, on Zulip):

@nikomatsakis mmh let me back out for a second about this, I'm not even sure it would be an error now

scalexm (Oct 29 2018 at 18:49, on Zulip):

especially since I did not implement WF checking of trait decls in chalk :p

nikomatsakis (Oct 29 2018 at 18:49, on Zulip):

one place that will not work right now

nikomatsakis (Oct 29 2018 at 18:49, on Zulip):

sorry, not sure what you mean by "now"

nikomatsakis (Oct 29 2018 at 18:49, on Zulip):

error in chalk or rustc :)

scalexm (Oct 29 2018 at 18:49, on Zulip):

now = now I think about this lol

nikomatsakis (Oct 29 2018 at 18:51, on Zulip):

I see :)

nikomatsakis (Oct 29 2018 at 18:51, on Zulip):

well, in short, I think it's a good idea for us to figure out just how trait aliases fit in and that should guide us

Alexander Regueiro (Oct 29 2018 at 18:51, on Zulip):

@nikomatsakis yes, I agree. that will allow us to check the bounds properly at definition point of the alias too, I think?

scalexm (Oct 29 2018 at 18:52, on Zulip):

@nikomatsakis I'll think more deeply about this, but it seems to me that this shouldn't be an error actually, it should rather be up to a Bar implementor to prove that T: Send hold

Alexander Regueiro (Oct 29 2018 at 18:52, on Zulip):

@nikomatsakis want to think about it and leave notes here/on the PR later? hopefully we can reuse most of the existing code.

tmandry (Oct 29 2018 at 18:52, on Zulip):

FWIW I find this line confusing, anyway

trait Foo<T> = Bar<T: Send>;
Alexander Regueiro (Oct 29 2018 at 18:52, on Zulip):

we'll need to adapt the selection routine for trait alias of course

Alexander Regueiro (Oct 29 2018 at 18:53, on Zulip):

@tmandry it's not confusing if Send is an implied bound, but yes, it would require education. doesn't really have an analogue in existing Rust, whereas trait Foo<T: Send> does

nikomatsakis (Oct 29 2018 at 18:54, on Zulip):

I'll think more deeply about this, but it seems to me that this shouldn't be an error actually, it should rather be up to a Bar implementor to prove that T: Send hold

ok, let's circle back, I also could see it going either way

Alexander Regueiro (Oct 29 2018 at 19:01, on Zulip):

@nikomatsakis how do you mean?

Alexander Regueiro (Oct 29 2018 at 20:17, on Zulip):

@nikomatsakis ?

Alexander Regueiro (Oct 30 2018 at 15:57, on Zulip):

Had any more thoughts about the above @nikomatsakis? I wasn't quite sure what you meant yesterday.

nikomatsakis (Oct 30 2018 at 17:08, on Zulip):

I guess the questions at hand are:

What do we need to do to land this PR?

nikomatsakis (Oct 30 2018 at 17:08, on Zulip):

We basically hit two bugs -- one of which you kind of solved by changing elaboration -- both of which were effectively pre-existing

nikomatsakis (Oct 30 2018 at 17:09, on Zulip):

The main thing I am wondering is whether we ought to "rollback" that elaboration fix and consider it separately, since it ultimately is a "special case hack" to fix https://github.com/rust-lang/rust/issues/20671

nikomatsakis (Oct 30 2018 at 17:09, on Zulip):

otoh if it works for now, maybe we should just leave it

nikomatsakis (Oct 30 2018 at 21:20, on Zulip):

@Alexander Regueiro sorry I'll get back to this tomorrow and try to pick a specific path fwd, but I think the TL;DR is that we should land the PR either "as is" or else by reverting slightly and spinning some of that into a separat PR

Alexander Regueiro (Oct 30 2018 at 21:40, on Zulip):

@nikomatsakis Okay. What about bounds checking for trait alias definitions which we still need to enforce?

nikomatsakis (Oct 31 2018 at 16:08, on Zulip):

That's what I'm trying to decide, I plan to spend a few minutes and get back to you today on that :)

Alexander Regueiro (Oct 31 2018 at 16:16, on Zulip):

@nikomatsakis Okay, perfect. Look forward to it. :-)

Alexander Regueiro (Oct 31 2018 at 16:51, on Zulip):

at least I'm learning a lot about the trait system here...

Alexander Regueiro (Oct 31 2018 at 16:51, on Zulip):

will have to relearn a lot when Chalk lands of course, but oh well.

Alexander Regueiro (Oct 31 2018 at 18:09, on Zulip):

@nikomatsakis heading off for a bit now, and going to have dinner, but feel free to leave notes/advice here or on Github. if you're around when I'm back, we can talk then.

Alexander Regueiro (Oct 31 2018 at 20:41, on Zulip):

@nikomatsakis back

nikomatsakis (Oct 31 2018 at 20:41, on Zulip):

ok, I've been looking at it, writing up some notes now, trying to decide what I think =)

nikomatsakis (Oct 31 2018 at 20:42, on Zulip):

gonna have to run soon because :ghost: halloween :ghost:

Alexander Regueiro (Oct 31 2018 at 20:42, on Zulip):

Okay, thanks!

Alexander Regueiro (Oct 31 2018 at 20:43, on Zulip):

Hah fair. We don't celebrate it much over here. At least, adults don't. But enjoy!

nikomatsakis (Oct 31 2018 at 20:52, on Zulip):

hmm

nikomatsakis (Oct 31 2018 at 20:52, on Zulip):

so I'm probably not going to be done :)

nikomatsakis (Oct 31 2018 at 20:52, on Zulip):

I've got a local build

nikomatsakis (Oct 31 2018 at 20:53, on Zulip):

there are my stream of consciousness notes in this gist

nikomatsakis (Oct 31 2018 at 20:53, on Zulip):

I'm a bit confused about one part of the behavior and debugging a bit now

nikomatsakis (Oct 31 2018 at 20:54, on Zulip):

@Alexander Regueiro wait I must be missing a core assumption here :)

nikomatsakis (Oct 31 2018 at 20:55, on Zulip):

I thought that tcx.predicates_of(def_id_of_Some_trait_alias) would give you the things of the alias

nikomatsakis (Oct 31 2018 at 20:55, on Zulip):

but .. that doesn't seem to be true?

nikomatsakis (Oct 31 2018 at 20:55, on Zulip):

i.e., trait Foo = Bar, I expect predicates_of(Foo) to be [Self: Bar], but instead it is []

nikomatsakis (Oct 31 2018 at 20:56, on Zulip):

this I think is the reason for the unsoundness, or at least part of it :)

Alexander Regueiro (Oct 31 2018 at 21:01, on Zulip):

@nikomatsakis okay, I could work off that, thanks

nikomatsakis (Oct 31 2018 at 21:01, on Zulip):

I think the problem is in the explicit_predicates_of query, which has this logic:

                ItemKind::Trait(_, _, ref generics, .., ref items) => {
                    is_trait = Some((ty::TraitRef::identity(tcx, def_id), items));
                    generics
                }

in librustc_typeck/collect.rs on line 1719 but nothing for TraitAlias

nikomatsakis (Oct 31 2018 at 21:01, on Zulip):

ok, leaving now :)

Alexander Regueiro (Oct 31 2018 at 21:21, on Zulip):

@nikomatsakis last thing: is the current logic for considering all T: Foo bounds as opposed to just Self: Foo correct, or not?

Alexander Regueiro (Oct 31 2018 at 21:21, on Zulip):

anyway, thanks. enjoy the night!

Alexander Regueiro (Nov 01 2018 at 03:06, on Zulip):

okay, I've solved unsoundness thanks to your above tip! I definitely thought of that before, but then left it aside for some reason... it makes sense why it's needed now.

Alexander Regueiro (Nov 01 2018 at 03:08, on Zulip):

@nikomatsakis now we want to a) make sure this checking is done at the definition of the trait alias rather than the use of it

Alexander Regueiro (Nov 01 2018 at 03:08, on Zulip):

b) more deeply dereference aliases for trait objects, as you suggested in a PR comment

Alexander Regueiro (Nov 01 2018 at 03:08, on Zulip):

it's late here, so I probably won't even get to thinking about it until tomorrow

Alexander Regueiro (Nov 01 2018 at 03:09, on Zulip):

feel free to leave tips in the meanwhile. I should be around at some point

Alexander Regueiro (Nov 01 2018 at 03:09, on Zulip):

(I've pushed now, BTW)

Alexander Regueiro (Nov 01 2018 at 04:11, on Zulip):

I think we want to dereference aliases for trait objects in conv_object_ty_poly_trait_ref (or thereabouts), to solve that second bug, but not sure exactly how.

nikomatsakis (Nov 01 2018 at 16:20, on Zulip):

ok, coming back to this now

nikomatsakis (Nov 01 2018 at 16:52, on Zulip):

@Alexander Regueiro btw, I think it'd be helpful to take that testing checklist and persist it somewhere, perhaps in the tracking issue, with indications of which test are testing which pattern

nikomatsakis (Nov 01 2018 at 16:52, on Zulip):

it seems like the main problem now is the "trait object equivalence problem"?

nikomatsakis (Nov 01 2018 at 16:53, on Zulip):

that is, this one:

trait A<T: Send> {}
trait B<T: Send> = A<T>;

struct Foo<T>(T);
struct Bar();

impl<T: Send> A<T> for Foo<T> {}

impl !Send for Bar {}

fn main() {
    let b: Box<dyn B<Bar>> = Box::new(Foo(Bar()));
    let a: Box<dyn A<Bar>> = b;
}
nikomatsakis (Nov 01 2018 at 16:53, on Zulip):

I'm trying to decide if this is a pre-existing problem or not, to some extent :)

nikomatsakis (Nov 01 2018 at 16:53, on Zulip):

there was also the matter of specifying associated type values, right?

nikomatsakis (Nov 01 2018 at 16:55, on Zulip):

which is definitely pre-existing

nikomatsakis (Nov 01 2018 at 16:55, on Zulip):

to some extent, the "trait object equivalence" problem is equal to trait upcasting, but that's also a bit different

nikomatsakis (Nov 01 2018 at 16:55, on Zulip):

it's sort of a special case of upcasting in which there is a cycle

nikomatsakis (Nov 01 2018 at 16:59, on Zulip):

and of course it only sort of "makes sense" for the special case of trait Foo = Bar -- not e.g. trait Foo = Bar + Baz, though maybe for trait Foo = Bar + Send

nikomatsakis (Nov 01 2018 at 17:02, on Zulip):

I think @Alexander Regueiro I personally feel content to leave trait objects as "fixme" items for the purposes of this PR, though we should probably just turn to discussing how to fix

nikomatsakis (Nov 01 2018 at 17:10, on Zulip):

I think the idea would be that right around here, you would want to check the principal result and see if the def-id is a trait alias. If so, we need check it for object safety (which we are presently doing shortly thereafter). Assuming it passes, we would "expand it" into the supertraits (or predicates) before converting to the existential predicates. We also need to check that it expands to something we can represent in a trait object, so one "main" trait and various auto traits

nikomatsakis (Nov 01 2018 at 17:11, on Zulip):

This seems complex enough I might rather see it in its own PR

Alexander Regueiro (Nov 01 2018 at 17:32, on Zulip):

@Alexander Regueiro btw, I think it'd be helpful to take that testing checklist and persist it somewhere, perhaps in the tracking issue, with indications of which test are testing which pattern

yes I thought about that... will do it shortly

Alexander Regueiro (Nov 01 2018 at 17:32, on Zulip):

there was also the matter of specifying associated type values, right?

I think that's solved now? I forget, let me check

Alexander Regueiro (Nov 01 2018 at 17:35, on Zulip):

@nikomatsakis ah no, you're right of course: that still fails (but we said we'd delay it to a subsequent PR, since it's a pre-existing bug)

Alexander Regueiro (Nov 01 2018 at 17:35, on Zulip):

I commented out the test line for that

Alexander Regueiro (Nov 01 2018 at 17:36, on Zulip):

@nikomatsakis anyway, I agree. let's put in a FIXME for this test too, and try to land the PR, then turn to these two issues.

Alexander Regueiro (Nov 01 2018 at 17:36, on Zulip):

I wonder if it is worth addressing upcasting properly at this point...

Alexander Regueiro (Nov 01 2018 at 17:36, on Zulip):

since we discussed it anyway at some point in the past, though I was too busy to ever get around to making notes on it...

nikomatsakis (Nov 01 2018 at 17:41, on Zulip):

@Alexander Regueiro so the other question is the wfcheck rules... @scalexm and I were just discussing that over in the implied-bounds topic

Alexander Regueiro (Nov 01 2018 at 17:42, on Zulip):

@nikomatsakis oh, how could I forget heh!

Alexander Regueiro (Nov 02 2018 at 18:27, on Zulip):

@nikomatsakis also could you r+ my PR now please? Provifing the answer to my above question about impl items is not importt.

nikomatsakis (Nov 02 2018 at 18:30, on Zulip):

@Alexander Regueiro I wanted to do a review of tests and also setup issues for the "remaining items" first, which I hope to do shortly

Alexander Regueiro (Nov 02 2018 at 18:47, on Zulip):

Okay

Alexander Regueiro (Nov 02 2018 at 18:48, on Zulip):

What about my above question on ItemlKind::Impl?

nikomatsakis (Nov 02 2018 at 20:44, on Zulip):

@Alexander Regueiro to confirm, we did not (yet) fix this scenario, right:

If you have trait Foo { } and trait Bar = Foo, we do not currently consider dyn Foo and dyn Bar to be the same type.

nikomatsakis (Nov 02 2018 at 20:44, on Zulip):

Do we have an example of this? I was going to file an issue dedicated to it

Alexander Regueiro (Nov 02 2018 at 20:44, on Zulip):

@nikomatsakis correct. you can take the sample code from the PR if you like... it should probably be bundled in with upcasting

nikomatsakis (Nov 02 2018 at 20:45, on Zulip):

@Alexander Regueiro can you please attach the code to https://github.com/rust-lang/rust/issues/55629 for me?

Alexander Regueiro (Nov 02 2018 at 20:45, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/55101#issuecomment-433772151

Alexander Regueiro (Nov 02 2018 at 20:45, on Zulip):

sure

nikomatsakis (Nov 02 2018 at 20:45, on Zulip):

Thanks -- also @Alexander Regueiro if you can review https://github.com/rust-lang/rust/issues/55628 to see if there is anything else you know of that ought to be listed there as "pending to do items" it would be great

nikomatsakis (Nov 02 2018 at 20:46, on Zulip):

in general, attaching example programs for each failing is probably also awesome, if we have them

nikomatsakis (Nov 02 2018 at 20:47, on Zulip):

@Alexander Regueiro earlier you wrote this

Okay, I've added all the above tests (except for the ones that bound the type parameter on the LHS, since we've decided not to allow that in trait aliases, AFAIK)

Do we still disallow this?

nikomatsakis (Nov 02 2018 at 20:47, on Zulip):

Did you add the tests that use a where clause, in any case? I guess I will try to review and figure that out

nikomatsakis (Nov 02 2018 at 20:47, on Zulip):

that's the last thing I'd like to do before merging :)

Alexander Regueiro (Nov 02 2018 at 20:47, on Zulip):

@nikomatsakis oh no, I've enabled it now :-)

nikomatsakis (Nov 02 2018 at 20:47, on Zulip):

I may btw push a few minor commits to your branch :)

Alexander Regueiro (Nov 02 2018 at 20:47, on Zulip):

a lot has changed since that comment heh

Alexander Regueiro (Nov 02 2018 at 20:47, on Zulip):

yeah, go for it.

nikomatsakis (Nov 02 2018 at 20:49, on Zulip):

trying to get this r+'d before I leave for the day :)

Alexander Regueiro (Nov 02 2018 at 20:49, on Zulip):

@nikomatsakis just left two comments on your new issue

Alexander Regueiro (Nov 02 2018 at 20:49, on Zulip):

that would be cool.

Alexander Regueiro (Nov 02 2018 at 20:49, on Zulip):

what time zone are you btw? east coast US?

Alexander Regueiro (Nov 02 2018 at 20:55, on Zulip):

maybe change https://gist.github.com/nikomatsakis/e4dd2807581fc868ba308382855e68f6 to Markdown format btw?

nikomatsakis (Nov 02 2018 at 20:55, on Zulip):

ok, so, it appears I will not get the tests checked as I had hoped

nikomatsakis (Nov 02 2018 at 20:56, on Zulip):

I have to run now and i'm not that far :P

nikomatsakis (Nov 02 2018 at 20:56, on Zulip):

I created this HackMD document from the gist

nikomatsakis (Nov 02 2018 at 20:56, on Zulip):

and I've been attached notes to myself of where each test is

nikomatsakis (Nov 02 2018 at 20:57, on Zulip):

anyway, I gotta go, however, I'm game to r+ and we can do the "test checks" later

nikomatsakis (Nov 02 2018 at 20:57, on Zulip):

doens't really have to be "synchronous"

Alexander Regueiro (Nov 02 2018 at 21:08, on Zulip):

any advice on the associated types stuff or not?

Alexander Regueiro (Nov 02 2018 at 21:09, on Zulip):

anyway, thanks for approving

Alexander Regueiro (Nov 02 2018 at 21:09, on Zulip):

@nikomatsakis

Alexander Regueiro (Nov 05 2018 at 17:45, on Zulip):

Hi @nikomatsakis

Alexander Regueiro (Nov 05 2018 at 17:45, on Zulip):

Get my tags on Discord/GH?

Alexander Regueiro (Nov 05 2018 at 17:56, on Zulip):

@nikomatsakis I'm curious in particular what's making https://github.com/rust-lang/rust/pull/55687#issuecomment-435749000 fail.

scalexm (Nov 06 2018 at 14:48, on Zulip):

@Alexander Regueiro I could probably take a look

Alexander Regueiro (Nov 06 2018 at 14:49, on Zulip):

@scalexm I’d appreciate that.

nikomatsakis (Nov 06 2018 at 15:58, on Zulip):

@Alexander Regueiro why are we running crater on https://github.com/rust-lang/rust/pull/55687, out of curiosity?

nikomatsakis (Nov 06 2018 at 15:59, on Zulip):

Oh, I guess this:

As a by-product, this PR also makes repeated bindings of the same associated item in the same definition a hard error. This was previously a warning with a note about it becoming a hard error in the future. See #50589 for more info.

Alexander Regueiro (Nov 06 2018 at 15:59, on Zulip):

we shouldn't be. we should just be running it on master with -Dduplicate_associated_type_bindings

Alexander Regueiro (Nov 06 2018 at 15:59, on Zulip):

I think we are, just the crater run is attached to that issue?

nikomatsakis (Nov 06 2018 at 16:07, on Zulip):

ok

Alexander Regueiro (Nov 06 2018 at 16:19, on Zulip):

hmm, is there any way to determine which bound / where clause a projection predicate belongs to, from the results of tcx.predicates_of? @nikomatsakis @scalexm

Alexander Regueiro (Nov 06 2018 at 16:22, on Zulip):

I need to distinguish between e.g. Iterator<Item = i32> + Iterator<Item = u32> and Iterator<Item = i32, Item = u32>

Alexander Regueiro (Nov 06 2018 at 16:27, on Zulip):

anyway, I think the more fundamental issue is that calling predicates_of here can lead to cycles

Alexander Regueiro (Nov 06 2018 at 16:27, on Zulip):

hrmm

nikomatsakis (Nov 06 2018 at 16:27, on Zulip):

@Alexander Regueiro I believe we should be calling supertraits_of

nikomatsakis (Nov 06 2018 at 16:28, on Zulip):

that is already required to be acyclic

nikomatsakis (Nov 06 2018 at 16:28, on Zulip):

and that is what we use today for this sort of thing (e.g., to determine the set of assoc types that are in scope)

Alexander Regueiro (Nov 06 2018 at 16:28, on Zulip):

@nikomatsakis you just mean supertraits? I tried that...

nikomatsakis (Nov 06 2018 at 16:28, on Zulip):

I do; what happened when you tried it?

Alexander Regueiro (Nov 06 2018 at 16:30, on Zulip):

@nikomatsakis I presume we then call create_substs_for_ast_trait_ref iterably (that's what I tried)... the issue is, it requires a Span and PathSegment or such

Alexander Regueiro (Nov 06 2018 at 16:30, on Zulip):
            self.create_substs_for_ast_trait_ref(trait_ref.path.span,
                                                 trait_def_id,
                                                 self_ty,
                                                 trait_ref.path.segments.last().unwrap());
Alexander Regueiro (Nov 06 2018 at 16:30, on Zulip):

that's what's used for the direct bindings

Alexander Regueiro (Nov 06 2018 at 16:31, on Zulip):

for supertraits, trait_def_id just gets replaced by tr.def_id() of course, but the other 2 args are more problematic...

Alexander Regueiro (Nov 06 2018 at 16:31, on Zulip):

the supertraitsfn doesn't provide that info, because it works at the type system level, not the HIR level

Alexander Regueiro (Nov 06 2018 at 16:31, on Zulip):

afaik

Alexander Regueiro (Nov 06 2018 at 16:49, on Zulip):

@nikomatsakis @scalexm thoughts?

scalexm (Nov 06 2018 at 17:27, on Zulip):

@Alexander Regueiro you definitely want to use traits::supertraits because this reaches the transitive closure of all the super projection bounds

Alexander Regueiro (Nov 06 2018 at 17:28, on Zulip):

@scalexm yeah, the question is what next...

scalexm (Nov 06 2018 at 17:28, on Zulip):

now, for each projection predicate that you filter from traits::supertraits, I guess you could just substitute with the substitution returned by create_substs_for_ast_trait_ref for your principal trait?

Alexander Regueiro (Nov 06 2018 at 17:29, on Zulip):

hmm?

scalexm (Nov 06 2018 at 17:30, on Zulip):

well something like

scalexm (Nov 06 2018 at 17:32, on Zulip):
poly_projections.extend(traits::supertraits(trait_def_id)
    .into_iter()
    .filter_map(|pred| {
        if let ty::Predicate::Projection(proj) = pred {
            Some(proj)
        } else {
            None
        }
    })
    .map(|proj| proj.subst(substs)
);
scalexm (Nov 06 2018 at 17:33, on Zulip):

I don't know if supertraits return a Span along with each predicate, if not just put a DUMMY_SPAN for now in poly_projections, we're just trying to see if that works

Alexander Regueiro (Nov 06 2018 at 17:33, on Zulip):

yeah I don't think it does

Alexander Regueiro (Nov 06 2018 at 17:33, on Zulip):

let's try though

Alexander Regueiro (Nov 06 2018 at 17:36, on Zulip):

@scalexm this is very similar to something I already tried actually. anyway, the iterator that supertraits returns is of binders...so do I just do skip_binder or something?

scalexm (Nov 06 2018 at 17:37, on Zulip):

since we want PolyProjectionPredicate in our poly_projections vector, you should map the binder instead, we want to preserve it

Alexander Regueiro (Nov 06 2018 at 17:38, on Zulip):

okay sure

scalexm (Nov 06 2018 at 17:40, on Zulip):

ah ok the iterator only returns the trait bounds

Alexander Regueiro (Nov 06 2018 at 17:41, on Zulip):

?

scalexm (Nov 06 2018 at 17:41, on Zulip):

@Alexander Regueiro ok it might be better to call traits::elaborate_trait_ref instead I think

Alexander Regueiro (Nov 06 2018 at 17:41, on Zulip):

does that give spans?

Alexander Regueiro (Nov 06 2018 at 17:41, on Zulip):

/me checks

scalexm (Nov 06 2018 at 17:41, on Zulip):

because supertraits only returns trait refs, we wanted projection predicates at first

scalexm (Nov 06 2018 at 17:42, on Zulip):

it does not give spans but at least it gives all the transitive set of super predicates, including projection predicates (the ones we want)

Alexander Regueiro (Nov 06 2018 at 17:42, on Zulip):

ah yes

Alexander Regueiro (Nov 06 2018 at 17:42, on Zulip):

good point

Alexander Regueiro (Nov 06 2018 at 17:43, on Zulip):

trying that now...

Alexander Regueiro (Nov 06 2018 at 17:43, on Zulip):

spans may be the tricky bit in fact, but that's only for error reporting at least, not correctness

scalexm (Nov 06 2018 at 17:45, on Zulip):

@Alexander Regueiro no actually I think that if that works, the span might be not that hard to get, because internally elaborate_trait_ref uses super_predicates_of which does return the spans, so we will just have to forward that info

scalexm (Nov 06 2018 at 17:45, on Zulip):

currently elaborate_trait_ref just filters out the spans

Alexander Regueiro (Nov 06 2018 at 17:46, on Zulip):

oh I see, cool

Alexander Regueiro (Nov 06 2018 at 17:46, on Zulip):

how is predicates_of different to super_predicates of?

scalexm (Nov 06 2018 at 17:47, on Zulip):

super_predicates_of just includes predicates on Self, e.g. where Self: Trait<Item = Foo>

scalexm (Nov 06 2018 at 17:47, on Zulip):

(which can be also written with the shorter form trait Bar: Trait<Item = Foo> { ... })

Alexander Regueiro (Nov 06 2018 at 17:49, on Zulip):

oh yes, duh

Alexander Regueiro (Nov 06 2018 at 17:58, on Zulip):

gah, compiling without --keep-stage 0 is so slow..

Alexander Regueiro (Nov 06 2018 at 18:10, on Zulip):

@scalexm and is there a fundamental difference between super_predicates_of and elaborate_trait_ref other than the latter is guaranteed to terminate?

Alexander Regueiro (Nov 06 2018 at 18:12, on Zulip):

incidentally, got an error building:

error: internal compiler error: librustc/ty/subst.rs:462: Type parameter `T/#1` (T/1) out of range when substituting (root type=Some(T)) substs=[T]

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:538:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
scalexm (Nov 06 2018 at 18:12, on Zulip):

@Alexander Regueiro the latter calls super_predicates_of recursively

Alexander Regueiro (Nov 06 2018 at 18:13, on Zulip):

ah

scalexm (Nov 06 2018 at 18:13, on Zulip):

Ok I guess trying to substitute with the subst returned by create_substs_for_ast_trait_ref does not work so easily then

scalexm (Nov 06 2018 at 18:13, on Zulip):

@Alexander Regueiro I’m heading to a Rust meetup, I’ll get back to you later on that substitution thing

Alexander Regueiro (Nov 06 2018 at 18:14, on Zulip):

yep... is it necessary to do that substitution even though? @scalexm

Alexander Regueiro (Nov 06 2018 at 18:14, on Zulip):

okay no worries

Alexander Regueiro (Nov 06 2018 at 18:14, on Zulip):

talk later

scalexm (Nov 06 2018 at 18:14, on Zulip):

Could you just check that the panic happens in instantiate_poly_trait_ref_inner?

scalexm (Nov 06 2018 at 18:15, on Zulip):

Or whatever that function is called

Alexander Regueiro (Nov 06 2018 at 18:15, on Zulip):

sure

scalexm (Nov 06 2018 at 18:16, on Zulip):

You could try without substituting, I thought it was necessary but maybe I overlooked something

Alexander Regueiro (Nov 06 2018 at 18:23, on Zulip):

@scalexm I'm trying to think why it would be... hmm.

Alexander Regueiro (Nov 06 2018 at 18:23, on Zulip):

@scalexm it doesn't ICE now:

error[E0719]: the value of the associated type `Output` (from the trait `core::ops::Add`) is already specified

error[E0719]: the value of the associated type `Output` (from the trait `core::ops::Sub`) is already specified

error[E0719]: the value of the associated type `Output` (from the trait `core::ops::Div`) is already specified

error[E0719]: the value of the associated type `Output` (from the trait `core::ops::Shl`) is already specified
Alexander Regueiro (Nov 06 2018 at 18:24, on Zulip):

that's what I was talking about before I think

Alexander Regueiro (Nov 06 2018 at 18:24, on Zulip):

doesn't distinguish between e.g. Iterator<Item = i32> + Iterator<Item = u32> and Iterator<Item = i32, Item = u32>

Alexander Regueiro (Nov 06 2018 at 18:24, on Zulip):

@scalexm

scalexm (Nov 06 2018 at 18:26, on Zulip):

@Alexander Regueiro and is this a problem? It seems to me that both forms should lead to an error

Alexander Regueiro (Nov 06 2018 at 18:29, on Zulip):

@scalexm okay true. but what about e.g. impl<'a, 'b> Add<&'a u32> for &'b u32and impl<'a> Add<u32> for &'a u32 together?

Alexander Regueiro (Nov 06 2018 at 18:29, on Zulip):

they both define Output associated types

Alexander Regueiro (Nov 06 2018 at 18:30, on Zulip):

with just different type parameters

Alexander Regueiro (Nov 06 2018 at 18:30, on Zulip):

that's the key I guess

scalexm (Nov 06 2018 at 18:36, on Zulip):

@Alexander Regueiro right checking duplicates basing only on projection_def_id is probably too restrictive

Alexander Regueiro (Nov 06 2018 at 18:37, on Zulip):

indeed

Alexander Regueiro (Nov 06 2018 at 18:42, on Zulip):

tcx.associated_item(proj.projection_type.item_def_id).container

Alexander Regueiro (Nov 06 2018 at 18:42, on Zulip):

I think that's the DefId of the TraitRef containing the projection

Alexander Regueiro (Nov 06 2018 at 18:42, on Zulip):

maybe we want that?

Alexander Regueiro (Nov 06 2018 at 19:11, on Zulip):

@scalexm anyway I pushed a new attempt

Alexander Regueiro (Nov 06 2018 at 19:24, on Zulip):

got a strange failure when trying to build locally though

Alexander Regueiro (Nov 06 2018 at 19:27, on Zulip):

maybe because I had incremental on though

Alexander Regueiro (Nov 06 2018 at 19:31, on Zulip):
error: Could not compile `std`.

To learn more, run the command again with --verbose.
command did not execute successfully: "/Users/alex/Software/rust-devel/build/x86_64-apple-darwin/stage0/bin/cargo" "build" "--target" "x86_64-apple-darwin" "-j" "8" "--release" "--features" "panic-unwind backtrace" "--manifest-path" "/Users/alex/Software/rust-devel/src/libstd/Cargo.toml" "--message-format" "json"
expected success, got: exit code: 101
thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1101:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: <std::panicking::begin_panic::PanicPayload<A> as core::panic::BoxMeUp>::get
             at libstd/panicking.rs:476
   5: <core::str::CharIndices<'a> as core::iter::iterator::Iterator>::next
             at libstd/panicking.rs:410
   6: bootstrap::compile::run_cargo
             at bootstrap/compile.rs:1101
   7: std::thread::panicking
             at bootstrap/compile.rs:115
   8: bootstrap::builder::Builder::cargo::{{closure}}
             at bootstrap/builder.rs:1215
   9: bootstrap::compile::copy_apple_sanitizer_dylibs
             at bootstrap/compile.rs:342
  10: bootstrap::builder::Builder::cargo::{{closure}}
             at bootstrap/builder.rs:1215
  11: bootstrap::compile::test_cargo
             at bootstrap/compile.rs:464
  12: bootstrap::builder::Builder::cargo::{{closure}}
             at bootstrap/builder.rs:1215
  13: bootstrap::compile::compiler_file
             at bootstrap/compile.rs:959
  14: bootstrap::builder::Builder::cargo::{{closure}}
             at bootstrap/builder.rs:1215
  15: bootstrap::builder::Builder::run_step_descriptions
             at bootstrap/builder.rs:579
  16: bootstrap::compile::compiler_file
             at bootstrap/compile.rs:952
  17: bootstrap::builder::Builder::cargo::{{closure}}
             at bootstrap/builder.rs:1215
  18: bootstrap::builder::Builder::run_step_descriptions
             at bootstrap/builder.rs:579
  19: std::thread::panicking
             at bootstrap/compile.rs:55
  20: bootstrap::builder::StepDescription::maybe_run
             at bootstrap/builder.rs:191
  21: bootstrap::builder::StepDescription::run
             at bootstrap/builder.rs:234
  22: bootstrap::builder::Builder::run_step_descriptions
             at bootstrap/builder.rs:571
  23: bootstrap::builder::Builder::get_step_descriptions
             at bootstrap/builder.rs:561
  24: bootstrap::Crate::local_path
             at bootstrap/lib.rs:479
  25: bootstrap::main
             at bootstrap/bin/main.rs:29
  26: std::rt::lang_start::{{closure}}
             at libstd/rt.rs:74
  27: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  28: panic_unwind::dwarf::eh::read_encoded_pointer
             at libpanic_unwind/lib.rs:102
  29: std::alloc::default_alloc_error_hook
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  30: std::rt::lang_start
             at libstd/rt.rs:74
  31: bootstrap::main
failed to run: /Users/alex/Software/rust-devel/build/bootstrap/debug/bootstrap build libstd
Build completed unsuccessfully in 0:02:42
Alexander Regueiro (Nov 06 2018 at 19:39, on Zulip):

@scalexm was hoping this would work, but it seems not... https://github.com/rust-lang/rust/pull/55687/files#diff-1b4d50b82a87f1031cd5f99b64123de7R787

scalexm (Nov 06 2018 at 20:56, on Zulip):

@Alexander Regueiro ok so the cool thing is that if I comment out the check-for-duplicates code, rustc compiles and your run-pass tests as well

scalexm (Nov 06 2018 at 20:56, on Zulip):

so I see that there was some existing code which outputted warnings, that you removed

scalexm (Nov 06 2018 at 20:58, on Zulip):

any reason why you did not just use that existing code?

Alexander Regueiro (Nov 06 2018 at 22:03, on Zulip):

@scalexm it wasn’t great that existing code.

Alexander Regueiro (Nov 06 2018 at 22:04, on Zulip):

It needs to be adapted for supertraits.

Alexander Regueiro (Nov 06 2018 at 22:04, on Zulip):

And made a hard error. At the very least

scalexm (Nov 06 2018 at 22:04, on Zulip):

@Alexander Regueiro ok well I guess this is just a matter of getting it right now

Alexander Regueiro (Nov 06 2018 at 22:05, on Zulip):

Yep...

Alexander Regueiro (Nov 06 2018 at 22:05, on Zulip):

Will be home to test shortly

Alexander Regueiro (Nov 06 2018 at 22:05, on Zulip):

Still on the go right now

scalexm (Nov 06 2018 at 22:05, on Zulip):

if there weren't any binders you could just use the trait_ref associated to the projection predicate as a unique key

scalexm (Nov 06 2018 at 22:05, on Zulip):

but this may not work for higher-ranked projection predicates

Alexander Regueiro (Nov 06 2018 at 22:10, on Zulip):

True hmm

scalexm (Nov 06 2018 at 22:30, on Zulip):

so @Alexander Regueiro I was thinking

scalexm (Nov 06 2018 at 22:31, on Zulip):

the former lint was designed to handle cases like where T: Iterator<Item = (), Item = ()>, i.e. it does not trigger if you write where T: Iterator<Item = ()>, T: Iterator<Item = ()>

scalexm (Nov 06 2018 at 22:31, on Zulip):

which... seems ok because the latter is really harder to write "unintentionally"

scalexm (Nov 06 2018 at 22:32, on Zulip):

and if you ever write where T: Iterator<Item = u32>, T: Iterator<Item = i32>, you'll get an inference error in the end

Alexander Regueiro (Nov 06 2018 at 22:32, on Zulip):

@scalexm correct

scalexm (Nov 06 2018 at 22:34, on Zulip):

so I'd say: just use the former strategy of just checking T: Iterator<Item = (), Item = ()>: this should be needed only on the direct projection predicates, not the elaborated ones because they where already checked in the corresponding trait definitions

scalexm (Nov 06 2018 at 22:34, on Zulip):

so basically having your check in ast_type_binding_to_poly_projection_predicate is correct I think

scalexm (Nov 06 2018 at 22:34, on Zulip):

then since ast_type_binding_to_poly_projection_predicate talks about one specific projection predicate, you can just use the assoc item def-id as a unique key, as the former lint code

scalexm (Nov 06 2018 at 22:35, on Zulip):

(which of course you now turn into an error)

scalexm (Nov 06 2018 at 22:35, on Zulip):

then it's true that the type inference error you get when you write T: Iterator<Item = i32>, T: Iterator<Item = u32> isn't great, but IMO it should be a separate issue

scalexm (Nov 06 2018 at 22:36, on Zulip):

and really it seems like a difficult one because of higher-ranked projection predicates, you would have to check if one is "more specific" than the other or something like that

Alexander Regueiro (Nov 06 2018 at 22:40, on Zulip):

hmm

Alexander Regueiro (Nov 06 2018 at 22:41, on Zulip):

you may be right yes

scalexm (Nov 06 2018 at 22:43, on Zulip):

we can still ask @nikomatsakis what they think, but landing your PR as I described might be a good first step I think

scalexm (Nov 06 2018 at 22:45, on Zulip):

as an aside, now that we use elaborate_trait_ref, maybe add a test for "transitive" projection predicates, i.e. something like:

trait Foo: Iterator<Item = ()> { }
trait Bar: Foo { }

fn foo(x: dyn Bar) { }
Alexander Regueiro (Nov 06 2018 at 22:46, on Zulip):

I agree

Alexander Regueiro (Nov 06 2018 at 22:47, on Zulip):

I wish my builds would work locally though

Alexander Regueiro (Nov 06 2018 at 22:47, on Zulip):

hmm

Alexander Regueiro (Nov 06 2018 at 22:48, on Zulip):

@scalexm should poly_projections.extend for the supertraits still go in instantiate_poly_trait_ref_inner thoguh?

scalexm (Nov 06 2018 at 22:48, on Zulip):

@Alexander Regueiro yes I think so

scalexm (Nov 06 2018 at 22:48, on Zulip):

also you might want to tweak a bit elaborate_trait_ref as we discussed, so that it gives you spans

Alexander Regueiro (Nov 06 2018 at 22:49, on Zulip):

yeah

Alexander Regueiro (Nov 06 2018 at 22:49, on Zulip):

@scalexm ah wait, it can't... since poly_projections.extend isn't called until after ast_type_binding_to_poly_projection_predicate

Alexander Regueiro (Nov 06 2018 at 22:49, on Zulip):

hmm

scalexm (Nov 06 2018 at 22:50, on Zulip):

I don't think this is a problem?

Alexander Regueiro (Nov 06 2018 at 22:50, on Zulip):

/me wonders what speculative means

Alexander Regueiro (Nov 06 2018 at 22:51, on Zulip):

@scalexm it is a problem, because otherwise it doesn't see the poly_projections added for the supertraits

scalexm (Nov 06 2018 at 22:51, on Zulip):

you mean the check for duplicates?

Alexander Regueiro (Nov 06 2018 at 22:52, on Zulip):

huh?

scalexm (Nov 06 2018 at 22:52, on Zulip):

sorry, when you say "it doesn't see"

scalexm (Nov 06 2018 at 22:53, on Zulip):

what is "it"?

Alexander Regueiro (Nov 06 2018 at 22:53, on Zulip):

ast_type_binding_to_poly_projection_predicate

scalexm (Nov 06 2018 at 22:54, on Zulip):

so in ast_type_binding_to_poly_projection_predicate we would just include the check for duplicates

scalexm (Nov 06 2018 at 22:54, on Zulip):

and I think we don't need to check duplicates for projection predicates coming from super traits

scalexm (Nov 06 2018 at 22:55, on Zulip):

because, as they come for supertraits, they have been checked at their definition site already, i.e.:

trait Bar: Iterator<Item = (), Item = ()> {} // error here

fn foo<T>() where T: Bar // no need to check again, even if `Iterator<Item = ()>, Iterator<Item = ()>` is indeed elaborated
Alexander Regueiro (Nov 06 2018 at 22:57, on Zulip):

@scalexm you're missing the all-important case of

trait Foo = Iterator<Item = ()>;
trait Bar = Foo<Item = ()>;
scalexm (Nov 06 2018 at 22:57, on Zulip):

@Alexander Regueiro I deliberately missed it

scalexm (Nov 06 2018 at 22:58, on Zulip):

I think that this should not considered a problem to repeat this bound

Alexander Regueiro (Nov 06 2018 at 22:58, on Zulip):

it definitely should

Alexander Regueiro (Nov 06 2018 at 22:58, on Zulip):

but you want to leave it for another PR?

scalexm (Nov 06 2018 at 23:00, on Zulip):

the problem which may come is if you write:

trait Foo = Iterator<Item = ()>;
trait Bar = Foo<Item = u32>;

which will give a type inference error, which might be hard to understand for a user

scalexm (Nov 06 2018 at 23:00, on Zulip):

but at least it'll error out

scalexm (Nov 06 2018 at 23:00, on Zulip):

well

scalexm (Nov 06 2018 at 23:00, on Zulip):

I don't see how this is different from e.g. writing fn foo<T: Clone, T: Clone>()

scalexm (Nov 06 2018 at 23:00, on Zulip):

which is accepted today (and not even linted)

scalexm (Nov 06 2018 at 23:02, on Zulip):

but if we really want to handle that case, you come back on the problem of the uniqueness of projection predicates, which is difficult I think

Alexander Regueiro (Nov 06 2018 at 23:03, on Zulip):

hmm okay

Alexander Regueiro (Nov 06 2018 at 23:08, on Zulip):

@scalexm you're right, I think that's a very tricky problem. I'll leave it for now

scalexm (Nov 06 2018 at 23:08, on Zulip):

@Alexander Regueiro feel free to open an issue about it maybe

Alexander Regueiro (Nov 06 2018 at 23:08, on Zulip):

yes, could do

Alexander Regueiro (Nov 07 2018 at 00:29, on Zulip):

@scalexm do you have review/r+ privileges? I'll try to tie everything up so you can have a look tomorrow morning if you like.

Alexander Regueiro (Nov 07 2018 at 04:17, on Zulip):

@scalexm I can't replicate the code building

Alexander Regueiro (Nov 07 2018 at 04:18, on Zulip):

@scalexm I just pushed

Alexander Regueiro (Nov 07 2018 at 04:29, on Zulip):

https://gist.github.com/7ac67405c877c8014ad173eee1b61d6e

nikomatsakis (Nov 07 2018 at 09:30, on Zulip):

then it's true that the type inference error you get when you write T: Iterator<Item = i32>, T: Iterator<Item = u32> isn't great, but IMO it should be a separate issue

I agree this is a distinct issue

nikomatsakis (Nov 07 2018 at 09:31, on Zulip):

I just skimmed the backscroll; what @scalexm said makes sense to me. I plan to carve out some time for reviewing today fyi so @Alexander Regueiro if you can package up the PR I can review it, though i'm also happy if @scalexm reviews

scalexm (Nov 07 2018 at 10:42, on Zulip):

@Alexander Regueiro when are you getting that error?

scalexm (Nov 07 2018 at 10:42, on Zulip):

I don’t see it in travis

scalexm (Nov 07 2018 at 12:45, on Zulip):

@Alexander Regueiro ok I now see the failure you described

scalexm (Nov 07 2018 at 12:45, on Zulip):

turns out I was not compiling enough stuff (in particular std was compiling fine)

scalexm (Nov 07 2018 at 14:12, on Zulip):

@Alexander Regueiro ok so, I remember that in one of your first commits, you were hesitating between extending the projection bounds in fn instantiate_poly_trait_ref_inner or in fn conv_object_ty_poly_trait_ref

scalexm (Nov 07 2018 at 14:13, on Zulip):

actually the former get calls for every trait ref, while the latter is only about AST dyn Trait to ty::Dynamic conversion, so this is the good one :)

scalexm (Nov 07 2018 at 14:13, on Zulip):

I got a minimal reproduction for your failure: https://gist.github.com/rust-play/dc09cf51d30d50500826a7eea62bf1f6

scalexm (Nov 07 2018 at 14:14, on Zulip):

since we extended projection bounds in instantiate_poly_trait_ref_inner, even where clauses were extended with these new bounds

scalexm (Nov 07 2018 at 14:14, on Zulip):

most of the time, this was harmless, except in cases like this one

scalexm (Nov 07 2018 at 14:14, on Zulip):

because this clearly generated a recursive impl

scalexm (Nov 07 2018 at 14:15, on Zulip):

so I tried move the code which extends the projection bounds to fn conv_object_ty_poly_trait_ref and now everything works fine

scalexm (Nov 07 2018 at 14:16, on Zulip):

I would even consider merging that code with this loop: https://github.com/rust-lang/rust/pull/55687/files#diff-1b4d50b82a87f1031cd5f99b64123de7L1027

scalexm (Nov 07 2018 at 14:16, on Zulip):

where we would call elaborate_trait_ref instead of supertraits and match on TraitPredicate and ProjectionPredicate

scalexm (Nov 07 2018 at 14:21, on Zulip):

I'll sum up everything directly on your PR

Alexander Regueiro (Nov 07 2018 at 15:24, on Zulip):

@scalexm yep, this is all turning out very similar to what was in my original commit hah.

Alexander Regueiro (Nov 07 2018 at 15:24, on Zulip):

@scalexm I even had it merged with that loop ;-)

scalexm (Nov 07 2018 at 15:25, on Zulip):

@Alexander Regueiro lol ok, then I think we reached to a consensus now

Alexander Regueiro (Nov 07 2018 at 15:25, on Zulip):

yep

Alexander Regueiro (Nov 07 2018 at 15:43, on Zulip):

@nikomatsakis and yeah, I think we all agree now it should be a separate issue... the inference error is a tricky one to solve

Alexander Regueiro (Nov 07 2018 at 15:44, on Zulip):

no straightforward solution I suspect

Alexander Regueiro (Nov 07 2018 at 16:02, on Zulip):

@scalexm should be ready very soon. let's try to get it merged soon after :-)

Alexander Regueiro (Nov 07 2018 at 16:23, on Zulip):

@nikomatsakis any more thoughts on upcasting with trait aliases btw? should we be tackling upcasting in general?

Alexander Regueiro (Nov 07 2018 at 17:19, on Zulip):

@scalexm hrmm, now it doesn't work...

scalexm (Nov 07 2018 at 17:20, on Zulip):

@Alexander Regueiro what isn’t working exactly?

Alexander Regueiro (Nov 07 2018 at 17:20, on Zulip):

I still get:

Alexander Regueiro (Nov 07 2018 at 17:20, on Zulip):
error[E0191]: the value of the associated type `Item` (from the trait `std::iter::Iterator`) must be specified
  --> $DIR/trait-alias-object.rs:18:13
   |
LL |     let _: &dyn IteratorAlias = &vec![123].into_iter();
   |             ^^^^^^^^^^^^^^^^^ missing associated type `Item` value
Alexander Regueiro (Nov 07 2018 at 17:21, on Zulip):

hrmm

Alexander Regueiro (Nov 07 2018 at 17:23, on Zulip):

@scalexm
my code is

        for tr in traits::elaborate_trait_ref(tcx, principal) {
            match tr {
                ty::Predicate::Trait(pred) => {
                    associated_types.extend(tcx.associated_items(pred.def_id())
                                    .filter(|item| item.kind == ty::AssociatedKind::Type)
                                    .map(|item| item.def_id));
                }
                ty::Predicate::Projection(pred) => {
                    // Include projections defined on supertraits.
                    projection_bounds.push((pred, DUMMY_SP))
                }
                _ => ()
            }
        }

in conv_object_ty_poly_trait_ref

Alexander Regueiro (Nov 07 2018 at 17:24, on Zulip):

aha

Alexander Regueiro (Nov 07 2018 at 17:24, on Zulip):

I think I see why

Alexander Regueiro (Nov 07 2018 at 17:24, on Zulip):

no, I don't... :-(

scalexm (Nov 07 2018 at 17:26, on Zulip):

Mmh, weird because I had this test passing

Alexander Regueiro (Nov 07 2018 at 17:26, on Zulip):

@scalexm I just pushed if you want to take a look

scalexm (Nov 07 2018 at 17:26, on Zulip):

Ok

Alexander Regueiro (Nov 07 2018 at 17:37, on Zulip):

@scalexm any ideas?

scalexm (Nov 07 2018 at 17:49, on Zulip):

@Alexander Regueiro so

scalexm (Nov 07 2018 at 17:49, on Zulip):

we are talking about src/test/run-pass/traits/trait-alias-object.rs right?

Alexander Regueiro (Nov 07 2018 at 17:50, on Zulip):

yep @scalexm

scalexm (Nov 07 2018 at 17:52, on Zulip):

well, I just compiled your code, and running rustc +stage1 src/test/run-pass/traits/trait-alias-object.rs just works

Alexander Regueiro (Nov 07 2018 at 17:53, on Zulip):

huh...

Alexander Regueiro (Nov 07 2018 at 17:53, on Zulip):

and stage2?

scalexm (Nov 07 2018 at 17:54, on Zulip):

did not try stage2, I almost never compile up to stage2 since it takes so long :p

scalexm (Nov 07 2018 at 17:54, on Zulip):

running ./x.py test src/test/run-pass --stage 1 --test-args trait-alias-object.rs also passes

scalexm (Nov 07 2018 at 17:55, on Zulip):

I'm going to compile stage 2 just in case

Alexander Regueiro (Nov 07 2018 at 17:55, on Zulip):

@scalexm stage 2 is fairly quick normally, it's stage 0 and 1 that take forever. I normally do --keep-stage 0

scalexm (Nov 07 2018 at 17:55, on Zulip):

ah yeah you're probably right

Alexander Regueiro (Nov 07 2018 at 18:06, on Zulip):

@scalexm stage1 doesn't work for me either though, funnily enough

Alexander Regueiro (Nov 07 2018 at 18:06, on Zulip):

I just rebased over latest master and recompiled, but no diff

scalexm (Nov 07 2018 at 18:09, on Zulip):

(yeah ok it's stage 1 compiler artifacts and codegen artifacts that take very long when you run ./x.py build --stage 2)

Alexander Regueiro (Nov 07 2018 at 18:10, on Zulip):

indeed

Alexander Regueiro (Nov 07 2018 at 18:10, on Zulip):

-i --keep-stage 0 helps hugely heh

scalexm (Nov 07 2018 at 18:11, on Zulip):

did you run you recompilation with --keep-stage 0? Maybe that could be a mis-compilation issue due to this flag?

Alexander Regueiro (Nov 07 2018 at 18:13, on Zulip):

it's possible

Alexander Regueiro (Nov 07 2018 at 18:13, on Zulip):

but I doubt it

Alexander Regueiro (Nov 07 2018 at 18:13, on Zulip):

@scalexm let's see what you get, since you're compiling now

scalexm (Nov 07 2018 at 18:15, on Zulip):

./x.py test src/test/run-pass --stage 2 --test-args trait-alias-object.rs

scalexm (Nov 07 2018 at 18:15, on Zulip):
Check compiletest suite=run-pass mode=run-pass (x86_64-apple-darwin -> x86_64-apple-darwin)

running 1 test
.
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 2881 filtered out
Alexander Regueiro (Nov 07 2018 at 18:15, on Zulip):

okay wow

scalexm (Nov 07 2018 at 18:16, on Zulip):

I definitely think --keep-stage 0 plays in here

scalexm (Nov 07 2018 at 18:16, on Zulip):

well it's the only thing I can think of

Alexander Regueiro (Nov 07 2018 at 18:16, on Zulip):

mhm

Alexander Regueiro (Nov 07 2018 at 18:16, on Zulip):

usually not a problem though

Alexander Regueiro (Nov 07 2018 at 18:16, on Zulip):

weird

scalexm (Nov 07 2018 at 18:17, on Zulip):

maybe that's something else but then I have no idea :p

scalexm (Nov 07 2018 at 18:17, on Zulip):

at worst what you could try is, get all the tests working "theoretically", upload the PR and see what travis says

Alexander Regueiro (Nov 07 2018 at 18:20, on Zulip):

@scalexm that's precisely what I'm doing, concurrently to rebuilding without --keep-stage

Alexander Regueiro (Nov 07 2018 at 18:26, on Zulip):

@scalexm what are you working on these days by the way? implied bounds mainly?

scalexm (Nov 07 2018 at 18:26, on Zulip):

mostly chalk integration

scalexm (Nov 07 2018 at 18:26, on Zulip):

I think we are very near to a prototype implementation in rustc

Alexander Regueiro (Nov 07 2018 at 18:27, on Zulip):

oh nice

Alexander Regueiro (Nov 07 2018 at 18:28, on Zulip):

@scalexm so what will happen... rustc will be forked, and you'll try to integrate it... then there will be a longish period of testing, before finally a PR will be made to master?

Alexander Regueiro (Nov 07 2018 at 18:28, on Zulip):

or will this first integration not even look like the final integration?

scalexm (Nov 07 2018 at 18:29, on Zulip):

what we'll do rather is to add a compilation flag to switch between the current trait solver and the new one

scalexm (Nov 07 2018 at 18:29, on Zulip):

we thought about working on a fork to reduce bors waiting times, but since we are making other useful changes to the compiler for the integration with chalk to work, it's better to work on master directly

Alexander Regueiro (Nov 07 2018 at 19:02, on Zulip):

oh okay. fair enough. so when may we see that flag in master... a few weeks? :-)

scalexm (Nov 07 2018 at 19:04, on Zulip):

@Alexander Regueiro yeah I hope so

Alexander Regueiro (Nov 07 2018 at 19:04, on Zulip):

cool

Alexander Regueiro (Nov 07 2018 at 19:17, on Zulip):

@scalexm @nikomatsakis crater run completed. 3 regressions.

Alexander Regueiro (Nov 07 2018 at 19:20, on Zulip):

@scalexm I built afresh and still get the error

scalexm (Nov 07 2018 at 19:53, on Zulip):

@Alexander Regueiro no idea why then, but I’m pretty sure it works

scalexm (Nov 07 2018 at 19:53, on Zulip):

Update the ui tests so that Travis doesn’t fail on them

Alexander Regueiro (Nov 07 2018 at 21:01, on Zulip):

@scalexm need a working build before I start messing around...

scalexm (Nov 07 2018 at 21:04, on Zulip):

@Alexander Regueiro just remove src/test/ui/error-codes/E0719-trait-alias.rs, src/test/ui/error-codes/E0719-trait-alias-object.rs and src/test/ui/traits/trait-alias-object.rs temporarily

scalexm (Nov 07 2018 at 21:04, on Zulip):

if you havent

Alexander Regueiro (Nov 07 2018 at 21:05, on Zulip):

no, I need to creat new tests

scalexm (Nov 07 2018 at 21:05, on Zulip):

I mean, we just want to see if travis gets the same result than me

scalexm (Nov 07 2018 at 21:05, on Zulip):

because what's happening is definitely weird :p

scalexm (Nov 07 2018 at 21:06, on Zulip):

so if you haven't touched any code since then, and that you remove these tests, we may be able to see if travis agrees with me

Alexander Regueiro (Nov 07 2018 at 21:12, on Zulip):

@scalexm works now :-)

Alexander Regueiro (Nov 07 2018 at 21:12, on Zulip):

I had to completely wipe build/

scalexm (Nov 07 2018 at 21:12, on Zulip):

@Alexander Regueiro ah very good!

Alexander Regueiro (Nov 07 2018 at 21:21, on Zulip):

yep

Alexander Regueiro (Nov 07 2018 at 21:45, on Zulip):

@scalexm done! ready for final review and r+ :-)

scalexm (Nov 07 2018 at 21:54, on Zulip):

ok seems good

scalexm (Nov 07 2018 at 21:54, on Zulip):

just waiting for travis results before r+-ing

Alexander Regueiro (Nov 07 2018 at 21:57, on Zulip):

no problem

scalexm (Nov 07 2018 at 21:59, on Zulip):

btw the three crater regressions seem spurious

scalexm (Nov 07 2018 at 21:59, on Zulip):

two of them showed up already in a previous crater run, and I also checked the third one

Alexander Regueiro (Nov 07 2018 at 22:00, on Zulip):

@scalexm oh okay cool. the one "fix" was probably spurious too heh

Alexander Regueiro (Nov 07 2018 at 22:01, on Zulip):

onto https://github.com/rust-lang/rust/issues/54600#issuecomment-433049875 now! @nikomatsakis @scalexm

scalexm (Nov 07 2018 at 22:25, on Zulip):

@Alexander Regueiro yes and anyway no chance your PR hits bors before travis finishes so it won't use bors' time needlessly in case of a test fail, so ok I r+'d

Alexander Regueiro (Nov 13 2018 at 18:01, on Zulip):

@scalexm can there be lifetime binders in trait objects?

scalexm (Nov 13 2018 at 18:04, on Zulip):

@Alexander Regueiro you mean like in fn foo(x: &dyn for<'a> Fn(&'a i32))? If so, yes

Alexander Regueiro (Nov 13 2018 at 18:04, on Zulip):

okay, thanks

Alexander Regueiro (Nov 13 2018 at 18:04, on Zulip):

@scalexm I don't think that affects the rules about a single regular trait + arbitrary numebr of auto-traits though, right?

Alexander Regueiro (Nov 13 2018 at 18:06, on Zulip):

btw, turns out we need to make the extension to elaborate_trait_ref to return spans now :-P

scalexm (Nov 13 2018 at 18:06, on Zulip):

@Alexander Regueiro yeah I don't think neither

Alexander Regueiro (Nov 13 2018 at 18:17, on Zulip):

@scalexm ugh, there's an issue: when you call elaborate_trait_refs, and it ends up looking at a Predicate that's not a trait, there's seemingly no way to get the span...

Alexander Regueiro (Nov 13 2018 at 18:18, on Zulip):

is there a way to work around this, or should we just return a DUMMY_SPAN?

Alexander Regueiro (Nov 13 2018 at 18:19, on Zulip):

I think we're only interested in the spans for traits here, luckily...

scalexm (Nov 13 2018 at 18:43, on Zulip):

@Alexander Regueiro doesn't super_predicates_of return span information?

scalexm (Nov 13 2018 at 18:43, on Zulip):

ah you mean

scalexm (Nov 13 2018 at 18:43, on Zulip):

ok let me see

scalexm (Nov 13 2018 at 18:45, on Zulip):

so @Alexander Regueiro you are talking about the outlives components right?

Alexander Regueiro (Nov 13 2018 at 18:46, on Zulip):

@scalexm right

Alexander Regueiro (Nov 13 2018 at 18:46, on Zulip):

I'm putting DUMMY_SP in there for now and seeing how it works :-P

scalexm (Nov 13 2018 at 18:47, on Zulip):

I would put the same span as the TypeOutlives predicate you just got

scalexm (Nov 13 2018 at 18:48, on Zulip):

and probably you could initially feed elaborate_trait_ref with a span

scalexm (Nov 13 2018 at 18:48, on Zulip):

(which will end up being DUMMY_SP in most calls except yours I guess)

scalexm (Nov 13 2018 at 18:48, on Zulip):

actually how were you planning to forward than span info to begin with?

Alexander Regueiro (Nov 13 2018 at 18:50, on Zulip):

@scalexm ah true

Alexander Regueiro (Nov 13 2018 at 18:50, on Zulip):

let's do that

Alexander Regueiro (Nov 13 2018 at 19:43, on Zulip):

@scalexm a lot of refactoring needed in the end, even though it's quite straightforward... going out now, but I'll ping you when I get back, and maybe you can kindly review and r+ my PR. :-)

Alexander Regueiro (Nov 13 2018 at 23:28, on Zulip):

@scalexm hi

scalexm (Nov 14 2018 at 09:34, on Zulip):

Hi @Alexander Regueiro, anything needed? :)

Alexander Regueiro (Nov 14 2018 at 22:48, on Zulip):

@scalexm sorry for the delay. couldn't manage to finish it last night. and busy most of today. just wrapping things up now (I think)

Alexander Regueiro (Nov 17 2018 at 19:18, on Zulip):

@nikomatsakis @scalexm https://github.com/rust-lang/rust/pull/55994 is ready for final review and r+ I think.

nikomatsakis (Nov 19 2018 at 19:53, on Zulip):

awesome!

nikomatsakis (Nov 19 2018 at 19:53, on Zulip):

I skimmed it, will read in more depth soon

Alexander Regueiro (Nov 19 2018 at 20:07, on Zulip):

@nikomatsakis great, thanks.

Alexander Regueiro (Nov 20 2018 at 19:08, on Zulip):

@nikomatsakis thanks for the review. fixed the nit, should be good to go now.

nikomatsakis (Nov 20 2018 at 19:09, on Zulip):

r+

Alexander Regueiro (Nov 26 2018 at 17:14, on Zulip):

@nikomatsakis any idea about https://github.com/rust-lang/rust/pull/55994#issuecomment-441536830?

scalexm (Nov 26 2018 at 17:20, on Zulip):

@Alexander Regueiro could you maybe try to minimize the failing example?

Alexander Regueiro (Nov 26 2018 at 17:20, on Zulip):

I'll try yeah. Heading out now, but will be back later

Alexander Regueiro (Nov 26 2018 at 17:20, on Zulip):

not even sure the exact source it fails on mind you

scalexm (Nov 26 2018 at 17:22, on Zulip):

@Alexander Regueiro I'll try to take a look, I have some time available

scalexm (Nov 26 2018 at 18:16, on Zulip):

actually @Alexander Regueiro I think you fixed a bug

scalexm (Nov 26 2018 at 18:16, on Zulip):

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=7567fa0392b7a68425ba7d0f2822c0e5

scalexm (Nov 26 2018 at 18:17, on Zulip):

but it seems like the AppVeyor test suite is also testing some popular crates (e.g. servo, ripgrep)

scalexm (Nov 26 2018 at 18:18, on Zulip):

one of them is depending on the traitobject crate, which I think is flawed: https://github.com/reem/rust-traitobject/blob/master/src/impls.rs

scalexm (Nov 26 2018 at 18:18, on Zulip):
unsafe impl Trait for ::std::marker::Send + Sync { }
unsafe impl Trait for ::std::marker::Send + Send + Sync { }
scalexm (Nov 26 2018 at 18:23, on Zulip):

@Alexander Regueiro basically the dedup in the former code was wrong because it was only deduping trait_bounds[1..]

Alexander Regueiro (Nov 26 2018 at 18:41, on Zulip):

@scalexm Aha, good spot. I think you’re right.

Alexander Regueiro (Nov 26 2018 at 18:41, on Zulip):

I wonder what we should do here.

Alexander Regueiro (Nov 26 2018 at 18:41, on Zulip):

@nikomatsakis thoughts?

nikomatsakis (Nov 26 2018 at 18:48, on Zulip):

what is the problem exactly?

Alexander Regueiro (Nov 26 2018 at 18:53, on Zulip):

@scalexm I wonder why that crate even does that. Any idea? I’d be tempted to break it.

scalexm (Nov 26 2018 at 18:53, on Zulip):

@Alexander Regueiro it may have been generated by a script?

scalexm (Nov 26 2018 at 18:54, on Zulip):

that crate defines a trait Trait and implements it for all trait objects possibly coming from libstd

Alexander Regueiro (Nov 26 2018 at 18:54, on Zulip):

@scalexm Good point. Maybe I should open an issue there.

scalexm (Nov 26 2018 at 18:54, on Zulip):

probably so that when you bound with where T: Trait, you know you are receiving a trait object and can use this invariant in some unsafe code

scalexm (Nov 26 2018 at 18:55, on Zulip):

(and that would explain why said Trait is marked as unsafe)

scalexm (Nov 26 2018 at 18:55, on Zulip):

breaking it now is maybe not a good idea because some popular crate depends on it

scalexm (Nov 26 2018 at 18:55, on Zulip):

but opening a PR might be a good option

scalexm (Nov 26 2018 at 18:55, on Zulip):

(rather than an issue, seems like the fix is quite simple I believe)

scalexm (Nov 26 2018 at 18:55, on Zulip):

then wait for the upstream crates to update their dependencies (opening PRs as well might help)

nikomatsakis (Nov 26 2018 at 19:02, on Zulip):

I'm still trying to understand why it must be broken

nikomatsakis (Nov 26 2018 at 19:02, on Zulip):

because it has dyn Send + Send?

nikomatsakis (Nov 26 2018 at 19:02, on Zulip):

oh

nikomatsakis (Nov 26 2018 at 19:03, on Zulip):

because of coherence

nikomatsakis (Nov 26 2018 at 19:03, on Zulip):

@Alexander Regueiro what we can do perhaps is to do a future-compatibility warning period

nikomatsakis (Nov 26 2018 at 19:04, on Zulip):

e.g., we could detect the conflict, but permit it for traits with no items implemented on dyn Send

nikomatsakis (Nov 26 2018 at 19:04, on Zulip):

but issue a future-compatibility warning

nikomatsakis (Nov 26 2018 at 19:04, on Zulip):

as described here

nikomatsakis (Nov 26 2018 at 19:04, on Zulip):

we may have to do this for a time at least

Alexander Regueiro (Nov 26 2018 at 19:09, on Zulip):

@nikomatsakis Yeah sounds reasonable. I’ll try to implement that tonight when I get back.

Alexander Regueiro (Nov 26 2018 at 19:11, on Zulip):

@nikomatsakis Anyway, not much to update you on, except that clarification on where to instantiate opaque types (regular or nll type checking phase) would be great. Tagged you in an issue about impl Trait lifetimes that I’d personally like to see addressed soon, but yeah... did you see @eddyb’s Discord messages about invariant lifetimes in RPIT btw?

nikomatsakis (Nov 26 2018 at 19:11, on Zulip):

"RPIT"?

nikomatsakis (Nov 26 2018 at 19:12, on Zulip):

not sure if I saw those messages

nikomatsakis (Nov 26 2018 at 19:12, on Zulip):

I'm still catching up on discord/zulip

Alexander Regueiro (Nov 26 2018 at 19:13, on Zulip):

RPIT = return-position impl Trait. Sorry, something @varkor coined a while ago, wasn’t sure if it had fully caught on yet.

nikomatsakis (Nov 26 2018 at 19:20, on Zulip):

@Alexander Regueiro I went scanning back my discord mentions but didn't see the comments you are referring to

nikomatsakis (Nov 26 2018 at 19:20, on Zulip):

maybe send me a link

Alexander Regueiro (Nov 26 2018 at 19:36, on Zulip):

@nikomatsakis I’m just on my phone sorry. Ask @eddyb if you don’t mind? I was pretty sure he tagged you though hmm.

nikomatsakis (Nov 26 2018 at 19:41, on Zulip):

he probably did, it's just that I have a lot of tags...

nikomatsakis (Nov 26 2018 at 19:41, on Zulip):

I'll ping him in a bit, I have a bunch of msgs from him to catch up on anyway :)

Alexander Regueiro (Nov 27 2018 at 16:41, on Zulip):

@nikomatsakis hey, you around by chance?

nikomatsakis (Nov 27 2018 at 16:55, on Zulip):

not really, feeling quite sick today, heading back to bed :(

Alexander Regueiro (Nov 27 2018 at 17:02, on Zulip):

@nikomatsakis sorry to hear. rest up and get well!

Alexander Regueiro (Nov 29 2018 at 19:17, on Zulip):

@nikomatsakis hey. feeling any better today?

nikomatsakis (Nov 29 2018 at 21:21, on Zulip):

yep :) now just scrambling to catch up

Alexander Regueiro (Nov 29 2018 at 21:26, on Zulip):

:-)

nikomatsakis (Nov 30 2018 at 21:00, on Zulip):

@Alexander Regueiro what's up with https://github.com/rust-lang/rust/pull/55994 btw?

Alexander Regueiro (Nov 30 2018 at 21:37, on Zulip):

Sorry, going to get to that shortly.

Alexander Regueiro (Nov 30 2018 at 22:35, on Zulip):

did you ever catch up with Eddy about the invariant lifetimes in RPIT issue btw? :-)

Alexander Regueiro (Dec 03 2018 at 03:36, on Zulip):

@nikomatsakis ^ maybe this week? :-)

Alexander Regueiro (Dec 03 2018 at 05:09, on Zulip):

@nikomatsakis Btw, I think this is going to be extremely difficult to lint for. Because the bug was in astconv, but detection can only be done during coherence checking.

Alexander Regueiro (Dec 03 2018 at 05:13, on Zulip):

I wonder if we can move straight to a bug fix (hard error), and just alert the problematic crates of the issue.

Alexander Regueiro (Dec 03 2018 at 18:30, on Zulip):

@nikomatsakis was talking about this with @Ariel Ben-Yehuda and @centril and we were thinking the best/easiest thing to do might actually be to lint duplicate auto traits in general, which subsumes the above case. And @Ariel Ben-Yehuda tells me will have a go at linting on multiple impls where the auto traits are just rearranged. Sound reasonable? We can potentially turn both into hard errors in the future.

Alexander Regueiro (Dec 06 2018 at 18:04, on Zulip):

@nikomatsakis Not sure if you saw my above message... probably doesn't need your immediate attention now though. Anyway, about the is_trait_alias fn... is the right approach to make that cross-crate... somehow?

nikomatsakis (Dec 07 2018 at 10:20, on Zulip):

@Alexander Regueiro yep I think so

nikomatsakis (Dec 07 2018 at 10:21, on Zulip):

I imagine one way to do it would be to modify the DefPath

nikomatsakis (Dec 07 2018 at 10:21, on Zulip):

so that we identify TraitAliases

nikomatsakis (Dec 07 2018 at 10:21, on Zulip):

another way to do it would be to convert is_trait_alias into a query

nikomatsakis (Dec 07 2018 at 10:21, on Zulip):

and implement it differently cross-crate

nikomatsakis (Dec 07 2018 at 10:21, on Zulip):

but I .. feel like this is the kind of thing that overloading DefPath works well for

nikomatsakis (Dec 07 2018 at 10:21, on Zulip):

and implement it differently cross-crate

e.g. by consulting metadata

nikomatsakis (Dec 07 2018 at 10:22, on Zulip):

And @Ariel Ben-Yehuda tells me will have a go at linting on multiple impls where the auto traits are just rearranged. Sound reasonable? We can potentially turn both into hard errors in the future.

you mean that @Ariel Ben-Yehuda will take a stab at that?

Alexander Regueiro (Dec 07 2018 at 17:03, on Zulip):

Yep, he's always submitted a PR, and the crater run is finished. It's just waiting on approval.

Alexander Regueiro (Dec 07 2018 at 17:03, on Zulip):

I've added a lint for duplicate auto traits, which is working

Alexander Regueiro (Dec 07 2018 at 18:34, on Zulip):

@nikomatsakis did you get a chance to chat with @eddyb yet about the invariant lifetimes in RPIT issue? :-)

Alexander Regueiro (Dec 07 2018 at 18:34, on Zulip):

(or the other lifetimes issue I tagged you in a couple of weeks ago)

Alexander Regueiro (Dec 08 2018 at 03:28, on Zulip):

oh and if you could review https://github.com/rust-lang/rust/pull/55994 on Monday, I'd be much obliged!

Alexander Regueiro (Dec 12 2018 at 05:41, on Zulip):

Any idea how I should modify build_reduced_graph_for_external_crate_def to handle Def::TraitAlias now?

nikomatsakis (Dec 13 2018 at 17:36, on Zulip):

@Alexander Regueiro :wave:

Alexander Regueiro (Dec 13 2018 at 17:48, on Zulip):

hi

Alexander Regueiro (Dec 13 2018 at 17:48, on Zulip):

I only have about 15 mins now

Alexander Regueiro (Dec 13 2018 at 17:48, on Zulip):

but let's try to fit in what we can...

Alexander Regueiro (Dec 13 2018 at 17:48, on Zulip):

@nikomatsakis

Alexander Regueiro (Dec 13 2018 at 17:51, on Zulip):

?

nikomatsakis (Dec 13 2018 at 17:51, on Zulip):

ok =)

nikomatsakis (Dec 13 2018 at 17:51, on Zulip):

sorry for the bad timing then

nikomatsakis (Dec 13 2018 at 17:51, on Zulip):

we could reschedule for tomorrow

nikomatsakis (Dec 13 2018 at 17:52, on Zulip):

(same time)

nikomatsakis (Dec 13 2018 at 17:52, on Zulip):

or maybe a bit earlier

Alexander Regueiro (Dec 13 2018 at 17:53, on Zulip):

can't do tomorrow.

nikomatsakis (Dec 13 2018 at 17:53, on Zulip):

(next week?)

Alexander Regueiro (Dec 13 2018 at 17:53, on Zulip):

I could do 7:00 tonight maybe

Alexander Regueiro (Dec 13 2018 at 17:53, on Zulip):

a little over an hour

nikomatsakis (Dec 13 2018 at 17:53, on Zulip):

an hour from now?

Alexander Regueiro (Dec 13 2018 at 17:54, on Zulip):

yeah

Alexander Regueiro (Dec 13 2018 at 17:54, on Zulip):

if you can...

nikomatsakis (Dec 13 2018 at 17:54, on Zulip):

I guess I can do that, sure

Alexander Regueiro (Dec 13 2018 at 17:54, on Zulip):

apart from that... maybe next Tues. not sure.

Alexander Regueiro (Dec 13 2018 at 17:54, on Zulip):

okay thanks

Alexander Regueiro (Dec 13 2018 at 17:55, on Zulip):

if you have any spare moment in the meanwhile, a final review of my https://github.com/rust-lang/rust/pull/55994 PR would be great :-)

Alexander Regueiro (Dec 13 2018 at 17:55, on Zulip):

(you already reviewed some of it before.)

nikomatsakis (Dec 13 2018 at 19:16, on Zulip):

@Alexander Regueiro ?

Alexander Regueiro (Dec 13 2018 at 19:21, on Zulip):

hi @nikomatsakis

Alexander Regueiro (Dec 13 2018 at 19:21, on Zulip):

apologies

Alexander Regueiro (Dec 13 2018 at 19:22, on Zulip):

my meeting from 6:00 to 7:00 overran a bit

nikomatsakis (Dec 13 2018 at 19:24, on Zulip):

no worries

nikomatsakis (Dec 13 2018 at 19:24, on Zulip):

so, what's on your mind :)

Alexander Regueiro (Dec 13 2018 at 19:26, on Zulip):

@nikomatsakis did you take a look at the PR yet? :-)

Alexander Regueiro (Dec 13 2018 at 19:28, on Zulip):

@nikomatsakis did you have a team meeting today btw? or will that be later?

nikomatsakis (Dec 13 2018 at 19:28, on Zulip):

@Alexander Regueiro do you mean this one https://github.com/rust-lang/rust/pull/56225 ?

nikomatsakis (Dec 13 2018 at 19:29, on Zulip):

if so, not yet

nikomatsakis (Dec 13 2018 at 19:29, on Zulip):

there is a lang team meeting in ~30 minutes

Alexander Regueiro (Dec 13 2018 at 19:29, on Zulip):

the one I linked to earlier

nikomatsakis (Dec 13 2018 at 19:29, on Zulip):

Implement RFC 2338, "Type alias enum variants" #56225

Alexander Regueiro (Dec 13 2018 at 19:29, on Zulip):

so no, the trait alias one :-)

nikomatsakis (Dec 13 2018 at 19:29, on Zulip):

that's the one I linked to, anyway

Alexander Regueiro (Dec 13 2018 at 19:29, on Zulip):

@Vadim Petrochenkov will review that one

nikomatsakis (Dec 13 2018 at 19:29, on Zulip):

oh oh

Alexander Regueiro (Dec 13 2018 at 19:29, on Zulip):

he already has in fact

Alexander Regueiro (Dec 13 2018 at 19:29, on Zulip):

just needs a final review

nikomatsakis (Dec 13 2018 at 19:30, on Zulip):

I think I r+'d it

Alexander Regueiro (Dec 13 2018 at 19:30, on Zulip):

sorry for the confusion: I was only asking about that one because of the type inference issue

nikomatsakis (Dec 13 2018 at 19:30, on Zulip):

This one:

Various fixes to trait alias feature #55994

Alexander Regueiro (Dec 13 2018 at 19:30, on Zulip):

okay cool!

Alexander Regueiro (Dec 13 2018 at 19:30, on Zulip):

I hadn't checked yet

Alexander Regueiro (Dec 13 2018 at 19:30, on Zulip):

thanks

nikomatsakis (Dec 13 2018 at 19:31, on Zulip):

yeah, those changes looked good

Alexander Regueiro (Dec 13 2018 at 19:31, on Zulip):

I presume you're happy with the linting behaviour there and similarly how @Ariel Ben-Yehuda's lint for trait ordering in trait objects behaves

Alexander Regueiro (Dec 13 2018 at 19:31, on Zulip):

great.

Alexander Regueiro (Dec 21 2018 at 16:04, on Zulip):

hey @nikomatsakis

Alexander Regueiro (Dec 21 2018 at 16:06, on Zulip):

was wondering what your opinion is to do about the crates obstructing the bug fix for duplicate auto traits

centril (Dec 21 2018 at 16:07, on Zulip):

@Alexander Regueiro can you say more?

Alexander Regueiro (Dec 21 2018 at 16:09, on Zulip):

@centril Here's a dependency graph of crates that would be broken by the bugfix: https://gist.github.com/d305248be22d77c97c60dc0b1833dd46

centril (Dec 21 2018 at 16:10, on Zulip):

@Alexander Regueiro root regressions at top? -- I thought traitobject was the only root

Alexander Regueiro (Dec 21 2018 at 16:10, on Zulip):

yeah I think so

Alexander Regueiro (Dec 21 2018 at 16:10, on Zulip):

and I thought so too

Alexander Regueiro (Dec 21 2018 at 16:10, on Zulip):

it's the main one by far still

Alexander Regueiro (Dec 21 2018 at 16:10, on Zulip):

by number of traits, I think

Alexander Regueiro (Dec 21 2018 at 16:10, on Zulip):

but definitely not the only

Alexander Regueiro (Dec 21 2018 at 16:10, on Zulip):

(it seems)

centril (Dec 21 2018 at 16:11, on Zulip):

@Alexander Regueiro what do you mean by "obstructing" ... is there someone refusing to do it?

Alexander Regueiro (Dec 21 2018 at 16:11, on Zulip):

no

Alexander Regueiro (Dec 21 2018 at 16:11, on Zulip):

I mean the current state of the codebases

Alexander Regueiro (Dec 21 2018 at 16:11, on Zulip):

for those crates

centril (Dec 21 2018 at 16:11, on Zulip):

@Alexander Regueiro well; filing PRs against root regressions would help the situation

Alexander Regueiro (Dec 21 2018 at 16:11, on Zulip):

I'm not going to file PRs for all of them :-P

Alexander Regueiro (Dec 21 2018 at 16:11, on Zulip):

issues maybe

centril (Dec 21 2018 at 16:12, on Zulip):

@Alexander Regueiro or issues; that is also good

Alexander Regueiro (Dec 21 2018 at 16:12, on Zulip):

okay

Alexander Regueiro (Dec 21 2018 at 16:12, on Zulip):

I could do that

centril (Dec 21 2018 at 16:12, on Zulip):

@Alexander Regueiro is this just for duplicate-auto and not for order-dependent?

Alexander Regueiro (Dec 21 2018 at 16:12, on Zulip):

yes. well even more specifically than that...

Alexander Regueiro (Dec 21 2018 at 16:12, on Zulip):

incoherence caused by duplicate-auto

Alexander Regueiro (Dec 21 2018 at 16:13, on Zulip):

I mean, technically incoherence... auto-trait impls don't have methods of coufse

centril (Dec 21 2018 at 16:13, on Zulip):

@Alexander Regueiro well yes; the forward compat warning is about types that are different today but will become the same tomorrow

centril (Dec 21 2018 at 16:14, on Zulip):

@Alexander Regueiro you might want to deal with creating issues for order-dependent as well... my feeling is that the set of root regression crates are roughly the same

Alexander Regueiro (Dec 21 2018 at 16:14, on Zulip):

actually...

Alexander Regueiro (Dec 21 2018 at 16:15, on Zulip):

I think the previous crater run just looked for duplicate auto traits

Alexander Regueiro (Dec 21 2018 at 16:15, on Zulip):

and errored

Alexander Regueiro (Dec 21 2018 at 16:15, on Zulip):

it didn't look at coherence

Alexander Regueiro (Dec 21 2018 at 16:15, on Zulip):

maybe we should do another crater run

Alexander Regueiro (Dec 21 2018 at 16:15, on Zulip):

where it only errors for incoherence

centril (Dec 21 2018 at 16:15, on Zulip):

@Alexander Regueiro oh; ok -- hopefully the will-error-due-to-coherence will be smaller once you've nailed down the error condition

Alexander Regueiro (Dec 21 2018 at 16:16, on Zulip):

exactly

Alexander Regueiro (Dec 21 2018 at 16:16, on Zulip):

and if that's really tiny...

Alexander Regueiro (Dec 21 2018 at 16:16, on Zulip):

we can go straight to hard error :-)

Alexander Regueiro (Dec 21 2018 at 16:16, on Zulip):

which Niko is happy with I think.

Alexander Regueiro (Dec 21 2018 at 16:16, on Zulip):

certainly, if it's just traitobject

Alexander Regueiro (Dec 21 2018 at 16:16, on Zulip):

or possibly just a small handful

Alexander Regueiro (Dec 21 2018 at 16:16, on Zulip):

of infrequently used crates

centril (Dec 21 2018 at 16:17, on Zulip):

@Alexander Regueiro well... traitobject is a very small root regression but it has a tonne of reverse dependencies

Alexander Regueiro (Dec 21 2018 at 16:17, on Zulip):

@centril yep, annoyingly. and the author has been AWOL for like 1.5 years or more :-P

Alexander Regueiro (Dec 21 2018 at 16:17, on Zulip):

and doesn't reply to issues/PRs/emails

Alexander Regueiro (Dec 21 2018 at 16:18, on Zulip):

so at some point we may have to say "screw it, the hard error is going into nightly, you have between 6 and 12 weeks to fix to fix your library and release before it gets broken on stable." ;-)

Alexander Regueiro (Dec 21 2018 at 16:18, on Zulip):

it's a bug-fix after all

centril (Dec 21 2018 at 16:27, on Zulip):

@Alexander Regueiro well no... https://github.com/jupyterlab/jupyterlab/issues/5789

centril (Dec 21 2018 at 16:27, on Zulip):

2 days ago

Alexander Regueiro (Dec 21 2018 at 16:27, on Zulip):

huh?

Alexander Regueiro (Dec 21 2018 at 16:27, on Zulip):

oh yes

Alexander Regueiro (Dec 21 2018 at 16:28, on Zulip):

he just refuses to reply to anyhting Rust related evidently :-P

Alexander Regueiro (Dec 21 2018 at 16:28, on Zulip):

at least from me

Alexander Regueiro (Dec 21 2018 at 16:36, on Zulip):

@centril

centril (Dec 21 2018 at 16:38, on Zulip):

@Alexander Regueiro maybe you can reach out to reem's "associates" and see if they can reach out to them?

Alexander Regueiro (Dec 21 2018 at 16:38, on Zulip):

no

Alexander Regueiro (Dec 21 2018 at 16:38, on Zulip):

I don't really feel it's my duty (or that of the Rust team) to do this.

Alexander Regueiro (Dec 21 2018 at 16:38, on Zulip):

we've created an issue, PR, and emailed him

Alexander Regueiro (Dec 21 2018 at 16:38, on Zulip):

we'll give him a bit more time

centril (Dec 21 2018 at 16:39, on Zulip):

its true

Alexander Regueiro (Dec 21 2018 at 16:39, on Zulip):

but after that, it's his problem. :-)

Alexander Regueiro (Dec 21 2018 at 16:39, on Zulip):

hopefully there won't be any more offending traits after the new crater run...

centril (Dec 21 2018 at 16:40, on Zulip):

@Alexander Regueiro the problem with "his problem" / "screw it" is that there are all these other crates that rely on traitobject which will be broken... its not that breaking traitobject is a major problem; its breaking all those reverse dependencies

centril (Dec 21 2018 at 16:40, on Zulip):

so we need to be creative

Alexander Regueiro (Dec 21 2018 at 16:40, on Zulip):

I agree...

Alexander Regueiro (Dec 21 2018 at 16:40, on Zulip):

here's one idea:

centril (Dec 21 2018 at 16:40, on Zulip):

its a pickle :confused:

Alexander Regueiro (Dec 21 2018 at 16:40, on Zulip):

(yep)

centril (Dec 21 2018 at 16:40, on Zulip):

too bad the crate doesn't have more owners

Alexander Regueiro (Dec 21 2018 at 16:41, on Zulip):

-- we create a normal duplicate auto-traits lint, but add a note saying "this is dangerous, because it can lead to incoherence if you implement the same trait for objects types differing only by duplicate auto traits"

Alexander Regueiro (Dec 21 2018 at 16:41, on Zulip):

and then hard error after one more cycle

Alexander Regueiro (Dec 21 2018 at 16:41, on Zulip):

well, keep the lint around

Alexander Regueiro (Dec 21 2018 at 16:41, on Zulip):

but hard error the coherence thing

Alexander Regueiro (Dec 21 2018 at 16:41, on Zulip):

it's just, the coherence thing is very difficult to lint against, so the above might be a good compromise?

Alexander Regueiro (Dec 21 2018 at 16:42, on Zulip):

would be curious what @nikomatsakis thinks of this too.

centril (Dec 21 2018 at 16:42, on Zulip):

yeah I'd check with niko re. implementation difficulties

centril (Dec 21 2018 at 16:42, on Zulip):

maybe he can help out

Alexander Regueiro (Dec 21 2018 at 16:42, on Zulip):

I also meant the above strategy

Alexander Regueiro (Dec 21 2018 at 16:42, on Zulip):

but yes...

centril (Dec 21 2018 at 16:43, on Zulip):

ideally we'd like to have a lint for duplicates which we'll keep as warn-by-default forever and then we have a second warning-will-become-error lint for the narrower subset of "duplicates becoming different types"-problem

Alexander Regueiro (Dec 21 2018 at 16:44, on Zulip):

yes, but I'm saying the second bit might be too difficult

Alexander Regueiro (Dec 21 2018 at 16:44, on Zulip):

so we could merge it into the first as a temporary "note: dangerous" thing

Alexander Regueiro (Dec 21 2018 at 16:44, on Zulip):

but let's see

centril (Dec 21 2018 at 16:45, on Zulip):

@Alexander Regueiro Yeah I understand; maybe Niko can help make it easier for you or if you cannot maybe Niko can impl it...

Alexander Regueiro (Dec 21 2018 at 16:45, on Zulip):

yes. or maybe he'll say it's too hard too...

Alexander Regueiro (Dec 21 2018 at 16:45, on Zulip):

I'm not sure.

Alexander Regueiro (Dec 21 2018 at 16:45, on Zulip):

hopefully he'll be around later today.

centril (Dec 21 2018 at 16:45, on Zulip):

@Alexander Regueiro we have to figure out the coherence thing anyways since otherwise we don't have the condition that will eventually become an error

centril (Dec 21 2018 at 16:45, on Zulip):

since we cannot just make all duplicate auto traits an error

Alexander Regueiro (Dec 21 2018 at 16:46, on Zulip):

I know

Alexander Regueiro (Dec 21 2018 at 16:46, on Zulip):

I wasn't suggesting that

Alexander Regueiro (Dec 21 2018 at 16:46, on Zulip):

it would be a note on the duplicate traits lint (default: warning)

Alexander Regueiro (Dec 21 2018 at 18:33, on Zulip):

@centril note: #[deny(order_dependent_trait_objects)] on by default

Alexander Regueiro (Dec 21 2018 at 18:33, on Zulip):

err lol wut?

Alexander Regueiro (Dec 21 2018 at 18:33, on Zulip):

looks like arielby has already broken backwards compatibility haha

centril (Dec 21 2018 at 18:35, on Zulip):

^-- @Ariel Ben-Yehuda

Alexander Regueiro (Dec 21 2018 at 18:36, on Zulip):

I can change to warn-by-default easily enough in my PR. or he can make a separate one ASAP. I think we want that?

centril (Dec 21 2018 at 18:36, on Zulip):

@Alexander Regueiro link to PR which broke stuff?

Alexander Regueiro (Dec 21 2018 at 18:37, on Zulip):

not sure

Alexander Regueiro (Dec 21 2018 at 18:37, on Zulip):

ask him

centril (Dec 21 2018 at 18:39, on Zulip):

in https://github.com/rust-lang/rust/pull/56481

Alexander Regueiro (Dec 21 2018 at 18:40, on Zulip):

@centril yep, looks right. :-)

Alexander Regueiro (Dec 21 2018 at 18:41, on Zulip):

I was searching for "order' in my browser history

Alexander Regueiro (Dec 21 2018 at 18:41, on Zulip):

heh

centril (Dec 21 2018 at 18:41, on Zulip):

@Alexander Regueiro oh; this one had just 2 regressions where one was traitobject (not sure why it didnt break reverse dependent crates) and the other was a barely used crate

Alexander Regueiro (Dec 21 2018 at 18:41, on Zulip):

no wonder

centril (Dec 21 2018 at 18:41, on Zulip):

so it doesn't seem so bad

Alexander Regueiro (Dec 21 2018 at 18:41, on Zulip):

it must have been warn-by-default when the crater run was performed

Alexander Regueiro (Dec 21 2018 at 18:41, on Zulip):

because it clearly breaks traitobject when on deny

Alexander Regueiro (Dec 21 2018 at 18:42, on Zulip):

or... maybe it's the interaction of his PR with my PR?

Alexander Regueiro (Dec 21 2018 at 18:42, on Zulip):

hmm

centril (Dec 21 2018 at 18:42, on Zulip):

hmm; cc @nikomatsakis

Alexander Regueiro (Dec 21 2018 at 18:42, on Zulip):

yeah, not sure any more. could be an interactional thing.

centril (Dec 21 2018 at 18:42, on Zulip):

ill ask on github

Alexander Regueiro (Dec 21 2018 at 18:42, on Zulip):

ta

centril (Dec 21 2018 at 18:43, on Zulip):

seems we went for deny in the PR

centril (Dec 21 2018 at 18:43, on Zulip):

so it wasnt interactional

Alexander Regueiro (Dec 21 2018 at 18:46, on Zulip):

I know... but I'm whether it was deny when the crater run was made. presumably yes?

Alexander Regueiro (Dec 21 2018 at 18:53, on Zulip):

ah. he replied @centril

Alexander Regueiro (Dec 21 2018 at 18:53, on Zulip):

I'm not sure he's aware how many reverse deps traitobject has hehe.

nikomatsakis (Dec 26 2018 at 18:06, on Zulip):

@Alexander Regueiro left a review here -- curious to hear your thoughts on that

nikomatsakis (Jan 03 2019 at 18:16, on Zulip):

@Alexander Regueiro I left a review on your PR

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

@Taylor Cramer Ah okay, thanks for letting me now. We had our weekly meeting, but this is a fair excuse heh.

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

Thanks @nikomatsakis

Last update: Nov 18 2019 at 00:55UTC