search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.67k stars 12.75k forks source link

Cleanup and consolidate operator/method dispatch code #18741

Open nikomatsakis opened 10 years ago

nikomatsakis commented 10 years ago

Once upon a time, both operator dispatch and method lookup went through the same tangled, twisty paths. It was a mess. But lo, they were severed and made two. Each could follow its own path. Overall, this is progress. But there is still room for improvement:

  1. The second half of "operator dispatch" is basically the same as confirm::confirm, but with some slight differences.
  2. The autoderef loop for [] is basically the same as probe, but with some slight differences (e.g., at each step it consists builtin [] as well).
  3. The probe loop, which uses check::autoderef, isn't able to be part of an inference transaction between check::autoderef uses operator dispatch which adds things into the main fulfillment context, thus leaking inference types etc outside the transaction.

It feels like things could still be cleaned up a bit further, allowing for more code reuse and happiness all around.

Some FIXMEs are scattered about at relevant points of the code.

nikomatsakis commented 10 years ago

I am happy to mentor.

steveklabnik commented 8 years ago

@nikomatsakis is this ticket still valid?

nikomatsakis commented 8 years ago

On Thu, Dec 31, 2015 at 10:26:12AM -0800, Steve Klabnik wrote:

@nikomatsakis is this ticket still valid?

yes

Phyllostachys commented 8 years ago

Inspired by this and having been a long time fan of Rust and wanting to contribute, I'm interested in solving this issue. With that said, is there anything I need to know to get started? Also, where can I start looking?

nikomatsakis commented 8 years ago

@Phyllostachys to be honest it's a bit out of cache for me. I'd have to go read into the code some to see where things stand. Anyway, usually a good place to start is to try and touch base at some point on IRC or over e-mail.

Phyllostachys commented 8 years ago

I'll have to jump on IRC tonight. Who would I email if I emailed someone?

nikomatsakis commented 8 years ago

@Phyllostachys me, my e-mail is on my github page

ahmedcharles commented 8 years ago

@nikomatsakis Are you still willing to mentor this?

Mark-Simulacrum commented 7 years ago

@ahmedcharles Are you still interested? We can try to connect you with niko if so

nikomatsakis commented 7 years ago

Well, it's quite out of cache, but let me start by giving a few very basic pointers. The relevant code is now found at src/librustc_typeck/check/method/mod.rs -- the fns in question are lookup_method() and lookup_method_in_trait_adjusted(). You may find the README.md in that same directory useful.

citizen428 commented 7 years ago

@Mark-Simulacrum @nikomatsakis It's been a while since anyone showed any interest in this. If it's not picked up soon, I'll give it a shot, though I'm fairly certain I'll need more mentoring than what has a been provided so far.

Mark-Simulacrum commented 7 years ago

Let us know if you want further instructions and we'll do our best to connect you with @nikomatsakis.

citizen428 commented 7 years ago

Work got in the way a bit, but I'm still planning on tackling this. Started looking through the README and code a bit, will get back to you if I need more info.

citizen428 commented 7 years ago

Sorry for the long radio silence. Anyway, I just looked at this again and I don't think that this issue is a good fit for my current level of knowledge regarding rustc internals. Therefore I'm also not quite sure if commits like de0ffadb6722371b9eb636ee9b2d4627db9e02fa already addressed some of the issues outlined here.

sergey-melnychuk commented 4 years ago

@nikomatsakis is this still relevant? I'm not afraid to give it a try :)

Seems like FIXMEs are still there: https://github.com/rust-lang/rust/blob/9fe05e9456b84996637c2f29b35c37960e537540/src/librustc_typeck/check/mod.rs#L3424 https://github.com/rust-lang/rust/blob/175631311716d7dfeceec40d2587cde7142ffa8c/src/librustc_typeck/check/method/mod.rs#L285

nikomatsakis commented 4 years ago

To be quite honest, I don't know! It's been an awfully long time. I imagine it's still relevant, though how much things can be improved I'm not sure.

koivunej commented 3 years ago

Came across this issue through E-Mentor, looked up the above mentioned FIXME locations now more than a year later just in case someone is interested to do a more thorough peek:

https://github.com/rust-lang/rust/blob/39eee173fbcc21462d255b364b87715cd33b62db/compiler/rustc_typeck/src/check/place_op.rs#L52-L54

https://github.com/rust-lang/rust/blob/39eee173fbcc21462d255b364b87715cd33b62db/compiler/rustc_typeck/src/check/method/mod.rs#L293-L295

nikomatsakis commented 3 years ago

I'm removing E-mentor not because I wouldn't want to mentor but because i prefer to use the tag only for issues with some decent mentoring instructions, and I don't think this quite qualifies.