[PATCH] D107414: [Bazel] Add support for lld

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:16
+    outs = ["include/lld/Common/Version.inc"],
+    cmd = "echo \"#define LLD_VERSION_STRING \\\"git\\\"\" > $@",
+)
----------------
Use single quotes for echo arguments to avoid some escapes.


================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:22
+    outs = ["Common/VCSVersion.inc"],
+    cmd = "echo '#define LLD_REVISION \"git\"' >> $@\n" +
+          "echo '#undef LLD_REPOSITORY' >> $@\n",
----------------
If LLD_REPOSITORY is undefined, `LLD_REVISION` doesn't need to be defined.


================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:70
+        "ELF/*.cpp",
+        "ELF/*.h",
+        "ELF/Arch/*.cpp",
----------------
Is `*.h` needed in `srcs`?


================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:147
+        "//llvm:TransformUtils",
+        "//llvm:WindowsManifest",
+    ],
----------------
In CMake, with `LLVM_ENABLE_LIBXML2` this dependency is essentially a no-op. Can Bazel allow optional builds?


================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:239
+cc_library(
+    name = "MachO2",
+    srcs = glob(["MachO/**/*.cpp"]),
----------------
Just `MachO`

The special name can go to the old port which will go away soon.


================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:304
+    name = "wasm_options_inc_gen",
+    # Bazel seems to be broken when you try to include generated files from the
+    # same directory using only their basename, so we have to strip the prefix.
----------------
This comment is repeated multiple times. Is there a Bazel bug or anything we can fix in this file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107414