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

Workaround rust nightly borrowcheck error (#1860) #1861

Merged

Conversation

mitchmindtree
Copy link
Contributor

Surprisingly, this fixes the error filed at #1860!

This seems suspicious, perhaps indicative of a bug in Rust's non-lexical
lifetime handling on nightly?

The lifetimes in the handlebars::Renderable::render method signature
are quite complicated, and its unclear to me whether or not Rust is
catching some new safety edge-case that wasn't previously handled
correctly...

Possibly related to drop order, which I think is related to the
order of binding statements?

Surprisingly, this fixes the error filed at rust-lang#1860!

This seems suspicious, perhaps indicative of a bug in Rust's non-lexical
lifetime handling?

The lifetimes in the `handlebars::Renderable::render` method signature
are quite complicated, and its unclear to me whether or not Rust is
catching some new safety edge-case that wasn't previously handled
correctly...

Possibly related to `drop` order, which I *think* is related to the
order of binding statements?
@andrewdavidmackenzie
Copy link
Contributor

Do you plan to release a v0.4.21 on crates.io?

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for the quick fix!

Yea, this fix looks correct. I don't think it is an issue with the borrow checker. What happens is that the handlebars render function has this signature:

fn render<'reg: 'rc, 'rc>(
    &'reg self,
    registry: &'reg Registry<'reg>,
    context: &'rc Context,
    rc: &mut RenderContext<'reg, 'rc>,
    out: &mut dyn Output,
) -> Result<(), RenderError>;

Based on that signature, RenderContext is mutable and has a 'rc lifetime, which means in theory it could receive references from &'rc Context. If local_ctx is dropped before local_rc, then in theory local_rc could contain a dangling reference into something from local_ctx.

I don't think that happens. The RenderContext is mutable for other reasons.

I don't have time right now to do a release, but I will make one later today.

@ehuss ehuss merged commit 8f01d02 into rust-lang:master Jul 22, 2022
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request Sep 7, 2022
Removes an old patch that was once required after a Rust update revealed
some borrow-related soundness issues in mdbook.
rust-lang/mdBook#1861
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request Sep 8, 2022
Removes an old patch that was once required after a Rust update revealed
some borrow-related soundness issues in mdbook.
rust-lang/mdBook#1861
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request Sep 8, 2022
…2742)

Removes an old patch that was once required after a Rust update revealed
some borrow-related soundness issues in mdbook.
rust-lang/mdBook#1861
daviddrysdale added a commit to daviddrysdale/mdbook-footnote that referenced this pull request Mar 23, 2023
Add a CHANGELOG.md file.

Update the main mdbook dependency to something more recent, in particular
to ensure that rust-lang/mdBook#1861 is included.
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.

3 participants