Stream: wg-traits

Topic: clause-builder


nikomatsakis (Oct 24 2019 at 12:58, on Zulip):

So I opened https://github.com/rust-lang/chalk/pull/266 -- I've also asked @tmandry to review this, though he very kindly reviewed the generic PR. This is basically just "cleanup", laying the groundwork to actually be able to move projections from something that happens during unification into something that we handle lowering. It also aims to make the program-clause logic significantly more readable -- you can browse through the final program_clauses.rs to get a sense of what I mean. Hopefully you agree -- although reading through it, I think there's still various tweaks we could do to remove a number of random clone and cast calls and try to focus the attention more on the core logic.

(Example: instead of trait_ref.clone().well_formed(), should probably change well_formed to be &self and to do the clone -- I think all callers invoke clone first. Similarly, adding methods like that to Ty instead of doing WellFormed::Ty(ty). And making some other functions take a impl Cast<T> instead of T, so that callers don't have to write cast. And lifting various operations to iterators, to avoid map. And so forth. But I'd rather do that sort of thing later. )

nikomatsakis (Oct 24 2019 at 12:59, on Zulip):

(Next step after this, I think, is to try and make the clause logic generic over the target TF)

nikomatsakis (Oct 24 2019 at 13:00, on Zulip):

(But I've not started that)

nikomatsakis (Oct 24 2019 at 13:02, on Zulip):

Oh, and cc @Florian Diebold -- this PR makes a number of changes to the rust-ir data structures. The biggest change being to associated type values and how their binders work. I did try to document pretty thoroughly, but I'm sure I missed stuff.

nikomatsakis (Oct 24 2019 at 13:03, on Zulip):

I suppose I could attempt to rebase rust-analyzer over these changes as a way to get a bit more into the code.

Florian Diebold (Oct 24 2019 at 13:04, on Zulip):

note that we haven't updated to the dyn/impl changes, so you would have to fix those or cherry-pick the changes from https://github.com/rust-analyzer/rust-analyzer/pull/2030 as well

nikomatsakis (Oct 24 2019 at 13:05, on Zulip):

it probably makes step to bump the commit in steps

nikomatsakis (Oct 24 2019 at 13:05, on Zulip):

actually i'm not sure if that's true

Florian Diebold (Oct 24 2019 at 13:55, on Zulip):

I guess we could actually update to newest Chalk master and just not convert our Dyn to Chalk's Dyn for now to avoid the test failures

Laurențiu Nicola (Oct 24 2019 at 13:57, on Zulip):

IIRC it panics in chalk, which is a bit worse than some failing tests :smiley:

Florian Diebold (Oct 24 2019 at 14:04, on Zulip):

yeah I guess also panics during real usage ;)

nikomatsakis (Oct 24 2019 at 15:09, on Zulip):

Yeah chalk's dyn support is just not complete

nikomatsakis (Oct 24 2019 at 15:09, on Zulip):

Not far away though

nikomatsakis (Oct 24 2019 at 15:09, on Zulip):

I .. think :)

Florian Diebold (Oct 26 2019 at 14:32, on Zulip):

@nikomatsakis I've upgraded RA to current Chalk master

Last update: Nov 12 2019 at 15:35UTC