Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo_compile: iterate packages once, not three times #4494

Merged
merged 2 commits into from
Sep 18, 2017

Conversation

RalfJung
Copy link
Member

I forgot to push this into #4492

r? @matklad

@matklad
Copy link
Member

matklad commented Sep 14, 2017

What do you think about using map/collect here? That way, we can avoid marking the vector as mut, and this is useful, because the future reader would not have to wonder if the variable is mutated down the line.

@RalfJung
Copy link
Member Author

I tried, but ended up getting lifetime errors about packages not living long enough or so.

    let to_builds = specs.iter()
        .map(|p| {
            p.query(resolve_with_overrides.iter()).and_then(|p| {
                packages.get(p).and_then(|p| {
                    p.manifest().print_teapot(ws.config());
                    Ok(p)
                })
            })
        })
        .collect::<CargoResult<Vec<_>>>()?;
error[E0597]: `**packages` does not live long enough
   --> src/cargo/ops/cargo_compile.rs:249:17
    |
248 |             p.query(resolve_with_overrides.iter()).and_then(|p| {
    |                                                             --- capture occurs here
249 |                 packages.get(p).and_then(|p| {
    |                 ^^^^^^^^ does not live long enough
...
255 |         .collect::<CargoResult<Vec<_>>>()?;
    |                                         - borrowed value only lives until here
...
354 | }
    | - borrowed value needs to live until here

@matklad
Copy link
Member

matklad commented Sep 17, 2017

The following seems to work for me:

    let to_builds = specs.iter().map(|p| {
        let pkgid = p.query(resolve_with_overrides.iter())?;
        let p = packages.get(pkgid)?;
        p.manifest().print_teapot(ws.config());
        Ok(p)
    }).collect::<CargoResult<Vec<_>>>()?;

@RalfJung
Copy link
Member Author

Wow, I did not know that one could use the ? inside the map closure.

@matklad
Copy link
Member

matklad commented Sep 18, 2017

Wow, I did not know that one could use the ? inside the map closure.

It's ok as long as compiler is able to figure out the types.

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Sep 18, 2017

📌 Commit f490f16 has been approved by matklad

@bors
Copy link
Contributor

bors commented Sep 18, 2017

⌛ Testing commit f490f16 with merge 525323f...

bors added a commit that referenced this pull request Sep 18, 2017
cargo_compile: iterate packages once, not three times

I forgot to push this into <#4492>

r? @matklad
@bors
Copy link
Contributor

bors commented Sep 18, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing 525323f to master...

@bors bors merged commit f490f16 into rust-lang:master Sep 18, 2017
@RalfJung RalfJung deleted the virtual branch January 23, 2021 12:42
@ehuss ehuss added this to the 1.22.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants