[WIP] rustc_llvm: use rust-bindgen to generate FFI bindings

[tamird]

This is my WIP attempt at #17795; note that the majority of the work involves hoisting various definitions from rustllvm/*.cpp to rustllvm/rustllvm.h, and then mapping various LLVMFoo to Foo in Rust to match the current exported names.

This doesn't compile since many definitions haven't yet been hoisted, but librustc_llvm itself does compile, so this should be a decent proof of concept for y'all to give feedback on before I proceed.

I suspect @eddyb will have opinions.

EDIT: compiles now.

[tamird]

It turns out I can feed the .cpp files directly into bindgen, negating the need for most of the manual work of moving things into the header! Let me know if that's preferable.

[tamird]

Ah, there's no libclang on the builders. Maybe the generated bindings should just be checked in.

[tamird]

Anyway, I was able to bootstrap with this locally 🎉

[tamird]

here's the first mistake caught by this.

[cuviper]

Ah, there's no libclang on the builders. Maybe the generated bindings should just be checked in.

Requiring libclang would also make it harder on third-party builders, like distros. I think it would make sense to have bindgen use as an optional feature. Then yes, either check in the bindings, or just use them as a sanity check on the current bindings.

[tamird]

@cuviper how would I plumb the optional feature through? It doesn't look like I can pass --features rustc_llvm/rebuild_bindings from the top level cargo invocation - would I have to plumb "rebuild_llvm_bindings" through the dependency tree?

[cuviper]

would I have to plumb "rebuild_llvm_bindings" through the dependency tree?

Hmm, that's annoying.

It could just be a standalone crate in src/tools/ which performs bindgen. It would act basically like an early build script for rustc_llvm -- Cargo wouldn't know about it, but rustbuild could manage it and build/run it when enabled. This may even be better overall, since tools don't have to be rebuilt for each stage of the compiler bootstrap.

[tamird]

That's actually somewhat more annoying since we'd need to reimplement the logic that locates `llvm-config`. Any ideas on that?

…

On Nov 29, 2017 18:01, "Josh Stone" ***@***.***> wrote: would I have to plumb "rebuild_llvm_bindings" through the dependency tree? Hmm, that's annoying. It could just be a standalone crate in src/tools/ which performs bindgen. It would act basically like an early build script for rustc_llvm -- Cargo wouldn't know about it, but rustbuild could manage it and build/run it when enabled. This may even be better overall, since tools don't have to be rebuilt for each stage of the compiler bootstrap. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#46353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPAni8IXN1whp7uxGk9HiMcgdqOcdks5s7eI_gaJpZM4QuRnR> .

[tamird]

Actually, that may not be so bad. I'm looking into it.

[tamird]

Eh, on second thought, I have no idea how bootstrap is organized. Perhaps someone more familiar with it could help with this part?

[eddyb]

cc @alexcrichton

[bors]

☔ The latest upstream changes (presumably #46144) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

Hm I think this requires a dependency on libclang, right? I think for that reason alone (unfortunately) I wouldn't want to take this route. If we want to validate the bindings then I think we should set up ctest which doesn't require any extra system dependencies.

[tamird]

I think the suggestion was to have an in-tree tool that regenerates the bindings, perhaps in addition to ctest? The advantage of having it in bootstrap is that bootstrap knows the path to llvm-config. Looking at ctest, it seems we'll need to run that from bootstrap as well since it also needs to locate llvm-config to get include paths. So...who can help me with the bootstrap scaffolding? On Nov 30, 2017 08:54, "Alex Crichton" <[email protected]> wrote: Hm I think this requires a dependency on libclang, right? I think for that reason alone (unfortunately) I wouldn't want to take this route. If we want to validate the bindings then I think we should set up ctest <https://github.com/alexcrichton/ctest> which doesn't require any extra system dependencies. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#46353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPOm-CZp3P6jYuMoOBl7j2kfAelc8ks5s7rOygaJpZM4QuRnR> .

[alexcrichton]

I would personally prefer to do validation rather than generation of bindings here myself, I've found handwritten bindings to be quite easy to maintain over time and it doesn't require contributors to get libclang set up or anything like that.

For rustbuild integration of verification it would probably look like a dedicated whole new test suite, I don't think it would fit into any of our existing test suites. You'd basically be twiddling with support in src/bootstrap/check.rs I believe.

[shepmaster]

@tamird We haven't heard from you in over 7 days — will you be able to address the recent feedback about pivoting to a validation perspective?

[tamird]

Yeah, haven't had a chance to do it but I plan to.

…

On Dec 9, 2017 10:27, "Jake Goulding" ***@***.***> wrote: @tamird <https://github.com/tamird> We haven't heard from you in over 7 days — will you be able to address the recent feedback about pivoting to a validation perspective? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#46353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPKc_k4chFwT4B9dveqfVC4QmVKSTks5s-qbpgaJpZM4QuRnR> .

[alexcrichton]

Ok it's been a few weeks now so I'm going to close this, but we can of course reconsider a resubmission!