[bazel] Add missing C++ style clang headers and modulemap

[aaronmondal]

Summary

These appear to have been overlooked.

[aaronmondal]

As I don't have commit access, It'd be nice if someone could commit on my behalf.
Name: Aaron Siddhartha Mondal
Email: [e-mail]

[aaronmondal]

Well, of course we want to ADD these headers, not exclude them 😅

[aaronmondal]

Move these headers into glob to clearly differentiate generated and non-generated files.

[chapuni]

FYI, I intend that builtin_headers_gen may be available for the target for packaging.
Its contents shall be the same (or subset) of the installaed image by CMake.

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

1483

Seems odd to pass non-wildcard files to glob.

Instead, could we reorganize the pattern? Like,

• lib/Headers/*.h
• lib/Headers/cuda_wrappers
• lib/Headers/openmp_wrappers
• lib/Headers/ppc_wrappers

(clang/lib/Headers/hlsl may be excluded here for now, since HLSL is not enabled in bazel build)

[aaronmondal]

Address comments.

[aaronmondal]

In D136452#3880688, @chapuni wrote:

FYI, I intend that builtin_headers_gen may be available for the target for packaging.
Its contents shall be the same (or subset) of the installaed image by CMake.

Ah now it makes sense to me why we are excluding two headers.

Regarding the updated diff, it will work, but may go slightly against the guideline in https://bazel.build/reference/be/functions#glob, stating

In general, you should try to provide an appropriate extension (e.g. *.html) instead of using a bare '*' for a glob pattern. The more explicit name is both self documenting and ensures that you don't accidentally match backup files, or emacs/vi/... auto-save files.

I don't think this is an issue in this particular case though.

[aaronmondal]

Add missing comma.

[aaronmondal]

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

1483

There would also be the possibility to use (note the last line)

"lib/Headers/*.h",
"lib/Headers/cuda_wrappers/*",
"lib/Headers/openmp_wrappers/*",
"lib/Headers/ppc_wrappers/*.h",

but I think just using the asterisks in this case should be clear enough. Omitting the asterisks won't work.

[chapuni]

Excuse me, I forgot trailer asterisks in my previous comment.

Regarding the updated diff, it will work, but may go slightly against the guideline in https://bazel.build/reference/be/functions#glob, stating

In general, you should try to provide an appropriate extension (e.g. *.html) instead of using a bare '*' for a glob pattern. The more explicit name is both self documenting and ensures that you don't accidentally match backup files, or emacs/vi/... auto-save files.

I don't think this is an issue in this particular case though.

I have rethought it should be an issue. We should not use glob for C++ suffix-less headers.
Your Diff 2 would be sufficient for now, I think.

Or shall we expand globbing?

[aaronmondal]

@chapuni I think fully expanding the glob does not really gain us anything. It would only add ~300 lines of headers, which I would assume to not really impact builds anyways, except if a header is missing as in openmp_headers/time.h. Keeping those up to date would likely be unnecessary maintenance burden.

I'll revert to Diff2 and add the module.modulemap as well, which is also part of the core files exported by the CMake build.

[aaronmondal]

Address comments. Add module.modulemap as well, since it is always exported by the CMake build.

[chapuni]

@aaronmondal Thank you for working. If you push your branch anywhere, I could fetch and commit it.

[aaronmondal]

I can't get arcanist or something else to work. As i don't have commit access, I pushed the changes to this git repo: https://github.com/aaronmondal/llvm-project/commit/652afbe8aff0dbf45d2d07ef5b74fdce137269e3

[aaronmondal]

@chapuni maybe this here would be better? I can't get this to run, but maybe they work for you with

arc patch D136452
git commit --amend --author="Aaron Siddhartha Mondal <[e-mail]>"

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

td_srcs = [
         "include/clang/Basic/riscv_vector.td",
     ],
 )
 
 # We generate the set of builtin headers under a special subdirectory in the
 # 'bin' section of the bazel output so that they can be used as data
 # dependencies. It requires listing explicitly all the generated inputs here.
 builtin_headers = glob(
-    ["lib/Headers/**/*.h"],
+    [
+        "lib/Headers/*.h"
+        "lib/Headers/cuda_wrappers/*",
+        "lib/Headers/openmp_wrappers/*",
+        "lib/Headers/ppc_wrappers/*",
+    ],
     exclude = [
         # FIXME: They are not handled in CMake side.
         "lib/Headers/openmp_wrappers/time.h",
         "lib/Headers/ppc_wrappers/nmmintrin.h",
     ],
 ) + [
     "lib/Headers/arm_cde.h",
     "lib/Headers/arm_fp16.h",