Stream: t-compiler/wg-nll

Topic: deny-nll-warning-lints


centril (Jul 26 2019 at 16:51, on Zulip):

I think it might be a good idea to move the NLL warnings to deny; at least on Rust 2018... wdyt?

cc @lqd

lqd (Jul 26 2019 at 16:54, on Zulip):

interesting thought, cc @WG-nll

lqd (Jul 26 2019 at 16:55, on Zulip):

I can't remember if we had outstanding or planned tasks, beyond the questions of "should we do both 2015 and 2018 at the same time"

lqd (Jul 26 2019 at 16:56, on Zulip):

seems like 2018 first from lang's meeting in march

lqd (Jul 26 2019 at 16:59, on Zulip):

(Matthew will remember more of the blockers for this)

centril (Jul 26 2019 at 16:59, on Zulip):

I'm thinking that it should be a minimal conservative next step since people using Rust 2018 likely saw migration errors when they switched editions

Matthew Jasper (Jul 26 2019 at 17:00, on Zulip):

I would rather not have edition differences.

centril (Jul 26 2019 at 17:00, on Zulip):

We can then follow up in two orthogonal directions:
- Move it to an Error on Rust 2018
- Move it to Deny on 2015

lqd (Jul 26 2019 at 17:00, on Zulip):

for technical reasons or process reasons (eg speeding up the process) Matthew ?

centril (Jul 26 2019 at 17:00, on Zulip):

@Matthew Jasper can you elaborate on why not?

centril (Jul 26 2019 at 17:01, on Zulip):

Tech-wise it seems like a trivial change to https://github.com/rust-lang/rust/pull/60914/files would do?

Matthew Jasper (Jul 26 2019 at 17:02, on Zulip):

Because (a) we don't handle macros correctly and (b) I don't think that we should be abusing editions as a migration system.

lqd (Jul 26 2019 at 17:02, on Zulip):

users can't manually deny/allow these warnings yet, right ?

centril (Jul 26 2019 at 17:03, on Zulip):

ah it's not a lint

lqd (Jul 26 2019 at 17:03, on Zulip):

(I feel some 2015 users asked for that recently-ish)

Matthew Jasper (Jul 26 2019 at 17:03, on Zulip):

They're not lints, so no.

centril (Jul 26 2019 at 17:03, on Zulip):

@lqd asked for making them allow, or making them deny?

Matthew Jasper (Jul 26 2019 at 17:04, on Zulip):

It should not be possible to allow them, if we do make them lints.

lqd (Jul 26 2019 at 17:04, on Zulip):

@centril my recollection is they wanted to deny them, because it showed a soundness issue they'd rather not have

lqd (Jul 26 2019 at 17:05, on Zulip):

if they can't allow them, what's the difference between making them lints and hard errors ?

centril (Jul 26 2019 at 17:05, on Zulip):

heh... we could have used the minimum lint infrastructure for this... =P

centril (Jul 26 2019 at 17:05, on Zulip):

(to make it a lint that cannot be allowed)

Matthew Jasper (Jul 26 2019 at 17:06, on Zulip):

To have warn/deny/forbid

centril (Jul 26 2019 at 17:06, on Zulip):

@lqd --cap-lints is also relevant here

lqd (Jul 26 2019 at 17:06, on Zulip):

true

centril (Jul 26 2019 at 17:06, on Zulip):

@Matthew Jasper so the warning ignores --cap-lints also then?

lqd (Jul 26 2019 at 17:07, on Zulip):

I don't think we had expected people to opt into the errors early on 2015, rather feared people would be upset their code started breaking :)

Matthew Jasper (Jul 26 2019 at 17:07, on Zulip):

It respects --cap-lints warn, and treats --cap-lints allow as warn, I guess.

centril (Jul 26 2019 at 17:08, on Zulip):

ah; so it's basically exactly as the min-lint-levels infrastructure then

centril (Jul 26 2019 at 17:09, on Zulip):

@lqd @Matthew Jasper should we perhaps try another crater run and see if anything changed since 2 months ago + if the blog post had any impact?

centril (Jul 26 2019 at 17:09, on Zulip):

I'm looking for something we can do to make some progress ^^

Matthew Jasper (Jul 26 2019 at 17:09, on Zulip):

Maybe it doesn't matter, if someone really wants to use cap-lints allow

lqd (Jul 26 2019 at 17:10, on Zulip):

I think a lot of what we saw in the run was old crates, which should still fail today ?

lqd (Jul 26 2019 at 17:10, on Zulip):

@Matthew Jasper yeah, and we'll have hard errors "soon enough" anyway

centril (Jul 26 2019 at 17:11, on Zulip):

@lqd sure; but I would like to see if perhaps we've gotten closer to "it is acceptable to make it a hard error"

centril (Jul 26 2019 at 17:11, on Zulip):

Do y'all feel we are there yet?

lqd (Jul 26 2019 at 17:11, on Zulip):

@centril I mean, we could for sure

Matthew Jasper (Jul 26 2019 at 17:11, on Zulip):

I don't expect a significant reduction in abandoned repos with an outdated URL dependency

lqd (Jul 26 2019 at 17:11, on Zulip):

me neither tbh

centril (Jul 26 2019 at 17:12, on Zulip):

I'm getting the sense that you don't think another year would change that substantially?

lqd (Jul 26 2019 at 17:12, on Zulip):

but maybe there could be an effort of making PRs (seems like a tall order but you never know) to some crates I dunno

Matthew Jasper (Jul 26 2019 at 17:12, on Zulip):

Maybe the crates.io situation is better though.

centril (Jul 26 2019 at 17:12, on Zulip):

abandoned repos with an outdated URL dependency

I largely don't care about abandonware ^^

lqd (Jul 26 2019 at 17:12, on Zulip):

@Matthew Jasper do you happen to remember if we have an issue regarding the work to do with macros, if any ? (I'm guessing you're on your phone, if so, I can search myself if you have a couple "keywords" from the top of your head)

lqd (Jul 26 2019 at 17:14, on Zulip):

@centril I can look at the "old" results we have, sort out some of the more active crates, just to have some numbers wrt "abandonware" and so on

centril (Jul 26 2019 at 17:15, on Zulip):

@lqd the crater queue is also empty atm so I think we can run another crater run in parallel

lqd (Jul 26 2019 at 17:15, on Zulip):

true as well

lqd (Jul 26 2019 at 17:15, on Zulip):

I'm not expecting a big change tbh

Pietro Albini (Jul 26 2019 at 17:15, on Zulip):

oh please don't start crater runs just yet

Pietro Albini (Jul 26 2019 at 17:15, on Zulip):

I need to deploy some changes

Pietro Albini (Jul 26 2019 at 17:16, on Zulip):

(and yes, I have notifications every time someone writes crater on zulip :P)

lqd (Jul 26 2019 at 17:16, on Zulip):

I was gonna ask :p

centril (Jul 26 2019 at 17:16, on Zulip):

LOL :D

Matthew Jasper (Jul 26 2019 at 17:17, on Zulip):

Well if it's just a lint level difference, then I think it can just be hamdled by the lint level infrastructure.

lqd (Jul 26 2019 at 17:19, on Zulip):

@Matthew Jasper if you wanted to do both editions at the same time, we'd have to let 2015 as-is for a little while more

lqd (Jul 26 2019 at 17:20, on Zulip):

right ?

lqd (Jul 26 2019 at 17:21, on Zulip):

at least for a couple more releases, that is

Cam Cope (Jul 26 2019 at 17:21, on Zulip):

cargo feature idea: suggest adding dependency replacements to cargo.toml for package versions known to be broken on your compiler version

lqd (Jul 26 2019 at 17:23, on Zulip):

or opening issues on the repos we know will break, maybe PRs for the popular dependencies only requiring a minor rev update

lqd (Jul 26 2019 at 17:24, on Zulip):

(but if they're indeed inactive, maybe it wouldn't matter much anyway)

pnkfelix (Jul 26 2019 at 17:24, on Zulip):

IMO our focus should first be on resolving #34596 (in a manner acceptable to community/teams at large) before we make waves here

lqd (Jul 26 2019 at 17:24, on Zulip):

(also we should be having this conversation with niko and felix present as well :)

pnkfelix (Jul 26 2019 at 17:24, on Zulip):

(But I’m not really here)

lqd (Jul 26 2019 at 17:25, on Zulip):

you need to enjoy your time off @pnkfelix :)

lqd (Jul 26 2019 at 17:26, on Zulip):

interesting issue indeed

Cam Cope (Jul 26 2019 at 17:26, on Zulip):

@lqd yeah, my idea is for the cases after all of that has been done, and you've either got an old cargo.lock or overly constrained transitive dependencies

Cam Cope (Jul 26 2019 at 17:27, on Zulip):

and it would prevent people from getting weird errors that have known fixes

Pietro Albini (Jul 26 2019 at 17:29, on Zulip):

y'all can start crater runs now!

lqd (Jul 26 2019 at 17:30, on Zulip):

maybe that can be an option; could be hard to be able to recommend alternative dependencies in a meaningful/useful way I dunno; seems unlikely we'd require this feature to change lints levels or have errors ?

Cam Cope (Jul 26 2019 at 17:34, on Zulip):

I'm just thinking of a basic blacklist of package versions for each compiler version, and replacement recommendations if a fix was made. only for widely-used packages like url though

lqd (Jul 26 2019 at 17:38, on Zulip):

at the very least I'll look at gathering more numbers about the previous results @centril (I think craterbot missed some commands while it was down, so the queue might not be really empty) -- until we know how to proceed wrt the lint, the topic of #34596, the possible remaining work on macros, which editions to do the change on, and so on

centril (Jul 26 2019 at 17:41, on Zulip):

@lqd Alright; I'll reopen your PR for now and start crater

centril (Jul 26 2019 at 17:41, on Zulip):

under "can't hurt"

centril (Jul 26 2019 at 17:44, on Zulip):

Bummer, merge conflicts... will make a new PR instead then

lqd (Jul 26 2019 at 17:47, on Zulip):

(If it’s not urgent, ie not right now, I can also take care of it; but it’s not super complicated as you’ve seen :)

centril (Jul 26 2019 at 17:51, on Zulip):

Yeah, seems easy enough

centril (Jul 26 2019 at 17:52, on Zulip):

and not urgent either ^^

RalfJ (Jul 26 2019 at 18:31, on Zulip):

IMO our focus should first be on resolving #34596 (in a manner acceptable to community/teams at large) before we make waves here

I was told some time ago that future-compat lints are shown when compiling dependencies. have I been lied to?

centril (Jul 26 2019 at 19:01, on Zulip):

@RalfJ They most certainly are not

lqd (Jul 31 2019 at 13:36, on Zulip):

@centril some statistics about the root regressions of full NLLs here for the crates published on crates.io

Pietro Albini (Sep 05 2019 at 14:06, on Zulip):

I'd like a crater run to be started to check what the impact of denying nll migration warnings just on rust 2018 is

Pietro Albini (Sep 05 2019 at 14:07, on Zulip):

the latest crater run denying on both 2015 and 2018 showed a bunch of regressions (1600+), and there are concerns (for example mines, see #release on discord) about breaking that many crates

Pietro Albini (Sep 05 2019 at 14:09, on Zulip):

if most of those of those regressions only happen on 2015 crates it might make a lot of sense to deny the warnings on 2018 asap (to prevent new regressions from appearing) and later discuss what to do with 2015

Pietro Albini (Sep 05 2019 at 14:09, on Zulip):

@centril @Matthew Jasper @nikomatsakis @simulacrum

simulacrum (Sep 05 2019 at 14:34, on Zulip):

I would be fine with that, yes

nikomatsakis (Sep 05 2019 at 14:40, on Zulip):

that sounds smart

nikomatsakis (Sep 05 2019 at 14:40, on Zulip):

I was thining that we should be making it a priority to tie this up

nikomatsakis (Sep 05 2019 at 14:40, on Zulip):

and I agree that's a good step to take

simulacrum (Sep 05 2019 at 14:40, on Zulip):

It sounds like @Matthew Jasper was against that historically based on Discord discussion

simulacrum (Sep 05 2019 at 14:41, on Zulip):

(to avoid edition divergence)

simulacrum (Sep 05 2019 at 14:41, on Zulip):

FWIW we can probably fairly easily determine which edition they are in w/o a crater run

simulacrum (Sep 05 2019 at 14:41, on Zulip):

I _suspect_ most are 2015

simulacrum (Sep 05 2019 at 14:42, on Zulip):

I think another possible step is to move the lint to deny by default on all editions

centril (Sep 05 2019 at 14:42, on Zulip):

@simulacrum it's not really a lint

simulacrum (Sep 05 2019 at 14:42, on Zulip):

well, whatever you call it, doesn't really matter

centril (Sep 05 2019 at 14:42, on Zulip):

it is just a warning emitted unconditionally and not respecting the --cap-lints

centril (Sep 05 2019 at 14:43, on Zulip):

@simulacrum if you make it "deny" you make it an error everywhere

nikomatsakis (Sep 05 2019 at 14:43, on Zulip):

we could plausibly change that

centril (Sep 05 2019 at 14:43, on Zulip):

i.e. migrate mode is over

nikomatsakis (Sep 05 2019 at 14:43, on Zulip):

but in any case I think making it a hard error on 2018 as a step seems reasonable

nikomatsakis (Sep 05 2019 at 14:43, on Zulip):

certainly at least to crater it seems good

centril (Sep 05 2019 at 14:44, on Zulip):

I'd personally like to go further and outright make it all hard errors everywhere

centril (Sep 05 2019 at 14:44, on Zulip):

since AST borrowck is a pretty bad maintenance burden atm

nikomatsakis (Sep 05 2019 at 14:44, on Zulip):

yes, I was thinking I'd like to make it an end-of-year goal to see if we can get rid of AST borrowck

centril (Sep 05 2019 at 14:45, on Zulip):

Matthew's plan was for 1.40

centril (Sep 05 2019 at 14:45, on Zulip):

that's like... end of year, I think

centril (Sep 05 2019 at 14:46, on Zulip):

@nikomatsakis so people who switched to edition 2018 must have seen this warning, so I think it's safe to switch those to an error without crater also

centril (Sep 05 2019 at 14:46, on Zulip):

Matthew made a point re. macro hygiene tho

nikomatsakis (Sep 05 2019 at 14:46, on Zulip):

it seems like one step might be to broadcast our intentions pretty loudly

centril (Sep 05 2019 at 14:46, on Zulip):

which is apt, but perhaps not too severe if on a temporary basis

nikomatsakis (Sep 05 2019 at 14:46, on Zulip):

particularly for cases like url where there exist fixes

nikomatsakis (Sep 05 2019 at 14:46, on Zulip):

but people need to cargo update

nikomatsakis (Sep 05 2019 at 14:47, on Zulip):

i.e., if it comes down to a list of crates that can be fixed

simulacrum (Sep 05 2019 at 14:47, on Zulip):

realistically, many of those are likely abandoned git repos, but yes

nikomatsakis (Sep 05 2019 at 14:47, on Zulip):

maybe we could also do some kind of "email blast" or other automated notification

centril (Sep 05 2019 at 14:47, on Zulip):

@nikomatsakis worth noting that we broadcasted as widely as we possibly can in https://blog.rust-lang.org/2019/07/04/Rust-1.36.0.html#nll-for-rust-2015

simulacrum (Sep 05 2019 at 14:47, on Zulip):

I don't think that's as wide as possible

simulacrum (Sep 05 2019 at 14:47, on Zulip):

I'm thinking a standalone blog post

centril (Sep 05 2019 at 14:47, on Zulip):

well I suppose targeted emails is better :P

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

confirm

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

well, also

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

it's about the things we say

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

I mean that's a good 1st step :)

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

but if we have a standalone blog post and

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

we advertise specific actions

Pietro Albini (Sep 05 2019 at 14:48, on Zulip):

rethrowing my idea from #release, how bad would it be to allow those holes on MIR just for 2015?

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

e.g., "to check if you are affected, try this"

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

and "here is how you fix the major things we know about"

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

that feels good to me

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

rethrowing my idea from #release, how bad would it be to allow those holes on MIR just for 2015?

very bad :)

nikomatsakis (Sep 05 2019 at 14:48, on Zulip):

in partcular, it'd be hard to maintain multiple versions of the borrow checker

nikomatsakis (Sep 05 2019 at 14:49, on Zulip):

(leaving aside the correctness concerns)

centril (Sep 05 2019 at 14:49, on Zulip):

@nikomatsakis did you see Felix's blog post?

centril (Sep 05 2019 at 14:49, on Zulip):

it has bunch of "here's how you fix it"

nikomatsakis (Sep 05 2019 at 14:49, on Zulip):

I had forgotten it, but yes

Pietro Albini (Sep 05 2019 at 14:49, on Zulip):

it's on felix's blog though, not blog.rust-lang.org

centril (Sep 05 2019 at 14:49, on Zulip):

we can always re-broadcast that

nikomatsakis (Sep 05 2019 at 14:49, on Zulip):

I think we would want to point into that post

centril (Sep 05 2019 at 14:49, on Zulip):

(we link to Felix's blog post in the release blog post)

nikomatsakis (Sep 05 2019 at 14:50, on Zulip):

my main point @centril is that no single post is as loud as we can :)

centril (Sep 05 2019 at 14:50, on Zulip):

@nikomatsakis yes yes, it's a good point :P

nikomatsakis (Sep 05 2019 at 14:50, on Zulip):

but you're right that we should link to that post, definitely

centril (Sep 05 2019 at 14:50, on Zulip):

I was just reminding y'alls of Felix's blog post is all :P

simulacrum (Sep 05 2019 at 14:50, on Zulip):

I'm planning to try and followup with all or at least most of the root regressions and such over the next week or two to try and get them fixed (PRs, etc.)

simulacrum (Sep 05 2019 at 14:50, on Zulip):

that does leave the long tail of ~1000 or so dependent crates

simulacrum (Sep 05 2019 at 14:51, on Zulip):

(most of which are github repos)

centril (Sep 05 2019 at 14:51, on Zulip):

are there particular holes which had fewer regressions?

centril (Sep 05 2019 at 14:51, on Zulip):

maybe we can close those specifically in the interim

nikomatsakis (Sep 05 2019 at 14:51, on Zulip):

can we make a gist with the high-levle bullet points and plan? I'd like to run this by core team

centril (Sep 05 2019 at 14:51, on Zulip):

like with the bind-by-move story

simulacrum (Sep 05 2019 at 14:51, on Zulip):

Sure, yeah, though not sure how much that buys us

simulacrum (Sep 05 2019 at 14:51, on Zulip):

I can try and get a gist put together yes

centril (Sep 05 2019 at 14:52, on Zulip):

@simulacrum the good morale from the appearance of progress? :P

centril (Sep 05 2019 at 14:52, on Zulip):

@nikomatsakis meanwhile, let's discuss 2018-to-error on T-lang today?

nikomatsakis (Sep 05 2019 at 14:53, on Zulip):

it sounds like the tl;dr is sort of

?

nikomatsakis (Sep 05 2019 at 14:53, on Zulip):

I am not sure if 1.40 is realistic, but sounds plausible

nikomatsakis (Sep 05 2019 at 14:53, on Zulip):

I guess it is, that's end of year

centril (Sep 05 2019 at 14:53, on Zulip):

additionally: possibly turn specific i-unsound issues into errors

centril (Sep 05 2019 at 14:54, on Zulip):

(we already closed 2 ones)

nikomatsakis (Sep 05 2019 at 14:54, on Zulip):

yep

nikomatsakis (Sep 05 2019 at 14:54, on Zulip):

I guess the way to approach that might be to see if we can make all of them hard errors except a few key choices

nikomatsakis (Sep 05 2019 at 14:54, on Zulip):

i.e., those that affect the root deps

nikomatsakis (Sep 05 2019 at 14:54, on Zulip):

not sure if that makes any sense or not

centril (Sep 05 2019 at 14:54, on Zulip):

like checking for specific crate names?

nikomatsakis (Sep 05 2019 at 14:55, on Zulip):

oh, wow, I hadn't thought of going that far but... not the worst idea :)

nikomatsakis (Sep 05 2019 at 14:55, on Zulip):

I meant more like "categories of errors"

centril (Sep 05 2019 at 14:55, on Zulip):

it's very naughty :P

centril (Sep 05 2019 at 14:56, on Zulip):

@nikomatsakis iirc there are just 4 NLL-fixed-by-NLL + I-unsound issues remaining

centril (Sep 05 2019 at 14:56, on Zulip):

so... it's not a whole lot :D

nikomatsakis (Sep 05 2019 at 14:57, on Zulip):

great

centril (Sep 05 2019 at 15:01, on Zulip):

( https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3ANLL-fixed-by-NLL+label%3A%22I-unsound+%F0%9F%92%A5%22 )

centril (Sep 05 2019 at 15:02, on Zulip):

went ahead and nominated https://github.com/rust-lang/rust/pull/63565 for t-lang

simulacrum (Sep 05 2019 at 15:05, on Zulip):

one additional idea is to make this an error in nightly once we have fixes for all the root deps (but not let it ride into stable)

Matthew Jasper (Sep 05 2019 at 15:07, on Zulip):

#31567 was closed, but is still relevant here: it's the url issue.

centril (Sep 05 2019 at 15:08, on Zulip):

thanks

Matthew Jasper (Sep 05 2019 at 15:13, on Zulip):

#38899 and #31567 both look like they could be emulated in the MIR borrow checker. But #27868 requires a borrow checker using a different CFG to reliable implement.

simulacrum (Sep 05 2019 at 15:20, on Zulip):

One potential problem is that it loosely sounds like at least rusttype maintainers don't really want to release a patch release (https://gitlab.redox-os.org/redox-os/rusttype/issues/138#note_18122)

centril (Sep 05 2019 at 15:22, on Zulip):

@simulacrum you might want to clarify that "we" == the compiler devs

centril (Sep 05 2019 at 15:23, on Zulip):

the comment might be interpreted as a random user wanting this for their own project

simulacrum (Sep 05 2019 at 15:23, on Zulip):

good point

centril (Sep 05 2019 at 15:28, on Zulip):

Sure if you insist, I've added a v0.2 branch.

Success! :tada:

simulacrum (Sep 05 2019 at 15:29, on Zulip):

Now I just need to figure out pushing to gitlab

simulacrum (Sep 05 2019 at 15:39, on Zulip):

(well, and actually fix this code. I admit I don't fully understand how to do so)

simulacrum (Sep 05 2019 at 15:56, on Zulip):

@centril one item for lang team, potentially, is some of the new NLL breakage could be made into a lint

simulacrum (Sep 05 2019 at 15:56, on Zulip):

specifically the assignment into field of moved value

simulacrum (Sep 05 2019 at 15:57, on Zulip):

Felix even writes that this is entirely sound

centril (Sep 05 2019 at 15:57, on Zulip):

@simulacrum link?

simulacrum (Sep 05 2019 at 15:57, on Zulip):

to Felix or ?

centril (Sep 05 2019 at 15:57, on Zulip):

the issue/Felix's comment

simulacrum (Sep 05 2019 at 15:58, on Zulip):

http://blog.pnkfx.org/blog/2019/06/26/breaking-news-non-lexical-lifetimes-arrives-for-everyone/#Outlawed.partial.assignment.to.moved.values..for.now.

centril (Sep 05 2019 at 16:00, on Zulip):

lemme try to recreate that one in the playground

simulacrum (Sep 05 2019 at 16:02, on Zulip):

I didn't see too many of these in practice though fwiw

simulacrum (Sep 05 2019 at 16:02, on Zulip):

https://gist.github.com/Mark-Simulacrum/ff49698b6bffc5a40bf9c20ec255d619 @nikomatsakis

simulacrum (Sep 05 2019 at 16:13, on Zulip):

@Pietro Albini so it looks like maybe crater is not properly updating github repos? e.g. https://crater-reports.s3.amazonaws.com/pr-63565/try%2375eff020d0923c035c2fe220db4a0465cd847048/gh/demostf.parser/log.txt checks out a commit from May 25th, and there are more recent commits

Pietro Albini (Sep 05 2019 at 16:37, on Zulip):

@simulacrum can you open an issue on crater and assign me to it?

simulacrum (Sep 05 2019 at 16:37, on Zulip):

yes

centril (Sep 05 2019 at 16:38, on Zulip):

A simplified version is https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=f70a27a3aefa97415df9dfd416bc3e68

centril (Sep 05 2019 at 16:39, on Zulip):

we did decide previously on a language team meeting to ban this

centril (Sep 05 2019 at 16:40, on Zulip):

and they were few in practice.

simulacrum (Sep 05 2019 at 16:40, on Zulip):

yeah, I was just thinking if we want to do things piecemeal that might be a good candidate

simulacrum (Sep 05 2019 at 16:41, on Zulip):

but I agree there's probably not much point

centril (Sep 05 2019 at 16:41, on Zulip):

@simulacrum ah; but if we do it piecemeal then we should specifically turn that one into an error rather than a warning?

centril (Sep 05 2019 at 16:42, on Zulip):

(it's already a warning that ignores cap-lints)

simulacrum (Sep 05 2019 at 16:42, on Zulip):

well, I think that's a bit up to us? Like, presumably we can say "for now, we'll not turn on this bit of error"

simulacrum (Sep 05 2019 at 16:42, on Zulip):

(whereas perhaps other bits are turned on)

centril (Sep 05 2019 at 16:43, on Zulip):

but if it occurs less frequently then surely that's a reason to move ahead earlier with this bit?

simulacrum (Sep 05 2019 at 16:45, on Zulip):

hm, I guess I could see that argument too. I was thinking along the lines of "don't do a bunch of small things that aren't unsound" -> "land this sooner"

simulacrum (Sep 05 2019 at 16:45, on Zulip):

but if we're targeting 1.40 then I realized that's actually like ~3 weeks away or so realistically from compiler team's perspective

simulacrum (Sep 05 2019 at 16:45, on Zulip):

(since we're in 1.39 on nightly right now)

simulacrum (Sep 05 2019 at 16:49, on Zulip):

rusttype 0.2.4 is out so only nalgebra as other major root remains

simulacrum (Sep 05 2019 at 16:54, on Zulip):

updated gist

nikomatsakis (Sep 05 2019 at 17:31, on Zulip):

thanks @simulacrum !

centril (Sep 06 2019 at 14:55, on Zulip):

@Matthew Jasper converted https://github.com/rust-lang/rust/pull/63565 into the PR for 2018 and then https://github.com/rust-lang/rust/pull/64221 for 2015 one version later

simulacrum (Sep 06 2019 at 15:34, on Zulip):

FWIW scope of impact may have been slightly misjudged due to (https://github.com/rust-lang/rustwide/commit/f83bedda664eaebaf3801471043a4c96246795ad) which affected unknown number of crates

simulacrum (Sep 06 2019 at 15:35, on Zulip):

so we may want to rerun crater on 2015 once that commit is deployed

simulacrum (Sep 06 2019 at 15:35, on Zulip):

(per our plans anyway, but just wanted to raise this here as well)

centril (Sep 06 2019 at 17:17, on Zulip):

@simulacrum ugh; can I trust the results in https://github.com/rust-lang/rust/pull/63247# ?

simulacrum (Sep 06 2019 at 17:18, on Zulip):

kind of, they're just maybe a bit outdated

simulacrum (Sep 06 2019 at 17:18, on Zulip):

which is in some sense always true :)

RalfJ (Sep 16 2019 at 10:51, on Zulip):

#38899 and #31567 both look like they could be emulated in the MIR borrow checker. But #27868 requires a borrow checker using a different CFG to reliable implement.

I (and many others) would be really sad if we cannot remove the AST borrow checker for good eventually.
It's not just that code itself, also things like e.g. static promotion of temporaries are currently duplicated because of our two borrow checkers.

pnkfelix (Sep 16 2019 at 11:26, on Zulip):

I don't understand; we have moved to NLL by default everywhere. isn't #27868 effectively resolved by that?

pnkfelix (Sep 16 2019 at 11:27, on Zulip):

and now we've gone from migrate mode to hard errors in 2018 edition. @centril has stated he intends for the same to happen for 2015 edition in the next release cycle, and after that we will remove AST borrow-check. I guess I don't see why we are worrying about #27868

pnkfelix (Sep 16 2019 at 11:28, on Zulip):

or .. that's what I thought. What version of nightly is the playground using ...

pnkfelix (Sep 16 2019 at 11:28, on Zulip):

ah, nightly from august 23rd, still

pnkfelix (Sep 16 2019 at 11:28, on Zulip):

I should go figure out what is blocking an upgrade there.

lqd (Sep 16 2019 at 11:32, on Zulip):

I should go figure out what is blocking an upgrade there.

(simulacrum asked Jake Goulding on discord infra about this, yesterday I think - no answer yet)

Matthew Jasper (Sep 16 2019 at 11:54, on Zulip):

My y comment was in response to a "can [we] allow those holes on 2015 inside the MIR borrowck" comment on discord.

RalfJ (Sep 16 2019 at 11:58, on Zulip):

My y comment was in response to a "can [we] allow those holes on 2015 inside the MIR borrowck" comment on discord.

oh I see. well that would still be sad but less sad than keeping AST borrowck around.^^

pnkfelix (Sep 16 2019 at 12:14, on Zulip):

okay now I've re-read more of this topic and see what @Matthew Jasper was responding to

centril (Sep 16 2019 at 12:22, on Zulip):

@RalfJ Not to worry; in ~10 days NLL migrate mode warnings are gone; in a few days after that AST borrowck is history.

simulacrum (Sep 16 2019 at 12:23, on Zulip):

we're scheduled to fully remove ast borrowck in 1.40, which'll be on master in 2 weeks or so

centril (Sep 16 2019 at 12:24, on Zulip):

@simulacrum yea, 26th is release, +10 days from now

RalfJ (Sep 16 2019 at 12:46, on Zulip):

RalfJ Not to worry; in ~10 days NLL migrate mode warnings are gone; in a few days after that AST borrowck is history.

<3

centril (Sep 25 2019 at 15:20, on Zulip):

@Matthew Jasper https://github.com/rust-lang/rust/pull/64221 is ready for review now that master is at 1.40

Matthew Jasper (Sep 25 2019 at 18:55, on Zulip):

@centril Are we just accepting the regressions caused by this? How many (latest versions of) crates.io crates does this affect now?

simulacrum (Sep 25 2019 at 18:56, on Zulip):

I believe based on the last crater run we have very few, or at least mostly non-core crate (i.e., few reverse deps) crates.io regressions

simulacrum (Sep 25 2019 at 18:58, on Zulip):

I can try and dig in tonight probably -- however, I suspect it might not be worth it. Most of the crates which care have issued fixes and I filed a few PRs that got accepted and released for a few other of the central crates (rusttype, IIRC, nalgebra maybe... possibly a few more)

centril (Sep 25 2019 at 18:58, on Zulip):

@Matthew Jasper yes, the consensus of the lang team meeting was that we feel comfortable doing so in 1.40.

centril (Sep 25 2019 at 18:59, on Zulip):

(special blog post and such coming later...)

Matthew Jasper (Sep 25 2019 at 18:59, on Zulip):

I just don't want to start ripping out AST borrowck code and then have to beta backport a revert.

simulacrum (Sep 25 2019 at 19:00, on Zulip):

ah, to be clear, there is no beta backport planned here

Matthew Jasper (Sep 25 2019 at 19:01, on Zulip):

I mean to the 1.40 beta

simulacrum (Sep 25 2019 at 19:03, on Zulip):

you mean if we make a patch now and then want to backport that to 1.39?

simulacrum (Sep 25 2019 at 19:03, on Zulip):

that seems unlikely -- I can't imagine us wanting to make fixes to ast borrowck in beta

Matthew Jasper (Sep 25 2019 at 19:03, on Zulip):

No, I mean, we branch 1.40 and then change our minds.

Matthew Jasper (Sep 25 2019 at 19:04, on Zulip):

due to, e.g., concern over regressions.

centril (Sep 25 2019 at 19:11, on Zulip):

@Matthew Jasper This is permanent, we're ripping out AST borrowck for good

simulacrum (Sep 25 2019 at 19:12, on Zulip):

ah, sure -- yeah, I don't think that's going to happen personally

simulacrum (Sep 25 2019 at 19:13, on Zulip):

like everyone is pretty aware of the breakage here (lang, compiler)

simulacrum (Sep 25 2019 at 19:13, on Zulip):

We can make another crater run and I can try to squash some more regressions though I guess over the next ~6 weeks

Matthew Jasper (Sep 25 2019 at 19:13, on Zulip):

1.40 beta will get one anyway

centril (Sep 25 2019 at 19:13, on Zulip):

yea; and we already have the old one

centril (Sep 25 2019 at 19:14, on Zulip):

I think some regressions will be fixed with time, and some are just abandonware

simulacrum (Sep 25 2019 at 19:14, on Zulip):

sure, yeah, I'm just saying that if we want to keep "track" of progress -- I think a new run would have like 10x less regressions, but I could of course be wrong

centril (Sep 25 2019 at 19:14, on Zulip):

I doubt we'll see that much movement so soon

centril (Sep 25 2019 at 19:15, on Zulip):

more likely in 1.40 beta because of time

Matthew Jasper (Sep 25 2019 at 19:16, on Zulip):

The crates.io regressions are the only ones that I'm concerned about.

Matthew Jasper (Sep 25 2019 at 19:17, on Zulip):

beta crater runs also have the advantage of not counting crates that require nightly

simulacrum (Sep 25 2019 at 19:17, on Zulip):

Sure -- I meant that the ~3 patches or so that I landed should've removed quite a lot of regressions

centril (Sep 25 2019 at 19:19, on Zulip):

@simulacrum nice work on that.

-- I think we (lang team) feel overall comfortable with moving ahead and I believe that's "the feeling of the room" here also?

lqd (Sep 25 2019 at 19:19, on Zulip):

did someone notice changes of the stats from the july run where eg 50% of the crates.io regressions were >1y old (I had sent you this centril, not sure if you've seen), or was it relatively similar in the last run ?

simulacrum (Sep 25 2019 at 19:20, on Zulip):

we didn't get stats like that this run, though we could go back and collect them

centril (Sep 25 2019 at 19:20, on Zulip):

@lqd well we added a bunch of crates from github

lqd (Sep 25 2019 at 19:22, on Zulip):

ok, one wouldn't expect completely inactive crates, or ones with no releases in a year to suddenly all become active while also ignoring the lints

simulacrum (Sep 25 2019 at 19:24, on Zulip):

sure, yes -- a lot of them are not themselves broken but had a dep that was I think

simulacrum (Sep 25 2019 at 19:24, on Zulip):

or that was my impression anyway

centril (Sep 25 2019 at 19:25, on Zulip):

@Matthew Jasper

File should be deleted

Only the .stderr file?

lqd (Sep 25 2019 at 19:25, on Zulip):

same for me

centril (Sep 25 2019 at 19:26, on Zulip):

(or also .rs?)

Matthew Jasper (Sep 25 2019 at 19:26, on Zulip):

Just the .migrate.stderr

centril (Sep 25 2019 at 19:59, on Zulip):

@Matthew Jasper the scribble file has:

// ignore-compare-mode-nll
// ignore-compare-mode-polonius

if I add // ignore-compare-mode-migrate, won't the .rs file do nothing?

Matthew Jasper (Sep 25 2019 at 19:59, on Zulip):

There isn't a compare-mode migrate. The .migrate.stderr file came from when the test used revisions

centril (Sep 25 2019 at 20:00, on Zulip):

@Matthew Jasper ah; you're saying I can just delete the file and --bless won't reappear it?

Matthew Jasper (Sep 25 2019 at 20:00, on Zulip):

Yes

centril (Sep 25 2019 at 20:00, on Zulip):

ah, got it

centril (Sep 25 2019 at 20:00, on Zulip):

will do

centril (Sep 25 2019 at 20:38, on Zulip):

Is anyone working on ripping out AST borrowck atm? I'll start with it otherwise

Matthew Jasper (Sep 25 2019 at 20:39, on Zulip):

I'm not yet. I have too much in-flight already.

centril (Sep 25 2019 at 20:39, on Zulip):

Aight; I'll give it a go

lqd (Sep 25 2019 at 20:42, on Zulip):

maybe Niko or Felix wanted to do it :)

Tshepang Lekhonkhobe (Sep 25 2019 at 20:46, on Zulip):

sounds lyk veri satisfying work

centril (Sep 25 2019 at 21:02, on Zulip):

indeed :slight_smile:

centril (Sep 26 2019 at 05:53, on Zulip):

@Matthew Jasper https://github.com/rust-lang/rust/pull/64221#issuecomment-535348513 -- had to bump iron & webrender in cargotest, can you have a look again?

centril (Sep 26 2019 at 05:53, on Zulip):

(they were depending on an old version of url)

centril (Sep 26 2019 at 14:58, on Zulip):

@pnkfelix btw, https://github.com/rust-lang/rust/pull/64790#discussion_r328502898 probably belongs to https://github.com/rust-lang/rust/pull/64221

centril (Sep 26 2019 at 15:07, on Zulip):

@pnkfelix so I should remove // ignore-compare-mode-nll from that PR then ostensibly

pnkfelix (Sep 26 2019 at 15:09, on Zulip):

Yes exactly, I believe.

centril (Sep 26 2019 at 15:10, on Zulip):

will do

pnkfelix (Sep 26 2019 at 15:11, on Zulip):

It won't matter, of course, unless/until Polonius starts using compare-mode itself.

pnkfelix (Sep 26 2019 at 15:11, on Zulip):

but why make things hard for them.

centril (Sep 26 2019 at 15:11, on Zulip):

Yea, good point

centril (Sep 26 2019 at 15:20, on Zulip):

...and fixed

Last update: Nov 21 2019 at 13:05UTC