[PATCH] Initial version of Go bindings.

[Peter Collingbourne]

================
Comment at: bindings/go/llvm/bitwriter.cpp:25
@@ +24,3 @@
+
+LLVMMemoryBufferRef gollvm_WriteBitcodeToMemoryBuffer(LLVMModuleRef M) {
+  std::string Data;
----------------
chandlerc wrote:
> Is there any particular reason to not add this to the existing C APIs?
> 
> Also, if it remains, this file should be named with CamelCase. Probably as GoBitWriter or BitWriterBindings.
> Is there any particular reason to not add this to the existing C APIs?

In general, for this change I tried to avoid modifying the C API, mainly because of the stability policy.

That said, this specific function looks fine to add to the C API. Done in r219643.

> Also, if it remains, this file should be named with CamelCase. Probably as GoBitWriter or BitWriterBindings.

>From the point of view of a maintainer, it's convenient to be able to find the C++ for `foo.go` in `foo.cpp`. Also, the coding standards don't seem to prescribe file names. But this isn't a big deal, I'm happy to rename them.

================
Comment at: bindings/go/llvm/dibuilder.cpp:18
@@ +17,3 @@
+#include "llvm/IR/DIBuilder.h"
+#include <iostream>
+
----------------
chandlerc wrote:
> iostreams aren't used in LLVM
Removed. Not sure how that got there.

================
Comment at: bindings/go/llvm/dibuilder.h:10
@@ +9,3 @@
+//
+// This file defines C bindings for the DIBuilder class.
+//
----------------
chandlerc wrote:
> This file too should be GoDIBuilderBindings.{h,cpp} or something CamelCased.
> 
> Do you think these should be Go-specific, or generic C APIs for bindings? I'm curious how you see this evolving over time.
There's no technical reason why these couldn't be part of the C API, but because of our C API stability policy, I think they should live here for now.

We could consider changing this policy in the future, for example by defining a core 'stable' subset of the C API, but I feel that that discussion should only happen as and when it needs to (e.g. if we add another set of language bindings that expose debug info).

================
Comment at: bindings/go/llvm/ir.h:24-26
@@ +23,5 @@
+
+void gollvm_AddFunctionAttr(LLVMValueRef Fn, uint64_t PA);
+uint64_t gollvm_GetFunctionAttr(LLVMValueRef Fn);
+void gollvm_RemoveFunctionAttr(LLVMValueRef Fn, uint64_t PA);
+
----------------
chandlerc wrote:
> I really thought we already had the C APIs for this... If not, they make a lot of sense to just add.
We have `LLVMAddFunctionAttr` etc, but they take a 32-bit attribute value; in particular, we need a 64-bit attribute value to set some of the sanitizer attributes. We could add a 64-bit variant of this function, but it would probably need its own set of constants.

================
Comment at: bindings/go/llvm/support.h:21-24
@@ +20,6 @@
+
+void gollvm_LoadLibraryPermanently(const char *Filename, char **ErrMsg);
+
+void gollvm_ParseCommandLineOptions(int argc, const char *const *argv,
+                                    const char *Overview);
+
----------------
chandlerc wrote:
> These both seem moderately terrifying to expose in the Go bindings. Why were they needed?
These were needed to implement `-fload-plugin` and `-mllvm` in the Go frontend; despite the problems with them, it is sometimes useful or convenient for Go frontend users or developers to be able to set `llvm::cl` flags or load plugins from the command line. I'd imagine that if we expose command line flags through some other mechanism in the future, this API would change as well.

================
Comment at: bindings/go/llvm/transforms_instrumentation.h:23-28
@@ +22,8 @@
+
+void gollvm_AddAddressSanitizerFunctionPass(LLVMPassManagerRef PM);
+void gollvm_AddAddressSanitizerModulePass(LLVMPassManagerRef PM);
+void gollvm_AddThreadSanitizerPass(LLVMPassManagerRef PM);
+void gollvm_AddMemorySanitizerPass(LLVMPassManagerRef PM);
+void gollvm_AddDataFlowSanitizerPass(LLVMPassManagerRef PM,
+                                     const char *ABIListFile);
+
----------------
chandlerc wrote:
> I think it would be reasonable to add an 'Instrumentation.h' to llvm-c/Transforms with these in it.
I am not sure if we can consider the APIs for these to be stable yet; in particular, there are some changes for DFSan that I have in mind that might change this API.

================
Comment at: test/Bindings/Go/go.test:1
@@ +1,2 @@
+; RUN: cd %S/../../../bindings/go/llvm && env CGO_CPPFLAGS="$(llvm-config --cppflags)" CGO_CXXFLAGS=-std=c++11 CGO_LDFLAGS="$(llvm-config --ldflags --libs --system-libs $(../build.sh --print-components))" %go test -tags byollvm .
+
----------------
chandlerc wrote:
> Ow.
> 
> Is there any way to factor some of this into the lit.local.cfg?
> 
> At the least split it into multiple lines with '\' to smash them together?
I wanted to keep the command output of lit.cfg copy-pastable if this test fails (at least in most normal situations).

I didn't know (or had forgotten) that you could use '\' on these lines. Changed.

http://reviews.llvm.org/D5684