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

Latest nightly (rustc 1.64.0-nightly (62b272d25 2022-07-21)) introduces a new borrowcheck error in previously compiling mdbook 0.4.20 #99591

Closed
mitchmindtree opened this issue Jul 22, 2022 · 7 comments
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another.

Comments

@mitchmindtree
Copy link
Contributor

As of rustc 1.64.0-nightly (62b272d25 2022-07-21), a new lifetime error has appeared when attempting to build mdbook 0.4.20.

Error

error[E0597]: `local_ctx` does not live long enough
   --> src/renderer/html_handlebars/helpers/navigation.rs:154:25
    |
154 |             t.render(r, &local_ctx, &mut local_rc, out)
    |                         ^^^^^^^^^^ borrowed value does not live long enough
155 |         })?;
    |         -
    |         |
    |         `local_ctx` dropped here while still borrowed
    |         borrow might be used here, when `local_rc` is dropped and runs the destructor for type `handlebars::RenderContext<'_, '_>`
    |
    = note: values in a scope are dropped in the opposite order they are defined

For more information about this error, try `rustc --explain E0597`.
error: could not compile `mdbook` due to previous error

Version it worked on

rustc 1.64.0-nightly (d68e7eb 2022-07-20)

Version with regression

rustc 1.64.0-nightly (62b272d 2022-07-21)

Details

You can find more details at the downstream mdbook issue rust-lang/mdBook#1860.

Curiously, the new error can be fixed by re-ordering some seemingly inocuous statements - see this PR rust-lang/mdBook#1861.

Perhaps this is indicative of an error in non-lexical-lifetime handling on the latest nightly? Or alternatively, this could be the result of fixing a soundness / safety issue related to the interop between lifetimes and drop order?

This error was originally discovered in an unrelated CI pass failing in the following PR: FuelLabs/sway#2359.

@mitchmindtree mitchmindtree added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jul 22, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 22, 2022
@5225225
Copy link
Contributor

5225225 commented Jul 22, 2022

Ran a bisection in mdBook, version rust-lang/mdBook@92afe9b

Looks to be a rollup, #99567. None of them jump out as obvious candidates.


searched nightlies: from nightly-2022-07-20 to nightly-2022-07-22
regressed nightly: nightly-2022-07-22
searched commit range: d68e7eb...62b272d
regressed commit: af7ab34

bisected with cargo-bisect-rustc v0.6.1

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start 2022-07-20 --end 2022-07-22 --preserve -- check 

@5225225
Copy link
Contributor

5225225 commented Jul 22, 2022

This might actually be #99413 (so, a soundness fix.)? A btreemap is used in that bit of code, but it's talking about a RenderContext.

@ehuss
Copy link
Contributor

ehuss commented Jul 22, 2022

Confirmed that it is caused by #99413. Here is a slightly smaller repro:

#![allow(unused)]
use std::collections::BTreeMap;
use std::marker::PhantomData;
use std::rc::Rc;

fn main() {
    let mut rc = RenderContext {
        local_helpers: BTreeMap::new(),
    };
    foo(&mut rc);
}

#[derive(Clone)]
struct RenderContext<'rc> {
    local_helpers: BTreeMap<String, Rc<dyn HelperDef + 'rc>>,
}

pub trait HelperDef {}

fn foo(rc: &mut RenderContext<'_>) {
    let mut local_rc = rc.clone();
    let local_ctx: BTreeMap<String, String> = BTreeMap::new();
    render(&local_ctx, &mut local_rc);
}

fn render<'rc>(ctx: &'rc BTreeMap<String, String>, rc: &mut RenderContext<'rc>) {}

I think the error here is correct and #99413 is a valid fix. It's just a consequence of how the lifetimes in the handlebars crate are defined that cause the error.

@danielhenrymantilla
Copy link
Contributor

Until the fix to mdbook is officially released, btw, for those running into this, the workaround is to use the git dev, potentially as a patch:

  • for those using cargo install mdbook, instead, do

    cargo install mdbook --git 'https://github.com/rust-lang/mdBook' --rev '8f01d0234f708042ea8d0dc5cac63859b3f14cb1'
  • for those depending on mdbook as a library, on the Cargo.toml file of your workspace (where the Cargo.lock file lies / is generated), add:

    [patch.crates-io.mdbook]
    version = "0.4.20"
    git = "https://github.com/rust-lang/mdBook"
    rev = "8f01d0234f708042ea8d0dc5cac63859b3f14cb1"

@Jules-Bertholet
Copy link
Contributor

@rustbot label -regression-untriaged +regression-from-stable-to-stable

@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed regression-untriaged Untriaged performance or correctness regression. labels Jun 10, 2023
@apiraino
Copy link
Contributor

@Jules-Bertholet changed the label to regression on stable: did you experience that it's still an issue? I've compiled mdbook (both on stable and nightly) and the reproducible at this comment now shows a better error, I believe.

@ehuss
Copy link
Contributor

ehuss commented Jun 21, 2023

I think this should be closed since it was an intentional soundness fix.

@apiraino apiraino closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2023
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another.
Projects
None yet
Development

No branches or pull requests

7 participants