Skip to content

Conversation

@ehuss
Copy link
Contributor

@ehuss ehuss commented Apr 30, 2019

Since the unit interner was added in #6867, I am getting a strange borrow check error compiling RLS with the latest Cargo (that is, linking rls with the new cargo). Adding explicit lifetimes to the Executor::init function seems to fix the problem. I don't 100% understand the error, because none of the relevant function or type signatures changed in that PR. The only thing that gives me a clue is this change to BuildPlan::add which does something similar, but that is not directly called by RLS.

The error looks like this:

error[E0623]: lifetime mismatch
   --> rls/src/build/cargo_plan.rs:149:24
    |
126 |         unit: &Unit<'_>,
    |                -------- these two types are declared with different lifetimes...
127 |         cx: &Context<'_, '_>,
    |              ---------------
...
149 |         let units = cx.dep_targets(unit);
    |                        ^^^^^^^^^^^ ...but data from `unit` flows into `cx` here

I generally don't like making changes if I don't understand them, but I'm a little stumped here how this is happening even though everything is defined with the same ('a) lifetime.

This unblocks updating RLS to the latest Cargo.

@rust-highfive
Copy link

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2019
@ehuss
Copy link
Contributor Author

ehuss commented Apr 30, 2019

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned nrc Apr 30, 2019
@alexcrichton
Copy link
Member

@bors: r+

Ah yeah this is expected in the sense that some methods on Context got more restrictive in terms of lifetime requirements, and dep_targets was one of them. This represents what we are actually handing executors and makes sense it's be required to call further methods

@bors
Copy link
Contributor

bors commented Apr 30, 2019

📌 Commit 1212c91 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 30, 2019
@bors
Copy link
Contributor

bors commented Apr 30, 2019

⌛ Testing commit 1212c91 with merge beb8fcb...

bors added a commit that referenced this pull request Apr 30, 2019
Add explicit lifetimes to Executor::init.

Since the unit interner was added in #6867, I am getting a strange borrow check error compiling RLS with the latest Cargo (that is, linking rls with the new cargo).  Adding explicit lifetimes to the `Executor::init` function seems to fix the problem.  I don't 100% understand the error, because none of the relevant function or type signatures changed in that PR.  The only thing that gives me a clue is [this change](https://github.com/rust-lang/cargo/pull/6867/files#diff-0bb62cb712c0811a17d7a726e068bf65L112) to `BuildPlan::add` which does something similar, but that is not directly called by RLS.

The error looks like this:
```
error[E0623]: lifetime mismatch
   --> rls/src/build/cargo_plan.rs:149:24
    |
126 |         unit: &Unit<'_>,
    |                -------- these two types are declared with different lifetimes...
127 |         cx: &Context<'_, '_>,
    |              ---------------
...
149 |         let units = cx.dep_targets(unit);
    |                        ^^^^^^^^^^^ ...but data from `unit` flows into `cx` here
```

I generally don't like making changes if I don't understand them, but I'm a little stumped here how this is happening even though everything is defined with the same (`'a`) lifetime.

This unblocks updating RLS to the latest Cargo.
@bors
Copy link
Contributor

bors commented May 1, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing beb8fcb to master...

@bors bors merged commit 1212c91 into rust-lang:master May 1, 2019
@ehuss ehuss added this to the 1.36.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants