[libcxx-commits] [PATCH] D61143: [gn] Support for building libc++

[Nico Weber via Phabricator via libcxx-commits]

thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

Nice!



================
Comment at: llvm/utils/gn/secondary/libcxx/BUILD.gn:4
+    "include",
+    "src(//llvm/utils/gn/build/toolchain:stage2_unix)",
+  ]
----------------
Mention in change description that we (currently -- for libc++ I can see this maybe changing in the future, unlike for other runtime libs) always build libc++ with a just-built clang (and build clang if it hasn't been built yet). Also mention that this currently only is for linux.


================
Comment at: llvm/utils/gn/secondary/libcxx/include/BUILD.gn:6
+declare_args() {
+  libcxx_install_support_headers = true
+}
----------------
Add a comment (to make `gn args --list` output nicer)


================
Comment at: llvm/utils/gn/secondary/libcxx/include/BUILD.gn:32
+  action("concat_config") {
+    script = "../utils/cat_files.py"
+    inputs = [
----------------
absolute path ("//libcxx/utils/cat_files.py")


================
Comment at: llvm/utils/gn/secondary/libcxx/include/BUILD.gn:74
+    "__functional_base_03",
+    "__hash_table",
+    "__libcpp_version",
----------------
Should we make the sync script handle files without extensions too?


================
Comment at: llvm/utils/gn/secondary/libcxx/src/BUILD.gn:12
+  # Build libc++ as a static library.
+  libcxx_enable_static = true
+
----------------
Are both of these enabled by default in cmake? (shared and static, that is)


================
Comment at: llvm/utils/gn/secondary/libcxx/src/BUILD.gn:173
+  if (libcxx_use_compiler_rt) {
+    cxx_sources += [ "filesystem/int128_builtins.cpp" ]
+  }
----------------
Probably needs a comment above the list entry to make the sync script happy?


================
Comment at: llvm/utils/gn/secondary/libcxx/src/BUILD.gn:238
+    output_name = "c++"
+    complete_static_lib = true
+    sources = cxx_sources
----------------
Maybe put configs -= [ thin_archive] right after this, since the two lines belong together conceptually


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D61143