[libc][bazel] Add bazel targets for libc/include/... tests. #141150
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-libc Author: None (jtstogel) ChangesThis PR also sets Full diff: https://github.com/llvm/llvm-project/pull/141150.diff 4 Files Affected:
diff --git a/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
index b37ec19330236..91820536d55f8 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
@@ -71,6 +71,7 @@ libc_test_library(
"//libc:llvm_libc_macros_stdfix_macros",
"//llvm:Support",
],
+ alwayslink = True,
)
libc_test_library(
diff --git a/utils/bazel/llvm-project-overlay/libc/test/include/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/include/BUILD.bazel
index 51e8424282af0..0740b4c30334f 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/include/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/include/BUILD.bazel
@@ -4,19 +4,315 @@
# Tests for LLVM libc public headers.
-load("//libc/test:libc_test_rules.bzl", "libc_test")
-
-package(default_visibility = ["//visibility:public"])
+load("//libc/test:libc_test_rules.bzl", "libc_c_test", "libc_test", "libc_test_library")
licenses(["notice"])
+libc_test(
+ name = "assert_test",
+ srcs = ["assert_test.cpp"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test(
+ name = "complex_test",
+ srcs = ["complex_test.cpp"],
+ deps = [
+ "//libc:public_headers_deps",
+ "//libc/test/UnitTest:fp_test_helpers",
+ ],
+)
+
+libc_test_library(
+ name = "fpclassify_test_fixture",
+ hdrs = ["FpClassifyTest.h"],
+ deps = ["//libc/test/UnitTest:fp_test_helpers"],
+)
+
+libc_test(
+ name = "fpclassify_test",
+ srcs = ["fpclassify_test.cpp"],
+ deps = [
+ ":fpclassify_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "fpclassifyf_test",
+ srcs = ["fpclassifyf_test.cpp"],
+ deps = [
+ ":fpclassify_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "fpclassifyl_test",
+ srcs = ["fpclassifyl_test.cpp"],
+ deps = [
+ ":fpclassify_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_c_test(
+ name = "fpclassify_c_test",
+ srcs = ["fpclassify_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test_library(
+ name = "isfinite_test_ficture",
+ hdrs = ["IsFiniteTest.h"],
+ deps = ["//libc/test/UnitTest:fp_test_helpers"],
+)
+
+libc_test(
+ name = "isfinite_test",
+ srcs = ["isfinite_test.cpp"],
+ deps = [
+ ":isfinite_test_ficture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isfinitef_test",
+ srcs = ["isfinitef_test.cpp"],
+ deps = [
+ ":isfinite_test_ficture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isfinitel_test",
+ srcs = ["isfinitel_test.cpp"],
+ deps = [
+ ":isfinite_test_ficture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_c_test(
+ name = "isfinite_c_test",
+ srcs = ["isfinite_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test_library(
+ name = "isinf_test_fixture",
+ hdrs = ["IsInfTest.h"],
+ deps = ["//libc/test/UnitTest:fp_test_helpers"],
+)
+
+libc_test(
+ name = "isinf_test",
+ srcs = ["isinf_test.cpp"],
+ deps = [
+ ":isinf_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isinff_test",
+ srcs = ["isinff_test.cpp"],
+ deps = [
+ ":isinf_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isinfl_test",
+ srcs = ["isinfl_test.cpp"],
+ deps = [
+ ":isinf_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_c_test(
+ name = "isinf_c_test",
+ srcs = ["isinf_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test_library(
+ name = "isnan_test_fixture",
+ hdrs = ["IsNanTest.h"],
+ deps = ["//libc/test/UnitTest:fp_test_helpers"],
+)
+
+libc_test(
+ name = "isnan_test",
+ srcs = ["isnan_test.cpp"],
+ deps = [
+ ":isnan_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isnanf_test",
+ srcs = ["isnanf_test.cpp"],
+ deps = [
+ ":isnan_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isnanl_test",
+ srcs = ["isnanl_test.cpp"],
+ deps = [
+ ":isnan_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_c_test(
+ name = "isnan_c_test",
+ srcs = ["isnan_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test_library(
+ name = "isnormal_test_fixture",
+ hdrs = ["IsNormalTest.h"],
+ deps = ["//libc/test/UnitTest:fp_test_helpers"],
+)
+
+libc_test(
+ name = "isnormal_test",
+ srcs = ["isnormal_test.cpp"],
+ deps = [
+ ":isnormal_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isnormalf_test",
+ srcs = ["isnormalf_test.cpp"],
+ deps = [
+ ":isnormal_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "isnormall_test",
+ srcs = ["isnormall_test.cpp"],
+ deps = [
+ ":isnormal_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_c_test(
+ name = "isnormal_c_test",
+ srcs = ["isnormal_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_c_test(
+ name = "issubnormal_c_test",
+ srcs = ["issubnormal_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test_library(
+ name = "iszero_test_fixture",
+ hdrs = ["IsZeroTest.h"],
+ deps = ["//libc/test/UnitTest:fp_test_helpers"],
+)
+
+libc_test(
+ name = "iszero_test",
+ srcs = ["iszero_test.cpp"],
+ deps = [
+ ":iszero_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "iszerof_test",
+ srcs = ["iszerof_test.cpp"],
+ deps = [
+ ":iszero_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "iszerol_test",
+ srcs = ["iszerol_test.cpp"],
+ deps = [
+ ":iszero_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_c_test(
+ name = "iszero_c_test",
+ srcs = ["iszero_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test_library(
+ name = "signbit_test_fixture",
+ hdrs = ["SignbitTest.h"],
+ deps = ["//libc/test/UnitTest:fp_test_helpers"],
+)
+
+libc_test(
+ name = "signbit_test",
+ srcs = ["signbit_test.cpp"],
+ deps = [
+ ":signbit_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "signbitf_test",
+ srcs = ["signbitf_test.cpp"],
+ deps = [
+ ":signbit_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
+libc_test(
+ name = "signbitl_test",
+ srcs = ["signbitl_test.cpp"],
+ deps = [
+ ":signbit_test_fixture",
+ "//libc:public_headers_deps",
+ ],
+)
+
libc_test(
name = "stdbit_test",
srcs = [
"stdbit_stub.h",
"stdbit_test.cpp",
],
- deps = [
- "//libc:public_headers_deps",
- ],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_c_test(
+ name = "signbit_c_test",
+ srcs = ["signbit_test.c"],
+ deps = ["//libc:public_headers_deps"],
+)
+
+libc_test(
+ name = "stdckdint_test",
+ srcs = ["stdckdint_test.cpp"],
+ deps = ["//libc:public_headers_deps"],
)
diff --git a/utils/bazel/llvm-project-overlay/libc/test/include/sys/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/include/sys/BUILD.bazel
new file mode 100644
index 0000000000000..2ad56b00e8686
--- /dev/null
+++ b/utils/bazel/llvm-project-overlay/libc/test/include/sys/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
+
+# Tests for LLVM libc public headers.
+
+load("//libc/test:libc_test_rules.bzl", "libc_test")
+
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+libc_test(
+ name = "queue_test",
+ srcs = ["queue_test.cpp"],
+ deps = [
+ "//libc:__support_char_vector",
+ "//libc:__support_cpp_string",
+ "//libc:public_headers_deps",
+ ],
+)
diff --git a/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl b/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl
index 123e05727aeff..60933fcffcf8c 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl
@@ -12,10 +12,18 @@ They come in two flavors:
When performing tests we make sure to always use the internal version.
"""
+load("@rules_cc//cc:cc_library.bzl", "cc_library")
+load("@rules_cc//cc:cc_test.bzl", "cc_test")
load("//libc:libc_build_rules.bzl", "libc_common_copts")
load("//libc:libc_configure_options.bzl", "LIBC_CONFIGURE_OPTIONS")
-def libc_test(name, copts = [], deps = [], local_defines = [], **kwargs):
+def _libc_test(
+ name,
+ copts = [],
+ deps = [],
+ local_defines = [],
+ use_test_framework = True,
+ **kwargs):
"""Add target for a libc test.
Args:
@@ -23,25 +31,37 @@ def libc_test(name, copts = [], deps = [], local_defines = [], **kwargs):
copts: The list of options to add to the C++ compilation command.
deps: The list of libc functions and libraries to be linked in.
local_defines: The list of target local_defines if any.
+ use_test_framework: Whether the unit test has a custom `main` function.
**kwargs: Attributes relevant for a cc_test.
"""
- native.cc_test(
+ deps = deps + [
+ "//libc:__support_macros_config",
+ "//libc:errno",
+ "//libc:func_aligned_alloc",
+ "//libc:func_free",
+ "//libc:func_malloc",
+ "//libc:func_realloc",
+ ]
+ if use_test_framework:
+ deps = deps + ["//libc/test/UnitTest:LibcUnitTest"]
+
+ cc_test(
name = name,
local_defines = local_defines + LIBC_CONFIGURE_OPTIONS,
- deps = [
- "//libc/test/UnitTest:LibcUnitTest",
- "//libc:__support_macros_config",
- "//libc:errno",
- "//libc:func_aligned_alloc",
- "//libc:func_free",
- "//libc:func_malloc",
- "//libc:func_realloc",
- ] + deps,
+ deps = deps,
copts = copts + libc_common_copts(),
linkstatic = 1,
**kwargs
)
+def libc_test(name, **kwargs):
+ """Add a target for a libc configured for libc."""
+ _libc_test(name, **kwargs)
+
+def libc_c_test(name, **kwargs):
+ """Add a target for a libc test in C that does not use a test framework."""
+ _libc_test(name, use_test_framework = False, **kwargs)
+
def libc_test_library(name, copts = [], local_defines = [], **kwargs):
"""Add target for library used in libc tests.
@@ -51,7 +71,7 @@ def libc_test_library(name, copts = [], local_defines = [], **kwargs):
local_defines: See cc_library.local_defines.
**kwargs: Other attributes relevant to cc_library (e.g. "deps").
"""
- native.cc_library(
+ cc_library(
name = name,
testonly = True,
copts = copts + libc_common_copts(),
|
jtstogel
changed the title
| @@ -71,6 +71,7 @@ libc_test_library( | |||
| "//libc:llvm_libc_macros_stdfix_macros", | |||
| "//llvm:Support", | |||
| ], | |||
| alwayslink = True, | |||
There was a problem hiding this comment.
What would happen if you try to link in this library to a test which already has main() before this change?
There was a problem hiding this comment.
It'll fail to link with duplicate symbol: main. In the case that a test has it's own main(), I figured it's least surprising to be explicit about which main() a program is using rather than falling back to the unit test framework's main() if there's e.g. a typo somewhere in the test or the test's main() is namespaced or something.
Happy to revert if you think it's unnecessary or there's another reason to avoid it!
There was a problem hiding this comment.
So, both with and without this change the link would fail with duplicate symbol: main if the unit test which has its own main() would depend on //libc/test/UnitTest:LibcUnitTest. Could you clarify which behavior are you trying to change?
E.g. if a unit test accidentally has its int main() wrapped in a namespace, then both with and without this change the test will instead pick up the main() from the framework, won't it?
There was a problem hiding this comment.
So, both with and without this change the link would fail with duplicate symbol: main if the unit test which has its own
main()would depend on//libc/test/UnitTest:LibcUnitTest.
I could be misconfigured on my machine, but AFAICT this isn't true?
Here's some code examples: main...jtstogel:llvm-project:linker-opt-example. My understanding (which may be incorrect) is that the linker won't include the object file unless a symbol is used, so if the test has it's own main(), the unit test framework main will just be ignored.
E.g. if a unit test accidentally has its int main() wrapped in a namespace, then both with and without this change the test will instead pick up the main() from the framework, won't it?
Sorry that example wasn't well formed. I think a better argument for why files like these should set alwayslink=True is that the program's semantics falls down to link order. In the following example, what does the test do? I would expect it to immediately fail with duplicate symbol: main, but on my machine the test either fails or passes based on the ordering of [":main1", ":main2"].
cc_test(
name = "example_test",
srcs = ["example_test.cc"],
deps = [":main1", ":main2"]
)
cc_library(name = "main1", srcs = ["main1.cc"])
cc_library(name = "main2", srcs = ["main2.cc"])
main1.cc
int main(void) { return 1; }
main2.cc
int main(void) { return 0; }
example_test.cc
// empty
There was a problem hiding this comment.
At the end of the day, this line of the change doesn't matter so much -- I only included it here since it seemed marginally better and non-controvertial. Happy to remove it if you feel it's worse for whatever reason.
There was a problem hiding this comment.
Thanks for the detailed explanation, I agree that this change is reasonable. Would you mind adding a comment above alwayslink = True to specify that we deliberately force linking in library to ensure the test case doesn't provide their own main()?
utils/bazel/llvm-project-overlay/libc/test/include/sys/BUILD.bazel
Outdated
Show resolved
Hide resolved
jtstogel
force-pushed
the
add-include-tests
branch
from
ea15c9e to
6638a34
Compare
| @@ -71,6 +71,7 @@ libc_test_library( | |||
| "//libc:llvm_libc_macros_stdfix_macros", | |||
| "//llvm:Support", | |||
| ], | |||
| alwayslink = True, | |||
There was a problem hiding this comment.
So, both with and without this change the link would fail with duplicate symbol: main if the unit test which has its own main() would depend on //libc/test/UnitTest:LibcUnitTest. Could you clarify which behavior are you trying to change?
E.g. if a unit test accidentally has its int main() wrapped in a namespace, then both with and without this change the test will instead pick up the main() from the framework, won't it?
| @@ -71,6 +71,7 @@ libc_test_library( | |||
| "//libc:llvm_libc_macros_stdfix_macros", | |||
| "//llvm:Support", | |||
| ], | |||
| alwayslink = True, | |||
There was a problem hiding this comment.
Thanks for the detailed explanation, I agree that this change is reasonable. Would you mind adding a comment above alwayslink = True to specify that we deliberately force linking in library to ensure the test case doesn't provide their own main()?
jtstogel
force-pushed
the
add-include-tests
branch
from
6638a34 to
b53f71b
Compare
Also, set `alwayslink=True` for `//libc/test:LibcUnitTest`. This ensures that the `main` function provided always gets linked into a test target. While not strictly necessary, it makes it so tests like https://github.com/llvm/llvm-project/blob/45d8759cbed0f216786729718608a8be72a505c6/libc/test/include/signbit_test.c will give a duplicate symbol error if they incorrectly depend on `LibcUnitTest`.
jtstogel
force-pushed
the
add-include-tests
branch
from
b53f71b to
7dddbb8
Compare
This PR also sets
alwayslink=Truefor//libc/test:LibcUnitTest. This ensures that themainfunction provided always gets linked into a test target. While not strictly necessary, it makes it so tests like https://github.com/llvm/llvm-project/blob/45d8759cbed0f216786729718608a8be72a505c6/libc/test/include/signbit_test.c will give a duplicate symbol error if they incorrectly depend on//libc/test:LibcUnitTest.This PR is missing tests for generated header includes since the current Bazel setup lacks generated headers or a mechanism to run hermetic tests. CMake version of the header include tests:
llvm-project/libc/test/include/CMakeLists.txt
Line 515 in a2ce564
See issue #134780