Skip to content

Commit

Permalink
[Bazel] Get //clang building on Windows with clang-cl.
Browse files Browse the repository at this point in the history 
This required substantially more invasive changes.

We need to handle some of the LLVM `config.h` changes differently from
the old pattern. These aren't always safe on the commandline, and the
Windows ones specifically break Clang. Instead, use conditional defines
in the header itself. This more closely matches how CMake builds see the
definitions. I think this is also just cleaner and we should maybe move
more of the macros out of Bazel.

The config defines for Windows that I've kept in Bazel are the ones that
LLVM's CMake does at the commandline as well. I've also added numerous
ones that CMake uses and we didn't replicate in Bazel.

I also 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.

Sadly, some tests also 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.

I also have cleaned up the Windows flags in `.bazelrc` to much more
closely match what CMake does.

Differential Revision: https://reviews.llvm.org/D112399
  • Loading branch information
@chandlerc
chandlerc committed Nov 2, 2021
1 parent a9a8952 commit 0198d76
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 64 deletions.
19 changes: 14 additions & 5 deletions utils/bazel/.bazelrc
View file
Expand Up @@ -72,8 +72,16 @@ build:generic_gcc --copt=-Werror --host_copt=-Werror
# Generic Windows flags common to both MSVC and Clang.
###############################################################################

# Yay for security warnings. Boo for non-standard.
build:windows --copt=/D_CRT_SECURE_NO_WARNINGS --host_copt=/D_CRT_SECURE_NO_WARNINGS
# C++14 standard version is required.
build:windows --cxxopt=/std:c++14 --host_cxxopt=/std:c++14

# Other generic dialect flags.
build:windows --copt=/Zc:strictStrings --host_copt=/Zc:strictStrings
build:windows --copt=/Oi --host_copt=/Oi
build:windows --cxxopt=/Zc:rvalueCast --host_cxxopt=/Zc:rvalueCast

# Use the more flexible bigobj format for C++ files that have lots of symbols.
build:windows --cxxopt=/bigobj --host_cxxopt=/bigobj

###############################################################################
# Windows specific flags for building with MSVC.
Expand Down Expand Up @@ -107,9 +115,6 @@ build:clang-cl --config=windows
# Switch from MSVC to the `clang-cl` compiler.
build:clang-cl --compiler=clang-cl

# C++14 standard version is required.
build:clang-cl --cxxopt=/std:c++14 --host_cxxopt=/std:c++14

# Use Clang's internal warning flags instead of the ones that sometimes map
# through to MSVC's flags.
build:clang-cl --copt=/clang:-Wall --host_copt=/clang:-Wall
Expand All @@ -121,6 +126,10 @@ build:clang-cl --copt=/clang:-Wno-unused --host_copt=/clang:-Wno-unused
# 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

###############################################################################

###############################################################################
Expand Down
54 changes: 39 additions & 15 deletions utils/bazel/llvm-project-overlay/clang/BUILD.bazel
View file
Expand Up @@ -1322,6 +1322,10 @@ cc_library(
# directly #including "Tools.h".
"lib/Driver",
],
linkopts = select({
"@bazel_tools//src/conditions:windows": ["version.lib"],
"//conditions:default": [],
}),
textual_hdrs = glob([
"include/clang/Driver/*.def",
]),
Expand Down Expand Up @@ -1741,12 +1745,13 @@ cc_library(
)

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",
Expand All @@ -1767,18 +1772,36 @@ cc_library(
],
)

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(
Expand Down Expand Up @@ -1811,14 +1834,12 @@ cc_binary(
deps = [
":ast",
":basic",
":c-bindings",
":codegen",
":config",
":frontend",
":index",
":lex",
":parse",
":sema",
":libclang",
":serialization",
"//llvm:Core",
"//llvm:MC",
Expand Down Expand Up @@ -1846,11 +1867,14 @@ cc_binary(
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",
],
Expand Down
12 changes: 11 additions & 1 deletion utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
View file
Expand Up @@ -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 conditionally below */

/* The LLVM product name and version */
#define BACKEND_PACKAGE_STRING "LLVM 12.0.0git"
Expand All @@ -100,4 +100,14 @@
/* Spawn a new process clang.exe for the CC1 tool invocation, when necessary */
#define CLANG_SPAWN_CC1 0

/* Directly provide definitions here behind platform preprocessor definitions.
* The preprocessor conditions are sufficient to handle all of the configuration
* on platforms targeted by Bazel, and defining these here more faithfully
* matches how the users of this header expect things to work with CMake.
*/

#ifndef _WIN32
#define CLANG_HAVE_RLIMITS 1
#endif

#endif
10 changes: 9 additions & 1 deletion utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
View file
Expand Up @@ -480,8 +480,16 @@ cc_test(
) + [
"libclang/TestUtils.h",
],
args = select({
"@bazel_tools//src/conditions:windows": [
# Need to disable the VFS tests that don't use Windows friendly
# paths. These are also disabled on Windows in the CMake build.
"--gtest_filter=-*VirtualFileOverlay*",
],
"//conditions:default": [],
}),
deps = [
"//clang:c-bindings",
"//clang:libclang",
"//llvm:Support",
"//llvm:gtest",
"//llvm:gtest_main",
Expand Down
95 changes: 58 additions & 37 deletions utils/bazel/llvm-project-overlay/llvm/cc_plugin_library.bzl
View file
Expand Up @@ -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,
)
12 changes: 9 additions & 3 deletions utils/bazel/llvm-project-overlay/llvm/config.bzl
View file
Expand Up @@ -57,9 +57,15 @@ macos_defines = posix_defines + [
]

win32_defines = [
# MSVC specific
"stricmp=_stricmp",
"strdup=_strdup",
# Windows system library specific defines.
"_CRT_SECURE_NO_DEPRECATE",
"_CRT_SECURE_NO_WARNINGS",
"_CRT_NONSTDC_NO_DEPRECATE",
"_CRT_NONSTDC_NO_WARNINGS",
"_SCL_SECURE_NO_DEPRECATE",
"_SCL_SECURE_NO_WARNINGS",
"UNICODE",
"_UNICODE",

# LLVM features
r'LTDL_SHLIB_EXT=\".dll\"',
Expand Down
17 changes: 15 additions & 2 deletions utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
View file
Expand Up @@ -352,10 +352,10 @@
#define HAVE_STD_IS_TRIVIALLY_COPYABLE 1

/* Define to a function implementing stricmp */
/* stricmp defined in Bazel */
/* stricmp defined conditionally below. */

/* Define to a function implementing strdup */
/* strdup defined in Bazel */
/* strdup defined conditionally below. */

/* Whether GlobalISel rule coverage is being collected */
#define LLVM_GISEL_COV_ENABLED 0
Expand All @@ -368,4 +368,17 @@

/* HAVE_PROC_PID_RUSAGE defined in Bazel */

/* Directly provide definitions here behind platform preprocessor definitions.
* The preprocessor conditions are sufficient to handle all of the configuration
* on platforms targeted by Bazel, and defining these here more faithfully
* matches how the users of this header expect things to work with CMake.
* FIXME: We should consider moving other platform defines to use this technique
* as well.
*/

#ifdef _WIN32
#define stricmp _stricmp
#define strdup _strdup
#endif

#endif

0 comments on commit 0198d76

Please sign in to comment.