[bazel] Include builtin headers with clang-tidy

[jathu]

Summary

Context: https://github.com/llvm/llvm-project/issues/63890

These builtin headers are required to exist alongside clang-tidy for it to work. Specifically, the tool looks for headers relatively in ../lib/clang/{major version}/include.

[jathu]

oops fixing bad merge

[jathu]

undo bazelrc lld comment

[aaronmondal]

I like this change. If something like this also works for the clang target and maybe a few others it would relieve downstream users from implementing logic around this resource path. (E.g. it could potentially reduce some of the hackiness in https://github.com/eomii/rules_ll/blob/37b5721a8083d36332eaca1f62fcc43b8aba7bd1/ll/args.bzl#L9-L47).

utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel

17

This seems to better fit into the clang directory in the overlay so that targets can consume //clang:builtin_headers.

utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel

372

I like this data attribute approach a lot. I wonder whether it's possible to do something similar for the clang binary target. This could reduce the complexity of using the clang binary target from the Bazel overlay.

utils/bazel/llvm-project-overlay/clang/BUILD.bazel

1704

The name gen_dir could be misinterpreted as Bazel's gendir variable. If this contains everything in the location from -resource-dir, it might make sense to call it resource_dir.

utils/bazel/llvm-project-overlay/clang/defs.bzl

6

Would a string flag with a default value of "staging/include/" make sense here? How is this handled in the CMake build?

[keith]

utils/bazel/llvm-project-overlay/clang-tools-extra/defs.bzl

8

I'm not sure if the LLVM repo wants this but there is a rule in the bazel-skylib for this https://github.com/bazelbuild/bazel-skylib/blob/main/docs/copy_file_doc.md#copy_file-allow_symlink

[jathu]

utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel

17

We can't do this because the tool looks for the path in its parent folder ../lib/clang/{major version}/include, where as your suggestion would move it within the clang folder

utils/bazel/llvm-project-overlay/clang-tools-extra/defs.bzl

8

I don't think that does everything we want:

1. The source only takes a single file, so we'd have to write a macro to loop through the files and create the targets anyways
2. We are chopping the path after the staging/include/ path and preserving everything right of it (the subdirectories) — which we'd have to do still do if we use that rule. The only difference will be calling that rule instead of ctx.actions.symlink

utils/bazel/llvm-project-overlay/clang/defs.bzl

6

I don't know the motivation behind this path, it seems to have been introduced in the initial commit: https://github.com/llvm/llvm-project/commit/4aeb2e60df98a07e6a2a3cc16fc9ad1c1001d563. I should say, I'm not introducing anything new here — the logic is preserved, I'm only allowing the path to be static and shared

[jathu]

rename gen_dir to resource_dir

[jathu]

bump @aaronmondal @keith

[jathu]

bump for anyone in the @bazel_build group. cc @goncharov @gchatelet @hokein (based on git history for clang-tidy)

This will unblock clang-tidy builds and allow it to be used out of the box

[jathu]

This comment was removed by jathu.

[jathu]

bump @aaronmondal

[jathu]

Closing this diff to attempt to get another fresh pair of eyes on this change: D158942

utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 load("@bazel_skylib//rules:expand_template.bzl", "expand_template")
-load("//:workspace_root.bzl", "workspace_root")
-load("//llvm:tblgen.bzl", "gentbl")
-load("//llvm:binary_alias.bzl", "binary_alias")
-load("//llvm:cc_plugin_library.bzl", "cc_plugin_library")
+load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")
+load("@rules_python//python:defs.bzl", "py_binary")
 load(
     "//:vars.bzl",
     "LLVM_VERSION",
     "LLVM_VERSION_MAJOR",
     "LLVM_VERSION_MINOR",
     "LLVM_VERSION_PATCH",
 )
+load("//:workspace_root.bzl", "workspace_root")
+load("//llvm:binary_alias.bzl", "binary_alias")
+load("//llvm:cc_plugin_library.bzl", "cc_plugin_library")
+load("//llvm:tblgen.bzl", "gentbl")
+load("defs.bzl", "BUILTIN_HEADERS_GEN_DIR")
 
 package(
     default_visibility = ["//visibility:public"],
     features = ["layering_check"],
 )
 
 licenses(["notice"])
 
) + [
     "lib/Headers/openmp_wrappers/cmath",
     "lib/Headers/openmp_wrappers/complex",
     "lib/Headers/openmp_wrappers/new",
 ]
 
 genrule(
     name = "builtin_headers_gen",
     srcs = builtin_headers,
-    outs = [hdr.replace("lib/Headers/", "staging/include/") for hdr in builtin_headers],
+    outs = [
+        hdr.replace("lib/Headers/", BUILTIN_HEADERS_GEN_DIR)
+        for hdr in builtin_headers
+    ],
     cmd = """
        for src in $(SRCS); do
-         relsrc=$${src#*"$(WORKSPACE_ROOT)"/clang/lib/Headers}
-         target=$(@D)/staging/include/$$relsrc
+         relsrc=$${{src#*"$(WORKSPACE_ROOT)"/clang/lib/Headers}}
+         target=$(@D)/{gen_dir}$$relsrc
          mkdir -p $$(dirname $$target)
          cp $$src $$target
-       done""",
+       done""".format(gen_dir = BUILTIN_HEADERS_GEN_DIR),
     output_to_bindir = 1,
     toolchains = [
         ":workspace_root",
     ],
 )
 
 cc_library(
     name = "frontend",

utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel

deps = CHECKS + [
         "//clang:tooling",
         "//llvm:Support",
     ],
 )
 
 cc_binary(
     name = "clang-tidy",
     srcs = ["tool/ClangTidyToolMain.cpp"],
+    data = ["//clang-tools-extra:builtin_headers"],
     stamp = 0,
     deps = [":tool"],
 )
 
 native_binary(
     name = "run-clang-tidy",
     src = "tool/run-clang-tidy.py",
     out = "run-clang-tidy",
     data = [":clang-tidy"],
 )

utils/bazel/llvm-project-overlay/clang-tools-extra/defs.bzl

+# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+BUILTIN_HEADERS_GEN_DIR = "staging/include/"

utils/bazel/llvm-project-overlay/clang/BUILD.bazel

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 load("@bazel_skylib//rules:expand_template.bzl", "expand_template")
-load("//:workspace_root.bzl", "workspace_root")
-load("//llvm:tblgen.bzl", "gentbl")
-load("//llvm:binary_alias.bzl", "binary_alias")
-load("//llvm:cc_plugin_library.bzl", "cc_plugin_library")
+load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")
+load("@rules_python//python:defs.bzl", "py_binary")
 load(
     "//:vars.bzl",
     "LLVM_VERSION",
     "LLVM_VERSION_MAJOR",
     "LLVM_VERSION_MINOR",
     "LLVM_VERSION_PATCH",
 )
+load("//:workspace_root.bzl", "workspace_root")
+load("//llvm:binary_alias.bzl", "binary_alias")
+load("//llvm:cc_plugin_library.bzl", "cc_plugin_library")
+load("//llvm:tblgen.bzl", "gentbl")
+load("defs.bzl", "BUILTIN_HEADERS_GEN_DIR")
 
 package(
     default_visibility = ["//visibility:public"],
     features = ["layering_check"],
 )
 
 licenses(["notice"])
 
) + [
     "lib/Headers/openmp_wrappers/cmath",
     "lib/Headers/openmp_wrappers/complex",
     "lib/Headers/openmp_wrappers/new",
 ]
 
 genrule(
     name = "builtin_headers_gen",
     srcs = builtin_headers,
-    outs = [hdr.replace("lib/Headers/", "staging/include/") for hdr in builtin_headers],
+    outs = [
+        hdr.replace("lib/Headers/", BUILTIN_HEADERS_GEN_DIR)
+        for hdr in builtin_headers
+    ],
     cmd = """
        for src in $(SRCS); do
-         relsrc=$${src#*"$(WORKSPACE_ROOT)"/clang/lib/Headers}
-         target=$(@D)/staging/include/$$relsrc
+         relsrc=$${{src#*"$(WORKSPACE_ROOT)"/clang/lib/Headers}}
+         target=$(@D)/{gen_dir}$$relsrc
          mkdir -p $$(dirname $$target)
          cp $$src $$target
-       done""",
+       done""".format(gen_dir = BUILTIN_HEADERS_GEN_DIR),
     output_to_bindir = 1,
     toolchains = [
         ":workspace_root",
     ],
 )
 
 cc_library(
     name = "frontend",

utils/bazel/llvm-project-overlay/clang/defs.bzl

+# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+BUILTIN_HEADERS_GEN_DIR = "staging/include/"