chandlerc created this revision.
chandlerc added a reviewer: GMNGeoffrey.
Herald added a subscriber: mcrosier.
chandlerc requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
This required substantially more invasive changes I'm afraid.
First, there is an issue with a Clang header that isn't really robust on
Windows unless you get the includes *just* right. I've reworked it to be
a bit more defensive.
Second, I needed a different approach to get `libclang` working well.
This, IMO, improves things on all platforms. Now we build the plugin and
actually wrap it back up with `cc_import`. We have to use a collection
of manually tagged `cc_binary` rules to get the naming to work out the
right way, but this isn't too different from the prior approach. By
directly having a `cc_binary` rule for each platform spelling of
`libclang`, we can actually extract the interface library from it and
correctly depend on it with `cc_import`. I think the result now is much
closer to the intent and to the CMake build for libclang.
Last but not least, some tests needed disabling. This is actually
narrower than what CMake does. The issue isn't indicative of anything
serious -- the test just assumes Unix-style paths.
https://reviews.llvm.org/D112399
Files:
clang/include/clang/Basic/Builtins.def
utils/bazel/.bazelrc
utils/bazel/llvm-project-overlay/clang/BUILD.bazel
utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
utils/bazel/llvm-project-overlay/llvm/cc_plugin_library.bzl
Index: utils/bazel/llvm-project-overlay/llvm/cc_plugin_library.bzl
===================================================================
--- utils/bazel/llvm-project-overlay/llvm/cc_plugin_library.bzl
+++ utils/bazel/llvm-project-overlay/llvm/cc_plugin_library.bzl
@@ -4,51 +4,72 @@
"""A macro to produce a loadable plugin library for the target OS.
-This macro produces a `cc_binary` rule with the name `name + "_impl"`. It
-forces the rule to statically link in its dependencies but to be linked as a
-shared "plugin" library. It then creates binary aliases to `.so`, `.dylib`
-,and `.dll` suffixed names for use on various platforms and selects between
-these into a filegroup with the exact name passed to the macro.
+This macro produces a set of platform-specific `cc_binary` rules, by appending
+the platform suffix (`.dll`, `.dylib`, or `.so`) to the provided `name`. It then
+connects these to a `cc_import` rule with `name` exactly and `hdrs` that can be
+used by other Bazel rules to depend on the plugin library.
+
+The `srcs` attribute for the `cc_binary` rules is `srcs + hdrs`. Other explicit
+arguments are passed to all of the rules where they apply, and can be used to
+configure generic aspects of all generated rules such as `testonly`. Lastly,
+`kwargs` is expanded into all the `cc_binary` rules.
"""
-load("@rules_cc//cc:defs.bzl", "cc_binary")
-load(":binary_alias.bzl", "binary_alias")
+load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_import", "cc_library")
-def cc_plugin_library(name, **kwargs):
+def cc_plugin_library(name, srcs, hdrs, include_prefix = None, strip_include_prefix = None, alwayslink = False, features = [], tags = [], testonly = False, **kwargs):
# Neither the name of the plugin binary nor tags on whether it is built are
- # configurable. Instead, we build a `cc_binary` that implements the plugin
- # library using a `_impl` suffix. Bazel will use appropriate flags to cause
- # this file to be a plugin library regardless of its name. We then create
- # binary aliases in the different possible platform names, and select
- # between these different names into a filegroup. The macro's name becomes
- # the filegroup name and it contains exactly one target that is the target
- # platform suffixed plugin library.
+ # configurable. Instead, we build a `cc_binary` with each name and
+ # selectively depend on them based on platform.
#
# All-in-all, this is a pretty poor workaround. I think this is part of the
# Bazel issue: https://github.com/bazelbuild/bazel/issues/7538
- cc_binary(
- name = name + "_impl",
- linkshared = True,
- linkstatic = True,
- **kwargs
- )
- binary_alias(
- name = name + ".so",
- binary = ":" + name + "_impl",
- )
- binary_alias(
- name = name + ".dll",
- binary = ":" + name + "_impl",
- )
- binary_alias(
- name = name + ".dylib",
- binary = ":" + name + "_impl",
- )
+ so_name = name + ".so"
+ dll_name = name + ".dll"
+ dylib_name = name + ".dylib"
+ interface_output_name = name + "_interface_output"
+ import_name = name + "_import"
+ for impl_name in [dll_name, dylib_name, so_name]:
+ cc_binary(
+ name = impl_name,
+ srcs = srcs + hdrs,
+ linkshared = True,
+ linkstatic = True,
+ features = features,
+ tags = ["manual"] + tags,
+ testonly = testonly,
+ **kwargs
+ )
native.filegroup(
- name = name,
+ name = interface_output_name,
srcs = select({
- "@bazel_tools//src/conditions:windows": [":" + name + ".dll"],
- "@bazel_tools//src/conditions:darwin": [":" + name + ".dylib"],
- "//conditions:default": [":" + name + ".so"],
+ "@bazel_tools//src/conditions:windows": [":" + dll_name],
+ "@bazel_tools//src/conditions:darwin": [":" + dylib_name],
+ "//conditions:default": [":" + so_name],
+ }),
+ output_group = "interface_library",
+ )
+ cc_import(
+ name = import_name,
+ interface_library = ":" + interface_output_name,
+ shared_library = select({
+ "@bazel_tools//src/conditions:windows": ":" + dll_name,
+ "@bazel_tools//src/conditions:darwin": ":" + dylib_name,
+ "//conditions:default": ":" + so_name,
}),
+ alwayslink = alwayslink,
+ features = features,
+ tags = tags,
+ testonly = testonly,
+ )
+ cc_library(
+ name = name,
+ hdrs = hdrs,
+ include_prefix = include_prefix,
+ strip_include_prefix = strip_include_prefix,
+ deps = [":" + import_name],
+ alwayslink = alwayslink,
+ features = features,
+ tags = tags,
+ testonly = testonly,
)
Index: utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
===================================================================
--- utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
@@ -479,8 +479,15 @@
) + [
"libclang/TestUtils.h",
],
+ args = select({
+ "@bazel_tools//src/conditions:windows": [
+ # Need to disable the VFS tests that don't use Windows friendly paths.
+ "--gtest_filter=-*VirtualFileOverlay*",
+ ],
+ "//conditions:default": [],
+ }),
deps = [
- "//clang:c-bindings",
+ "//clang:libclang",
"//llvm:Support",
"//llvm:gtest",
"//llvm:gtest_main",
Index: utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
===================================================================
--- utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
+++ utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
@@ -75,7 +75,7 @@
/* #undef CLANG_HAVE_LIBXML */
/* Define if we have sys/resource.h (rlimits) */
-#define CLANG_HAVE_RLIMITS 1
+/* CLANG_HAVE_RLIMITS defined in Bazel */
/* The LLVM product name and version */
#define BACKEND_PACKAGE_STRING "LLVM 12.0.0git"
Index: utils/bazel/llvm-project-overlay/clang/BUILD.bazel
===================================================================
--- utils/bazel/llvm-project-overlay/clang/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/clang/BUILD.bazel
@@ -357,6 +357,13 @@
"include/clang/Basic/Version.inc",
"include/clang/Config/config.h",
],
+ defines = select({
+ "@bazel_tools//src/conditions:windows": [],
+ "//conditions:default": [
+ # Clang-specific define on non-Windows platforms.
+ "CLANG_HAVE_RLIMITS=1",
+ ],
+ }),
includes = ["include"],
deps = [
# We rely on the LLVM config library to provide configuration defines.
@@ -1313,6 +1320,10 @@
# directly #including "Tools.h".
"lib/Driver",
],
+ linkopts = select({
+ "@bazel_tools//src/conditions:windows": ["version.lib"],
+ "//conditions:default": [],
+ }),
textual_hdrs = glob([
"include/clang/Driver/*.def",
]),
@@ -1732,12 +1743,13 @@
)
cc_library(
- name = "libclang_library",
+ name = "libclang_static",
srcs = glob([
"tools/libclang/*.cpp",
"tools/libclang/*.h",
]),
hdrs = glob(["include/clang-c/*.h"]),
+ defines = ["CINDEX_NO_EXPORTS"],
deps = [
":arc_migrate",
":ast",
@@ -1758,18 +1770,36 @@
],
)
-cc_library(
- name = "c-bindings",
+cc_plugin_library(
+ name = "libclang",
+ srcs = glob([
+ "tools/libclang/*.cpp",
+ "tools/libclang/*.h",
+ ]),
hdrs = glob(["include/clang-c/*.h"]),
+ copts = select({
+ "@bazel_tools//src/conditions:windows": ["-D_CINDEX_LIB_"],
+ "//conditions:default": [],
+ }),
+ strip_include_prefix = "include",
deps = [
- ":libclang_library",
+ ":arc_migrate",
+ ":ast",
+ ":basic",
+ ":codegen",
+ ":config",
+ ":driver",
+ ":frontend",
+ ":index",
+ ":lex",
+ ":rewrite",
+ ":sema",
+ ":tooling",
+ "//llvm:BitstreamReader",
+ "//llvm:FrontendOpenMP",
+ "//llvm:Support",
+ "//llvm:config",
],
- alwayslink = 1,
-)
-
-cc_plugin_library(
- name = "libclang",
- deps = [":c-bindings"],
)
filegroup(
@@ -1802,14 +1832,12 @@
deps = [
":ast",
":basic",
- ":c-bindings",
":codegen",
":config",
":frontend",
":index",
":lex",
- ":parse",
- ":sema",
+ ":libclang",
":serialization",
"//llvm:Core",
"//llvm:MC",
@@ -1837,11 +1865,14 @@
name = "c-arcmt-test",
testonly = 1,
srcs = ["tools/c-arcmt-test/c-arcmt-test.c"],
- copts = ["-std=gnu99"],
+ copts = select({
+ "@bazel_tools//src/conditions:windows": [],
+ "//conditions:default": ["-std=gnu99"],
+ }),
stamp = 0,
deps = [
- ":c-bindings",
":codegen",
+ ":libclang",
"//llvm:MC",
"//llvm:Support",
],
Index: utils/bazel/.bazelrc
===================================================================
--- utils/bazel/.bazelrc
+++ utils/bazel/.bazelrc
@@ -121,6 +121,10 @@
# There appears to be an unused constant in GoogleTest on Windows.
build:clang-cl --copt=/clang:-Wno-unused-const-variable --host_copt=/clang:-Wno-unused-const-variable
+# Disable some warnings hit even with `clang-cl` in Clang's own code.
+build:clang-cl --copt=/clang:-Wno-inconsistent-dllimport --host_copt=/clang:-Wno-inconsistent-dllimport
+build:clang-cl --cxxopt=/clang:-Wno-c++11-narrowing --host_cxxopt=/clang:-Wno-c++11-narrowing
+
###############################################################################
###############################################################################
Index: clang/include/clang/Basic/Builtins.def
===================================================================
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -1050,10 +1050,15 @@
LIBBUILTIN(wmemcpy, "w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
LIBBUILTIN(wmemmove,"w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
-// C99
-// In some systems setjmp is a macro that expands to _setjmp. We undefine
-// it here to avoid having two identical LIBBUILTIN entries.
+// In some systems, several of these are defined as macros that expand to other
+// names. We undefine them here to avoid having the wrong builtin names.
#undef setjmp
+#undef strdup
+#undef stricmp
+#undef strcasecmp
+#undef strncasecmp
+
+// C99
LIBBUILTIN(setjmp, "iJ", "fjT", "setjmp.h", ALL_LANGUAGES)
LIBBUILTIN(longjmp, "vJi", "frT", "setjmp.h", ALL_LANGUAGES)
@@ -1076,10 +1081,6 @@
LIBBUILTIN(rindex, "c*cC*i", "f", "strings.h", ALL_GNU_LANGUAGES)
LIBBUILTIN(bzero, "vv*z", "f", "strings.h", ALL_GNU_LANGUAGES)
LIBBUILTIN(bcmp, "ivC*vC*z", "f", "strings.h", ALL_GNU_LANGUAGES)
-// In some systems str[n]casejmp is a macro that expands to _str[n]icmp.
-// We undefine then here to avoid wrong name.
-#undef strcasecmp
-#undef strncasecmp
LIBBUILTIN(strcasecmp, "icC*cC*", "f", "strings.h", ALL_GNU_LANGUAGES)
LIBBUILTIN(strncasecmp, "icC*cC*z", "f", "strings.h", ALL_GNU_LANGUAGES)
// POSIX unistd.h
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
