[PATCH] Add llvm-go tool.

Peter Collingbourne peter at pcc.me.uk
Wed Oct 22 18:24:34 PDT 2014


================
Comment at: bindings/go/build.sh:31
@@ -57,11 +30,2 @@
 llvm_version="$($llvm_config --version)"
-llvm_cflags="$($llvm_config --cppflags)"
-llvm_ldflags="$($llvm_config --ldflags) $($llvm_config --libs $llvm_components) $($llvm_config --system-libs)"
-if [ $(uname) != "Darwin" ]; then
-  # OS X doesn't like -rpath with cgo. See:
-  # https://code.google.com/p/go/issues/detail?id=7293
-  llvm_ldflags="-Wl,-rpath,$($llvm_config --libdir) $llvm_ldflags"
-fi
-sed -e "s#@LLVM_CFLAGS@#$llvm_cflags#g; s#@LLVM_LDFLAGS@#$llvm_ldflags#g" $gollvmdir/llvm_config.go.in > \
-  $gollvmdir/llvm_config.go
 printf "package llvm\n\nconst Version = \"%s\"\n" "$llvm_version" > $gollvmdir/version.go
----------------
ruiu wrote:
> I think generated files shouldn't be written to the LLVM source tree. Instead it should be written to the build directory.
Normally that is the case, but the `build.sh` script is used in the case where we `go get` LLVM, so there is no build directory as such.

I don't think we necessarily need to generate `version.go` though. Separately from this change, I'll investigate if we can change this to something else that does not require us to generate it here.

================
Comment at: tools/llvm-go/llvm-go.go:35
@@ +34,3 @@
+
+type compilerflags struct {
+	cppflags, cxxflags, ldflags string
----------------
ruiu wrote:
> compilerFlags
Done.

================
Comment at: tools/llvm-go/llvm-go.go:36
@@ +35,3 @@
+type compilerflags struct {
+	cppflags, cxxflags, ldflags string
+}
----------------
ruiu wrote:
> drop "flags" from field name.
Done.

================
Comment at: tools/llvm-go/llvm-go.go:103
@@ +102,3 @@
+		} else if a == "-tags" && i+1 < len(args) {
+			args[i+1] = args[i+1] + tag
+			addedTag = true
----------------
ruiu wrote:
> This should be
> 
>   args[i+1] = args[i+1] + " " + tag
> 
> ?
Yes, done.

================
Comment at: tools/llvm-go/llvm-go.go:120
@@ +119,3 @@
+
+	fmt.Printf(`// +build !byollvm
+
----------------
ruiu wrote:
> You may want to a comment that this file is auto-generated by llvm-go.go.
Done.

================
Comment at: tools/llvm-go/llvm-go.go:152
@@ +151,3 @@
+
+		os.Symlink(filepath.Join(srcdir, p.llvmpath), path)
+	}
----------------
ruiu wrote:
> Need to check the return value. On Windows I think it will return an error.
Done. That reminds me that I should disable the Go stuff altogether on Windows for now.

================
Comment at: tools/llvm-go/llvm-go.go:189
@@ +188,3 @@
+	}
+	proc.Wait()
+
----------------
ruiu wrote:
> Check the error.
Done.

================
Comment at: tools/llvm-go/llvm-go.go:209
@@ +208,3 @@
+		printConfig()
+	}
+}
----------------
ruiu wrote:
> Print the help message if none above.
Done.

http://reviews.llvm.org/D5902






More information about the llvm-commits mailing list