Stream: t-compiler/wg-nll

Topic: #53123 "enduring" borrow


nikomatsakis (Aug 08 2018 at 17:41, on Zulip):

I'm creating a topic thread for #53123. @Sunjay Varma may take a look.

nikomatsakis (Aug 08 2018 at 17:42, on Zulip):

I can assign the issue to you @Sunjay Varma, if you want to "commit" (at least for now..)

Sunjay Varma (Aug 08 2018 at 17:45, on Zulip):

Hello! Hopefully I am using Zulip right

Sunjay Varma (Aug 08 2018 at 18:16, on Zulip):

@nikomatsakis Ok. So do you want to play around with candidate_should_be_dropped_in_favor_of or look at the alternate solution?

nikomatsakis (Aug 08 2018 at 18:17, on Zulip):

I think I would prefer to start by playing around with candidate_should_be_dropped_in_favor_of

nikomatsakis (Aug 08 2018 at 18:17, on Zulip):

the NLL hack is sort of a "fallback"

Sunjay Varma (Aug 08 2018 at 18:22, on Zulip):

It's my first time in the borrowing/lifetimes code. Can you give me a bit of an overview? It would be helpful to understand candidate_should_be_dropped_in_favor_of

nikomatsakis (Aug 08 2018 at 18:22, on Zulip):

well that code is really not concerned with borrowing or lietimes

nikomatsakis (Aug 08 2018 at 18:22, on Zulip):

it is in the trait solver

nikomatsakis (Aug 08 2018 at 18:22, on Zulip):

Ye Olde Crappy Trait solver, that is

nikomatsakis (Aug 08 2018 at 18:22, on Zulip):

rather than the New And Shiny Chalk ;)

nikomatsakis (Aug 08 2018 at 18:22, on Zulip):

there is actually a write-up of how it works in the rustc guide I think

nikomatsakis (Aug 08 2018 at 18:22, on Zulip):

I wonder how up to date it is

nikomatsakis (Aug 08 2018 at 18:23, on Zulip):

the write-up is in the Trait Solving (Old Style) chapter

nikomatsakis (Aug 08 2018 at 18:23, on Zulip):

in particular, the section on winnowing

Sunjay Varma (Aug 08 2018 at 18:24, on Zulip):

/me reading

Sunjay Varma (Aug 08 2018 at 18:26, on Zulip):

During type checking, we do not store the results of trait selection. We simply wish to verify that trait selection will succeed. Then later, at trans time, when we have all concrete types available, we can repeat the trait selection to choose an actual implementation, which will then be generated in the output binary.

Sunjay Varma (Aug 08 2018 at 18:26, on Zulip):

Is there a downside to storing the selected trait?

Sunjay Varma (Aug 08 2018 at 18:26, on Zulip):

Why search for it twice?

Sunjay Varma (Aug 08 2018 at 18:26, on Zulip):

Are there some impls that can only be known at trans time?

nikomatsakis (Aug 08 2018 at 18:26, on Zulip):

complexity, for one thing

nikomatsakis (Aug 08 2018 at 18:26, on Zulip):

but also it doesn't play so well with specialization

Sunjay Varma (Aug 08 2018 at 18:27, on Zulip):

Ah okay makes sense

Sunjay Varma (Aug 08 2018 at 18:27, on Zulip):

Thanks

nikomatsakis (Aug 08 2018 at 18:27, on Zulip):

in particular if we are checking a generic fn, we may find a generic impl, but once we know the concrete types, we can find a more narrow one

Sunjay Varma (Aug 09 2018 at 07:02, on Zulip):

@nikomatsakis Ok so I created a PR which adds the test case from the issue as a new run-pass test. As expected, the test currently fails. https://github.com/rust-lang/rust/pull/53212

Sunjay Varma (Aug 09 2018 at 07:04, on Zulip):

I printed out the arguments to candidate_should_be_dropped_in_favor_of and got the following:

victim = EvaluatedCandidate {
    candidate: BuiltinCandidate {
        has_nested: false
    },
    evaluation: EvaluatedToOk
}
other = EvaluatedCandidate {
    candidate: ParamCandidate(
        Binder(
            <&'a mut T as std::marker::Sized>
        )
    ),
    evaluation: EvaluatedToOk
}
...
victim = EvaluatedCandidate {
    candidate: BuiltinCandidate {
        has_nested: false
    },
    evaluation: EvaluatedToOk
}
other = EvaluatedCandidate {
    candidate: ParamCandidate(
        Binder(
            <&mut T as std::marker::Sized>
        )
    ),
    evaluation: EvaluatedToOk
}
Sunjay Varma (Aug 09 2018 at 07:04, on Zulip):

Tried out a few things here and there, but I'm not quite sure I completely understand the fix you're suggesting in the issue.

Sunjay Varma (Aug 09 2018 at 07:05, on Zulip):

It is taking me ridiculously long to make any progress because rustc takes a ridiculous amount of time to recompile every time I make even a tiny change (like adding logging)

Sunjay Varma (Aug 09 2018 at 07:06, on Zulip):
./x.py build -i --stage 1 --keep-stage 0 && RUST_LOG=rustc::traits ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/run-pass/nll/issue-53123-raw-pointer-cast.rs
Sunjay Varma (Aug 09 2018 at 07:06, on Zulip):

This is the entire command I'm running. Let me know if you have any tips :)

nikomatsakis (Aug 09 2018 at 10:00, on Zulip):

hmm the command I usually do is this:

./x.py build -i --stage 1 --keep-stage 1 std/libstd

and I find that adding logging or other small deltas are usually not that slow

nikomatsakis (Aug 09 2018 at 10:01, on Zulip):

the change I was proposing to fix the issue would be to modify the candidate_should_be_dropped_in_favor_of so that it prefers a BuiltinCandidate -- specifically, a BuiltinCandidate with has_nested: false -- to any other

nikomatsakis (Aug 09 2018 at 10:01, on Zulip):

the has_nested: false means that there are no further obligations

nikomatsakis (Aug 09 2018 at 10:02, on Zulip):

I think all of our built-in candidates are very general

nikomatsakis (Aug 09 2018 at 10:02, on Zulip):

so this would be the most accepting thing we can do

nikomatsakis (Aug 09 2018 at 10:23, on Zulip):

another pattern that I use a lot @Sunjay Varma is to do ./x.py check all the time until I am ready to test

nikomatsakis (Aug 09 2018 at 10:23, on Zulip):

I guess that won't help here

Sunjay Varma (Aug 09 2018 at 19:42, on Zulip):

If I pass src/libstd to the build command, won't it only build the standard library? I need it to build the trait solver

Sunjay Varma (Aug 09 2018 at 19:43, on Zulip):

Also, if you --keep-stage 1, won't it not build stage 1 at all?

simulacrum (Aug 09 2018 at 19:44, on Zulip):

Right, that command will rebuild the stage 0 compiler but skip rebuilding stage 1 std

simulacrum (Aug 09 2018 at 19:44, on Zulip):

which gives you a mostly fully functioning compiler

Sunjay Varma (Aug 09 2018 at 19:44, on Zulip):

I guess the docs here are a bit misleading then: https://rust-lang-nursery.github.io/rustc-guide/how-to-build-and-run.html#running-xpy-and-building-a-stage1-compiler

Sunjay Varma (Aug 09 2018 at 19:44, on Zulip):

Stage 0 is built from my local checkout?

Sunjay Varma (Aug 09 2018 at 19:44, on Zulip):

From those docs:

Stage 0: the stage0 compiler can be your existing (perhaps older version of) Rust compiler, the current beta compiler or you may download the binary from the internet.

simulacrum (Aug 09 2018 at 19:48, on Zulip):

Yes stage 0 compiler artifacts (stage1/bin/rustc)

Sunjay Varma (Aug 09 2018 at 19:49, on Zulip):

Ohhh. The result of stage 0 is stage 1. That makes sense actually. Thanks!

Sunjay Varma (Aug 09 2018 at 19:50, on Zulip):

So is the command I want to use ./x.py build -i --stage 1 --keep-stage 0 src/librustc?

Sunjay Varma (Aug 09 2018 at 19:50, on Zulip):

I tried --keep-stage 1 but I got some warnings and panics

Sunjay Varma (Aug 09 2018 at 19:50, on Zulip):
Updating only changed submodules
Submodules updated in 0.04 seconds
    Finished dev [unoptimized] target(s) in 0.19s
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.21s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage0 test artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.18s
Copying stage0 test from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage0 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.23s
Copying stage0 rustc from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage0 codegen artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu, llvm)
    Finished release [optimized] target(s) in 0.19s
Assembling stage1 compiler (x86_64-unknown-linux-gnu)
Warning: Using a potentially old libstd. This may not behave well.
Copying stage1 std from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
thread 'main' panicked at 'File::open(stamp) failed with No such file or directory (os error 2)', bootstrap/lib.rs:1109:12
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failed to run: /home/sunjay/Documents/projects/rust/build/bootstrap/debug/bootstrap build -i --stage 1 --keep-stage 1 src/librustc
simulacrum (Aug 09 2018 at 19:58, on Zulip):

Yeah so you need to build once without it

simulacrum (Aug 09 2018 at 19:58, on Zulip):

Then add it

nikomatsakis (Aug 09 2018 at 20:13, on Zulip):

@Sunjay Varma I usually use --keep-stage 1 personally -- if I get problems, I take it away for one cycle :)

nikomatsakis (Aug 09 2018 at 20:13, on Zulip):

but I don't really know what --keep-stage 0 does :)

Sunjay Varma (Aug 09 2018 at 20:15, on Zulip):

Currently building stage 1. I'll try it with --keep-stage 1 after that :)

Sunjay Varma (Aug 09 2018 at 20:15, on Zulip):

Thanks for both of your help!

Sunjay Varma (Aug 09 2018 at 20:23, on Zulip):

./x.py build -i --stage 1 --keep-stage 1 src/librustc compiles fast!

nikomatsakis (Aug 09 2018 at 20:23, on Zulip):

yes, yes it does.

simulacrum (Aug 09 2018 at 20:26, on Zulip):

Though you probably want src/libstd as skipping librustc/compiling it is useless

Sunjay Varma (Aug 09 2018 at 20:27, on Zulip):

pasted image

Sunjay Varma (Aug 09 2018 at 20:27, on Zulip):

Isn't the path what you are asking it to compile?

Sunjay Varma (Aug 09 2018 at 20:27, on Zulip):

It won't skip librustc

simulacrum (Aug 09 2018 at 20:28, on Zulip):

src/libstd is what you'd want

Sunjay Varma (Aug 09 2018 at 20:31, on Zulip):

Sorry, I'm trying to understand what you're recommending. The file I'm editing is src/librustc/traits/select.rs. Why pass src/libstd instead of src/librustc?

nikomatsakis (Aug 09 2018 at 20:34, on Zulip):

@Sunjay Varma the point is that there is a sequence of things to build:

nikomatsakis (Aug 09 2018 at 20:34, on Zulip):

so when you say --stage 1 src/libstd, you build everything up to there-- which includes rustc built by the stage0 compiler

Sunjay Varma (Aug 09 2018 at 20:37, on Zulip):

Oh! That makes sense! I wish that was explained in the rustc-guide (if it is, we should move it to the section about building rustc)

Sunjay Varma (Aug 09 2018 at 20:37, on Zulip):

Thanks!

nikomatsakis (Aug 09 2018 at 20:37, on Zulip):

I thought it was :)

nikomatsakis (Aug 09 2018 at 20:37, on Zulip):

but maybe not very well

nikomatsakis (Aug 09 2018 at 20:38, on Zulip):

that is, in the section you pointed at

nikomatsakis (Aug 09 2018 at 20:38, on Zulip):

it says:

What this command will do is the following:

nikomatsakis (Aug 09 2018 at 20:39, on Zulip):

but clearly could be clearer

nikomatsakis (Aug 09 2018 at 20:39, on Zulip):

also that section prob needs updating for --keep-stage 1...

Sunjay Varma (Aug 09 2018 at 21:10, on Zulip):

@nikomatsakis The PR is updated: https://github.com/rust-lang/rust/pull/53212

Sunjay Varma (Aug 09 2018 at 21:10, on Zulip):

See my comment in the PR for some information about what I did

Sunjay Varma (Aug 09 2018 at 21:10, on Zulip):

I basically implemented exactly what you said and made it so that BuiltInCandidate { has_nested: false } is preferred in all cases

nikomatsakis (Aug 09 2018 at 22:21, on Zulip):

looks about right

nikomatsakis (Aug 09 2018 at 22:22, on Zulip):

although that fn is always kind of mind-bending

nikomatsakis (Aug 09 2018 at 22:22, on Zulip):

/me can't wait for chalk

Sunjay Varma (Aug 09 2018 at 22:53, on Zulip):

I :heart: chalk

Sunjay Varma (Aug 09 2018 at 22:54, on Zulip):

@nikomatsakis Tests have passed in the PR. Anything else to be done or shall we merge it?

nikomatsakis (Aug 09 2018 at 22:57, on Zulip):

no, that's probably fine

Sunjay Varma (Aug 09 2018 at 22:59, on Zulip):

Ok. Then I guess it's good to go: https://github.com/rust-lang/rust/pull/53212

Sunjay Varma (Aug 09 2018 at 22:59, on Zulip):

:smile:

Last update: Nov 21 2019 at 23:25UTC