inkwell icon indicating copy to clipboard operation
inkwell copied to clipboard

Types derived from ContextRef are always shortlived

Open hatzka-nezumi opened this issue 5 years ago • 6 comments

It is not possible to write a function which takes something with a get_context as a parameter, use that context to create something else, and return that something else. get_context calls return ContextRefs rather than regular references, and Rust considers the ContextRef a temporary, so (I'm pretty sure) the lifetime on whatever you create is inferred to be the lifetime of the ContextRef.

use inkwell::{
    module::Module,
    types::BasicTypeEnum,
};

pub trait RuntimeModule<'c> {
    fn some_ty(&self) -> BasicTypeEnum<'c>;
}

impl<'c> RuntimeModule<'c> for Module<'c> {
    fn some_ty(&self) -> BasicTypeEnum<'c> {
        self.get_context().struct_type(&[
            // ...
        ], false).into()
    }
}

LLVM Version:

  • LLVM Version: 8.0.1, though I doubt it's relevant
  • Inkwell Branch Used: llvm8-0

Desktop:

  • OS: macOS 10.14.6

hatzka-nezumi avatar Apr 05 '20 00:04 hatzka-nezumi

You're correct. This is because context methods derive 'ctx from &'ctx self. This works perfectly for actual Context objects but less so for ContextRef.

I wonder if we should just get rid of ContextRef since it's sort of a historical artifact anyway... But it was a nice convenience type.

TLDR: Return types from Context methods (ie IntValue<'ctx>) only live as long as their parent &'ctx Context. ContextRef is always shortlived, and therefore anything derived from it is incredibly useless.

TheDan64 avatar Apr 06 '20 22:04 TheDan64

I think there are two options here, both of which require getting ride of the deref impl because it causes this problem.

Either A) We add our own "deref" method which lies about returning a &'ctx Context. I think this might be safe, but I'm not 100% sure. B) We add a method that takes a closure which lets you access the &'ctx Context. Ie Fn(&'ctx Context) -> T.

Obviously, A) is more ergonomic. But it boils down to safety.

TheDan64 avatar Apr 19 '20 12:04 TheDan64

I suspect A) is safe because:

  1. ContextRef<'ctx> cannot outlive the original Context ('ctx), which means the fake &'ctx Context living in the CR also cannot outlive the original Context.
  2. Even though the fake &'ctx Context's actual lifetime is &'ctx_ref (implicit on self) and there's a gap in the range of ['ctx_ref, 'ctx] because 'ctx: 'ctx_ref, borrowing &'ctx Context from the CR will increase the lifetime of 'ctx_ref towards 'ctx (but never past it due to 1.) I think?

That last part is the crux of whether or not this approach is safe. 1) is certainly true. 2) is iffy

TheDan64 avatar Apr 19 '20 13:04 TheDan64

I think this is related. I've been looking into using this library for a small pet project that would effectively be a JIT usable from a CFFI. In trying to create an abstraction over the types in this crate (i.e. some Compiler type that can live arbitrarily long, and deal with all the inner workings of the JIT), the fact that Context is borrowed by all the other major types (Module, Builder, etc.) has been getting in my way. I don't think what I'm trying is actually possible with safe code. Would it be possible to use Arcs to reference count the types at runtime rather than borrows?

rjb3977 avatar Jan 11 '21 17:01 rjb3977

Would it be possible to use Arcs to reference count the types at runtime rather than borrows?

If you're asking if it's possible for inkwell to do that, it's certainly technically possible. But I don't consider it to be the right approach since we'd lose many guarantees provided by the lifetimes.

If you're asking if it's possible to wrap types like Context in an Arc, it might be - but that won't just get rid of borrows magically. Lifetime rules still apply

TheDan64 avatar Jan 14 '21 02:01 TheDan64

You maybe want something like the rental crate, which allows for self-referential structs by doing the unsafe bits for you. Though it's worth noting that rental is no longer maintained I think. There might be more modern alternatives out there.

TheDan64 avatar Jan 14 '21 02:01 TheDan64