[PATCH] D60253: [gn] Support for building runtimes

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: llvm/utils/gn/secondary/clang/runtimes.gni:4
+
+runtimes_dir = "$clang_resource_dir/$llvm_target_triple/lib"
----------------
In http://llvm-cs.pcc.me.uk/utils/gn/secondary/compiler-rt/target.gni we have crt_current_out_dir which is basically the same as this.

IIUC this is the "new" style of runtime paths which is currently only used by Fuchsia. Should the code there be changed to do "if (is_fuchsia) use the new-style paths else use the old-style paths"?


================
Comment at: llvm/utils/gn/secondary/libcxx/src/BUILD.gn:4
+
+declare_args() {
+  # Build libc++ with definitions for operator new/delete.
----------------
Do you really need all of these arguments? I would recommend only implementing the default behavior for most of them except for the ones that you actually need.

(Same comment elsewhere.)


================
Comment at: llvm/utils/gn/secondary/libcxx/src/BUILD.gn:159
+]
+if (target_os == "solaris") {
+  cxx_sources += [
----------------
I think this code is basically dead because the rest of the build system doesn't support Solaris.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60253/new/

https://reviews.llvm.org/D60253