[clang-tools-extra] [Bazel][Clang Tidy] Include builtin headers with clang-tidy (PR #67626)

[Arthur Eubanks via cfe-commits]

aeubanks wrote:

I think there are two approaches to this problem
1) point `-resource-dir` to the directory where the headers are, e.g. 
`clang-tidy --extra-arg=-resource-dir=path/to/include`
2) mirror the CMake binary/include layout (this patch)

issues with 1: extra argument to pass, have to figure out include path to pass
issues with 2: relies on bazel paths, i.e. this puts the symlink at 
`bazel-bin/external/llvm-project/clang-tools-extra/` which happens to be two 
directories up from 
`bazel-bin/external/llvm-project/clang-tools-extra/clang-tidy/clang-tidy`

for 1, I'm not sure how hard it is in bazel to get the path for where we put 
the builtins
for 2, is it possible to explicitly copy clang-tidy and the headers to a path 
that mirrors the CMake build? e.g. put `clang-tidy` in a `bin/` dir and the 
headers in `lib/clang/18/include`. or is that too un-bazel-like? the gn build 
does this.

https://github.com/llvm/llvm-project/pull/67626
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Bazel][Clang Tidy] Include builtin headers with clang-tidy (PR #67626)

[Reid Kleckner via cfe-commits]

rnk wrote:

I think option 1 isn't really a permanent solution. We have lots of clang tools 
that need to find the resource directory, and it should happen automatically.

For option 2, we'd have to reimplement that for every other clang tool that 
needs to find resources, like LLD as well.

Should we consider baking in some kind of configurable resource directory 
searching logic into the binary, similar to an RPATH? Essentially, instead of 
searching at "$(dirname $argv0)/../lib/clang/${version}", we'd make 
"../lib/clang" configurable, or provide an absolute path.

Actually, a CMake option for this already exists, CLANG_RESOURCE_DIR:
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Config/config.h.cmake#L39C1-L39C1

Should Bazel go ahead and use that directly? Should we perhaps enhance it so 
that it is baked into each executable, rather than having it global to in 
config.h?

https://github.com/llvm/llvm-project/pull/67626
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Bazel][Clang Tidy] Include builtin headers with clang-tidy (PR #67626)

[Arthur Eubanks via cfe-commits]

aeubanks wrote:

my worry about baking in the resource dir for the bazel build is that it'll be 
tied to the bazel build directory structure implementation details, so it's 
likely that you can't copy the binaries around and create a toolchain out of 
it. if bazel users don't care about this, then that's fine, but it seems like a 
common use case to assemble a toolchain that resembles a toolchain built with 
CMake. that's why I think it'd be nice to explicitly copy files into a typical 
toolchain structure, and clang tools that rely on the default builtin headers 
relative path will all "just work"

or something along the lines of this patch where we symlink the headers 
(choosing relative directories in a more structured way if possible as 
mentioned earlier, otherwise this is fine if there's no cleaner way)

https://github.com/llvm/llvm-project/pull/67626
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Bazel][Clang Tidy] Include builtin headers with clang-tidy (PR #67626)

[via cfe-commits]

https://github.com/jathu updated https://github.com/llvm/llvm-project/pull/67626

>From 20d9ca99fbf11868a5816df217b2aad09e079fb6 Mon Sep 17 00:00:00 2001
From: jathu 
Date: Wed, 27 Sep 2023 18:01:19 -0700
Subject: [PATCH] [clang-tidy][bazel] Include builtin headers with clang-tidy

---
 .../clang-tools-extra/BUILD.bazel | 21 +++
 .../clang-tools-extra/clang-tidy/BUILD.bazel  |  1 +
 .../clang-tools-extra/defs.bzl| 26 +++
 .../llvm-project-overlay/clang/BUILD.bazel| 12 ++---
 .../bazel/llvm-project-overlay/clang/defs.bzl |  5 
 5 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 
utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel
 create mode 100644 utils/bazel/llvm-project-overlay/clang-tools-extra/defs.bzl
 create mode 100644 utils/bazel/llvm-project-overlay/clang/defs.bzl

diff --git a/utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel 
b/utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel
new file mode 100644
index 000..d7559a376698583
--- /dev/null
+++ b/utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel
@@ -0,0 +1,21 @@
+# 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("//:vars.bzl", "LLVM_VERSION_MAJOR")
+load("//clang:defs.bzl", "BUILTIN_HEADERS_GEN_DIR")
+load("defs.bzl", "symlink")
+
+package(
+default_visibility = ["//visibility:public"],
+features = ["layering_check"],
+)
+
+licenses(["notice"])
+
+symlink(
+name = "builtin_headers",
+srcs = ["//clang:builtin_headers_gen"],
+destination = "lib/clang/{0}/include".format(LLVM_VERSION_MAJOR),
+partition = BUILTIN_HEADERS_GEN_DIR,
+)
diff --git 
a/utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel 
b/utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel
index bbabc5397e98937..257c9e875d0133b 100644
--- a/utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel
@@ -370,6 +370,7 @@ cc_library(
 cc_binary(
 name = "clang-tidy",
 srcs = ["tool/ClangTidyToolMain.cpp"],
+data = ["//clang-tools-extra:builtin_headers"],
 stamp = 0,
 deps = [":tool"],
 )
diff --git a/utils/bazel/llvm-project-overlay/clang-tools-extra/defs.bzl 
b/utils/bazel/llvm-project-overlay/clang-tools-extra/defs.bzl
new file mode 100644
index 000..fd992c052b0f055
--- /dev/null
+++ b/utils/bazel/llvm-project-overlay/clang-tools-extra/defs.bzl
@@ -0,0 +1,26 @@
+# 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//lib:paths.bzl", "paths")
+
+def _symlink_impl(ctx):
+copied_files = []
+for input_file in ctx.files.srcs:
+(_, _, relative_filename) = 
input_file.path.rpartition(ctx.attr.partition)
+output_file = 
ctx.actions.declare_file(paths.join(ctx.attr.destination, relative_filename))
+ctx.actions.symlink(
+target_file = input_file,
+output = output_file,
+)
+copied_files.append(output_file)
+return DefaultInfo(files = depset(copied_files))
+
+symlink = rule(
+implementation = _symlink_impl,
+attrs = {
+"destination": attr.string(mandatory = True),
+"partition": attr.string(mandatory = True),
+"srcs": attr.label_list(allow_files = True),
+},
+)
diff --git a/utils/bazel/llvm-project-overlay/clang/BUILD.bazel 
b/utils/bazel/llvm-project-overlay/clang/BUILD.bazel
index cac8ec120929a37..02f0f6076c6fddd 100644
--- a/utils/bazel/llvm-project-overlay/clang/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/clang/BUILD.bazel
@@ -14,6 +14,7 @@ load(
 "LLVM_VERSION_MINOR",
 "LLVM_VERSION_PATCH",
 )
+load("defs.bzl", "BUILTIN_HEADERS_GEN_DIR")
 
 package(
 default_visibility = ["//visibility:public"],
@@ -1696,14 +1697,17 @@ builtin_headers = glob(
 genrule(
 name = "builtin_headers_gen",
 srcs = builtin_headers,
-outs = [hdr.replace("lib/Headers/", "staging/include/") for hdr in 
builtin_headers],
+outs = [
+header.replace("lib/Headers/", BUILTIN_HEADERS_GEN_DIR)
+for header 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)/{resource_dir}$$relsrc
  mkdir -p $$(dirname $$target)
  cp $$src $$target
-   done""",
+   done""".format(resource_dir = BUILTIN_HEADERS_GEN_DIR),
 output_to_bindir = 1,
 toolchains = [
 ":workspace_root",
diff --g

[flang] [clang] [compiler-rt] [llvm] [clang-tools-extra] [Bazel][Clang Tidy] Include builtin headers with clang-tidy (PR #67626)

[Matthew Stephenson via cfe-commits]

https://github.com/erl4ng commented:

+1

https://github.com/llvm/llvm-project/pull/67626
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [compiler-rt] [llvm] [flang] [clang-tools-extra] [Bazel][Clang Tidy] Include builtin headers with clang-tidy (PR #67626)

[via cfe-commits]

jathu wrote:

@chapuni can you help my understand what you mean by those two suggestions by 
providing some examples?

> construct the symlink-ed tree for one's convenience

Isn't that what we are doing in this diff? We are producing the symlinked tree 
for the tool.

https://github.com/llvm/llvm-project/pull/67626
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits