Rough-and-ready context lifetime management.

[dabrahams]

This is a mostly-mechanical change; careful review is required to make sure it's being done right.

[kyouko-taiga]

So IIUC, the idea is to hold a reference to the context in every data structure that it could manage. I think that will work but probably we need to guarantee more lifetimes than just that of the context. It's very likely that we also need to guarantee the lifetime of the module in which we create functions. In fact, I suppose pretty much every class that has a public destructor in C++ (and therefore a corresponding XXXDispose wrapper in the C API) needs some kind of lifetime management mechanism. So probably instructions would need to be written like this:

protocol InModule: Contextual { ... }
protocol Instruction: InModule { ... }

There's a lot of boilerplate involved in adding a reference to all of these data structures and I'm wondering if perhaps we should try a less clever approach. The context is a the most annoying object to track because it really doesn't fit nicely into a MVS-oriented wrapper. So my original idea was to hide LLVMContext under the rug. With the benefit of insights, however, I think it was a bad idea.

We need to use different contexts if we want to use LLVM in a multithreaded environment, which we'll probably want to do sooner than later (LLVM codegen currently takes ~10% of our compile time). So I guess at that point we'll want to uncouple the context from the module and write the code of a single thread in a lambda like:

queue.addOperation {
  SwiftyLLVM.withContext { (llvm) in
    // Emit some modules ...
  }
}

AFAICT, pretty much everything else is tied to the lifetime of a module. So perhaps we need to pull the same trick:

SwiftyLLVM.withContext { (llvm) in
  llvm.withNewModule("MyModule") { (m) in
    // Emit some functions ...
  }
}

Then we document that it is UB to use any context/module property outside of the corresponding closure and all existing data structures can remain as they are. In practice it shouldn't degrade the UX because I expect the common use case to look like this:

SwiftyLLVM.withContext { (llvm) in
  llvm.withNewModule("xyz") { (m) in emitModule(&m) }
}

func emitModule(_ m: inout SwfityLLVM.Module) {
  // Lifetime-extending closures shenanigans have no impact here
}

It is of course disappointing to offer an unsafe API in a safe-by-default language but at the same time I don't think we should make it a goal to create a safe API for LLVM. That's beyond the current purpose of SwiftyLLVM.

[kyouko-taiga]

As I commented on the PR, I think that's a sound approach but we should also consider the lifetime of other "disposable" LLVM objects.

[dabrahams]

Yes, the withXXX idiom is how I probably would have done this if I had started coding this from scratch. It's a way to make unsafety manageable with a little care. Lifetimes are probably not the only way we can create UB by working with the LLVM API, so managing them doesn't get us all the way to "safe". We should carefully document all the preconditions.

FWIW, this is how Swift is doing it with C++ interop; not very principled.

[kyouko-taiga]

not very principled.

Indeed, it feels "hacky".

[dabrahams]

@kyouko-taiga we should close this, then?

[kyouko-taiga]

Yes, let's close. I will submit another PR using the withXXX approach, hopefully this afternoon.