[PATCH] D54345: Add initial scaffolding for the GN build.

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:51
+    } else {
+      command = "ar rcsDT {{arflags}} -o {{output}} {{inputs}}"
+    }
----------------
thakis wrote:
> phosek wrote:
> > Can we make `ar` variable as well? I'd like to change this, e.g. for Clang toolchains to use `llvm-ar`.
> Sure, but let's do that when we need it?
Why not use `llvm-ar` if the `clang_base_path` is set? I don't think there's any reason not to?


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:169
+    libfile = "$dllfile.lib"
+    command = "link /nologo /dll {{ldflags}} /out:$dllfile /implib:$libfile {{libs}} /pdb:$dllfile.pdb {{inputs}}"
+    description = "LINK $dllfile"
----------------
Can we use `lld-link` if `clang_base_path` is set?


================
Comment at: llvm/utils/gn/secondary/llvm/lib/Demangle/BUILD.gn:2
+static_library("Demangle") {
+  output_name = "LLVMDemangle"
+
----------------
Setting `output_name` explicitly is generally considered an anti-pattern and should be used only when absolutely necessary. I think we should just  `"LLVMDemangle"` as the target name.


https://reviews.llvm.org/D54345