Stream: t-compiler/wg-nll

Topic: pr-51612-suggest-&mut


ashtneoi (Jul 06 2018 at 23:56, on Zulip):

(obligatory note: let me know if I'm doing this wrong or if I should ask somewhere else.) (also I know it's Friday evening. there's no hurry.)

ashtneoi (Jul 06 2018 at 23:59, on Zulip):

so I'm hacking on librustc_mir/borrow_check, and I'm having trouble telling the difference between an auto-deref binding and a literal ref/ref mut binding.

ashtneoi (Jul 07 2018 at 00:00, on Zulip):

I found BindingMode::BindByReference but it seems to apply to both. in theory I could check if the source span starts with "ref" followed by Pattern_Whitespace or whatever it's called, but that feels kinda hacky.

ashtneoi (Jul 07 2018 at 02:14, on Zulip):

actually I think this is almost exactly the same question I asked in the PR itself (and got an answer to). oopsie.

nikomatsakis (Jul 07 2018 at 11:23, on Zulip):

by "auto-deref binding" do you mean something like the default ref that occurs here match &foo { Some(x) => .. }?

nikomatsakis (Jul 07 2018 at 11:23, on Zulip):

in the past, we have sometimes extended the data structures to carry more information about what kind of "syntactic sugar" was used

nikomatsakis (Jul 07 2018 at 11:23, on Zulip):

so e.g. you imagine extending BindingMode to indicate whether this ref was explicit or added by us

nikomatsakis (Jul 07 2018 at 11:25, on Zulip):

I also think span-hacking .. isn't the worst thing ever, if it's simple to do, but I agree it's on the hack-y side.

nikomatsakis (Jul 07 2018 at 11:26, on Zulip):

(in the HIR at least the explicit binding mode vs actual binding mode distinction is visible...)

nikomatsakis (Jul 07 2018 at 11:28, on Zulip):

I think what we could try to do is to thread the HIR's BindingAnnotation information into the MIR, but I'd have to look at how easy/hard that would be

ashtneoi (Jul 07 2018 at 17:15, on Zulip):

hmm. sounds like it's not worth the effort unless we need it in lots of places.

nikomatsakis (Jul 07 2018 at 19:30, on Zulip):

yeah, my impression is it's not worth it right now, but it might be worth taking a look later to try as a cleanup

pnkfelix (Jul 09 2018 at 09:21, on Zulip):

hi @ashtneoi I am back from my leave of absence and had started last week looking at rebasing this PR. It sounds like you're close to getting something working though?

pnkfelix (Jul 09 2018 at 09:36, on Zulip):

(I think I just now actually got it working on my end; I ended up going with span inspection and snippet rewriting, the same way your original PR did.)

pnkfelix (Jul 09 2018 at 09:38, on Zulip):

but ... ah, maybe span/snippet inspection will not suffice, due to the hidden &-borrows injected by two-phase borrows. :(

pnkfelix (Jul 09 2018 at 09:39, on Zulip):

In particular, whatever solution we devise, we need to make sure we do not break src/test/ui/issue-27282-reborrow-ref-mut-in-guard.rs

pnkfelix (Jul 09 2018 at 09:40, on Zulip):

(where "breaking the test" here means erroneously suggesting to the user that they use ref mut mut)

ashtneoi (Jul 09 2018 at 10:06, on Zulip):

@pnkfelix as far as I know, I'm almost done with what I was trying to do. no guarantees it passes that test you mentioned though.

ashtneoi (Jul 09 2018 at 10:06, on Zulip):

I was working as if this were just a rebase, but I ended up trashing my original code, so I guess I should just push what I have so far and worry about squashing later.

pnkfelix (Jul 09 2018 at 10:07, on Zulip):

yeah the code changed a lot and i found it was easier to start from scratch (while looking at the previous PR for inspiration) rather than attempt a true rebase

ashtneoi (Jul 09 2018 at 10:07, on Zulip):

yep, same

pnkfelix (Jul 09 2018 at 10:07, on Zulip):

One "obvious" way to compensate for the particular case of issue-27282-reborrow-ref-mut-in-guard.rs would be to also detect if we started with ref mut and bail on making a suggestion in that case. But I'm a little wary of that approach

pnkfelix (Jul 09 2018 at 10:09, on Zulip):

(and I'm independently planning to revise at least part of the code in src/librustc_mir/build/matches/mod.rs to try to fix https://github.com/rust-lang/rust/issues/51348 )

ashtneoi (Jul 09 2018 at 10:20, on Zulip):

hmm, sounds like things are a lot more complicated than I expected when I opened the original PR. we'll see where I get tomorrow, but I might not have the rustc knowledge to figure this out.

pnkfelix (Jul 09 2018 at 10:39, on Zulip):

the simplest thing might be to just restrict the scope of your PR to just fix the suggestion that replaces & with &mut

pnkfelix (Jul 09 2018 at 10:39, on Zulip):

and leave the more general case for later work.

ashtneoi (Jul 09 2018 at 10:50, on Zulip):

ah, that sounds like a good idea

Last update: Nov 21 2019 at 14:55UTC