Test host-only code with cross-compilation.
Since Crubit itself only is expected to run on x86 (right now?), cross-compiling to arm, it makes sense to do the same in the unit tests. This makes the unit tests more like production, and avoids needing to actively work to get the rest of the internals working on arm. It's possible, but nah.
This was constructed by parameterizing the tests using an environment variable. This was easier than parameterizing the tests using loops, rstest, etc., since those would require adding each parameter to each function, when really we want to do ~all parameters for these (presumably).
This testing strategy only really works because both arm-linux and x86-linux use the Itanium ABI, which is effectively identical. If/when we add non-Itanium support, we'd need to add separate files, and restrict the tests to e.g. only run on Itanium platforms.
There is one major integration-y test that doesn't cross-compile for Arm, generate_bindings_and_metadata_test, but I think it's of relatively low value given the remaining "unit" tests and the integration tests. I'm fairly comfortable saying we test Arm -- we may even test it _too_ much, given how similar the two platforms are. We can mark the bug as fixed: Arm is tested and works.
PiperOrigin-RevId: 527324587
diff --git a/common/BUILD b/common/BUILD
index ec93fe3..fee1ad5 100644
--- a/common/BUILD
+++ b/common/BUILD
@@ -1,5 +1,5 @@
# Common libraries used in multiple Crubit tools.
-
+load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load(
"@rules_rust//rust:defs.bzl",
"rust_library",
@@ -73,6 +73,26 @@
],
)
+bzl_library(
+ name = "multiplatform_bzl",
+ srcs = ["multiplatform.bzl"],
+ visibility = [
+ "//:__subpackages__",
+ ],
+)
+
+rust_library(
+ name = "multiplatform_testing",
+ testonly = 1,
+ srcs = ["multiplatform_testing.rs"],
+ visibility = [
+ "//:__subpackages__",
+ ],
+ deps = [
+ "@crate_index//:once_cell",
+ ],
+)
+
rust_library(
name = "ffi_types",
srcs = ["ffi_types.rs"],
diff --git a/common/multiplatform_testing.bzl b/common/multiplatform_testing.bzl
new file mode 100644
index 0000000..5f49677
--- /dev/null
+++ b/common/multiplatform_testing.bzl
@@ -0,0 +1,29 @@
+# Part of the Crubit project, under the Apache License v2.0 with LLVM
+# Exceptions. See /LICENSE for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+"""Supporting macro for multiplatform code."""
+
+load("@rules_rust//rust:defs.bzl", "rust_test")
+
+_PLATFORMS = [
+ "x86_linux",
+ "arm_linux",
+]
+
+def multiplatform_rust_test(name, **kwargs):
+ """Macro to parameterize a test target by target platform."""
+
+ # TODO(jeanpierreda): Ideally we'd use `.`, not `-`, but this breaks for non-crate= rust_test targets
+ # because they create a crate with `.` in the name. That's illegal.
+ native.test_suite(
+ name = name,
+ tests = [name + "-" + platform for platform in _PLATFORMS],
+ )
+ rustc_env = kwargs.setdefault("env", {})
+ for platform in _PLATFORMS:
+ rustc_env["CRUBIT_TEST_PLATFORM"] = platform
+ test_name = name + "-" + platform
+ rust_test(
+ name = test_name,
+ **kwargs
+ )
diff --git a/common/multiplatform_testing.rs b/common/multiplatform_testing.rs
new file mode 100644
index 0000000..7fa559e
--- /dev/null
+++ b/common/multiplatform_testing.rs
@@ -0,0 +1,40 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+//! Vocabulary library for multi-platform tests which use cross-compilation.
+
+use once_cell::sync::Lazy;
+
+#[non_exhaustive]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+pub enum Platform {
+ X86Linux,
+ ArmLinux,
+}
+
+impl Platform {
+ pub fn target_triple(self) -> &'static str {
+ match self {
+ Platform::X86Linux => "x86_64-grtev4-linux-gnu",
+ Platform::ArmLinux => "aarch64-grtev4-linux-gnu",
+ }
+ }
+}
+
+/// Returns the platform the current test is running for with
+/// multiplatform_rust_test.
+pub fn test_platform() -> Platform {
+ *TEST_PLATFORM.as_ref().unwrap()
+}
+
+static TEST_PLATFORM: Lazy<Result<Platform, String>> = Lazy::new(|| {
+ let env = std::env::var("CRUBIT_TEST_PLATFORM")
+ .map_err(|_| "multiplatform tests must use `multiplatform_rust_test`.".to_string())?;
+ let platform = match env.as_str() {
+ "x86_linux" => Platform::X86Linux,
+ "arm_linux" => Platform::ArmLinux,
+ _ => return Err(format!("Unknown platform: {env}")),
+ };
+ Ok(platform)
+});
diff --git a/rs_bindings_from_cc/BUILD b/rs_bindings_from_cc/BUILD
index a735c79..dc06dc5 100644
--- a/rs_bindings_from_cc/BUILD
+++ b/rs_bindings_from_cc/BUILD
@@ -17,6 +17,10 @@
"rust_library",
"rust_test",
)
+load(
+ "//common:multiplatform_testing.bzl",
+ "multiplatform_rust_test",
+)
package(default_applicable_licenses = ["//:license"])
@@ -105,8 +109,9 @@
data = [
],
tags = [
- # TODO(b/269144587): parameterize the tests to test arm behavior in cross-compilation.
- "not_run:arm", # We don't need to run Crubit itself on aarch64.
+ # This test uses e.g. rustfmt and is difficult to run on ARM. We could run it using
+ # cross-compilation, but it's mostly uninteresting from a multi-platform POV.
+ "not_run:arm",
],
deps = [
":cc_ir",
@@ -337,6 +342,7 @@
":json_from_cc",
"//common:arc_anyhow",
"//common:ffi_types",
+ "//common:multiplatform_testing",
"@crate_index//:flagset",
"@crate_index//:itertools",
"@crate_index//:once_cell",
@@ -391,18 +397,16 @@
],
)
-rust_test(
+multiplatform_rust_test(
name = "ir_from_cc_test",
srcs = ["ir_from_cc_test.rs"],
- tags = [
- # TODO(b/269144587): parameterize the tests to test arm behavior in cross-compilation.
- "not_run:arm", # We don't need to run Crubit itself on aarch64.
- ],
+ tags = ["not_run:arm"],
deps = [
":ir",
":ir_matchers",
":ir_testing",
"//common:arc_anyhow",
+ "//common:multiplatform_testing",
"//common:rust_allocator_shims",
"@crate_index//:itertools",
"@crate_index//:proc-macro2",
@@ -447,16 +451,14 @@
],
)
-rust_test(
+multiplatform_rust_test(
name = "src_code_gen_impl_test",
crate = ":src_code_gen_impl",
- tags = [
- # TODO(b/269144587): parameterize the tests to test arm behavior in cross-compilation.
- "not_run:arm", # We don't need to run Crubit itself on aarch64.
- ],
+ tags = ["not_run:arm"],
deps = [
":ir_matchers",
":ir_testing",
+ "//common:multiplatform_testing",
"//common:rust_allocator_shims",
"//common:token_stream_matchers",
"@crate_index//:static_assertions",
@@ -520,6 +522,8 @@
"not_run:arm", # We don't need to run Crubit itself on aarch64.
],
deps = [
+ "//common:arc_anyhow",
+ "//common:multiplatform_testing",
"//common:rust_allocator_shims",
],
)
diff --git a/rs_bindings_from_cc/ir_from_cc_test.rs b/rs_bindings_from_cc/ir_from_cc_test.rs
index 7f9e6f4..33bfabd 100644
--- a/rs_bindings_from_cc/ir_from_cc_test.rs
+++ b/rs_bindings_from_cc/ir_from_cc_test.rs
@@ -6,12 +6,20 @@
use arc_anyhow::Result;
use ir::*;
use ir_matchers::{assert_ir_matches, assert_ir_not_matches, assert_items_match};
-use ir_testing::*;
+use ir_testing::{ir_id, retrieve_func, retrieve_record};
use itertools::Itertools;
use quote::quote;
use std::collections::{HashMap, HashSet};
use std::iter::Iterator;
+fn ir_from_cc(header: &str) -> Result<IR> {
+ ir_testing::ir_from_cc(multiplatform_testing::test_platform(), header)
+}
+
+fn ir_from_cc_dependency(header: &str, dep_header: &str) -> Result<IR> {
+ ir_testing::ir_from_cc_dependency(multiplatform_testing::test_platform(), header, dep_header)
+}
+
#[test]
fn test_function() {
let ir = ir_from_cc("int f(int a, int b);").unwrap();
@@ -243,9 +251,11 @@
);
}
-#[cfg(target_arch = "x86_64")] // vectorcall only exists on x86_64, not e.g. aarch64
#[test]
fn test_function_with_custom_calling_convention() {
+ if multiplatform_testing::test_platform() != multiplatform_testing::Platform::X86Linux {
+ return; // vectorcall only exists on x86_64, not e.g. aarch64
+ }
let ir = ir_from_cc("int f_vectorcall(int, int) [[clang::vectorcall]];").unwrap();
assert_ir_matches!(
ir,
@@ -898,10 +908,11 @@
assert_eq!(type_mapping["bool"], "bool");
// TODO(b/276790180, b/276931370): use c_char instead.
- #[cfg(target_arch = "aarch64")]
- assert_eq!(type_mapping["char"], "u8");
- #[cfg(not(target_arch = "aarch64"))]
- assert_eq!(type_mapping["char"], "i8");
+ if multiplatform_testing::test_platform() == multiplatform_testing::Platform::X86Linux {
+ assert_eq!(type_mapping["char"], "i8");
+ } else {
+ assert_eq!(type_mapping["char"], "u8");
+ }
assert_eq!(type_mapping["unsigned char"], "u8");
assert_eq!(type_mapping["signed char"], "i8");
diff --git a/rs_bindings_from_cc/ir_matchers.rs b/rs_bindings_from_cc/ir_matchers.rs
index 423ddf2..f0cd616 100644
--- a/rs_bindings_from_cc/ir_matchers.rs
+++ b/rs_bindings_from_cc/ir_matchers.rs
@@ -2,13 +2,14 @@
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-/// Like `token_stream_matchers::assert_cc_matches!`, but expects `IR` instance as input. The macro
-/// converts the instance to its corresponding struct expression and matches the pattern against
-/// that. See the documentation of `token_stream_matchers` for more information.
+/// Like `token_stream_matchers::assert_cc_matches!`, but expects `IR` instance
+/// as input. The macro converts the instance to its corresponding struct
+/// expression and matches the pattern against that. See the documentation of
+/// `token_stream_matchers` for more information.
///
/// Example:
/// ```rust
-/// let ir = ir_from_cc("struct SomeStruct {};').unwrap();
+/// let ir = ir_from_cc(..., "struct SomeStruct {};').unwrap();
/// assert_ir_matches!(
/// ir,
/// quote! {
@@ -45,12 +46,12 @@
macro_rules! assert_items_match {
($items:expr, $patterns:expr $(,)*) => {
assert_eq!($items.len(), $patterns.len());
- for (idx, (item, pattern)) in $items.into_iter().zip($patterns).enumerate() {
- $crate::internal::match_item(&item, &pattern).expect(&format!(
- "input at position {} unexpectedly didn't match the pattern",
- &idx
- ));
- }
+ for (idx, (item, pattern)) in $items.into_iter().zip($patterns).enumerate() {
+ $crate::internal::match_item(&item, &pattern).expect(&format!(
+ "input at position {} unexpectedly didn't match the pattern",
+ &idx
+ ));
+ }
};
}
@@ -129,9 +130,13 @@
#[cfg(test)]
mod tests {
use super::*;
- use ir_testing::ir_from_cc;
use quote::quote;
+ /// We aren't testing platform-specific details, just the matchers.
+ fn ir_from_cc(header: &str) -> arc_anyhow::Result<ir::IR> {
+ ir_testing::ir_from_cc(multiplatform_testing::Platform::X86Linux, header)
+ }
+
#[test]
fn test_optional_trailing_comma() {
assert_ir_matches!(ir_from_cc("").unwrap(), quote! { FlatIR { ... }});
@@ -143,10 +148,7 @@
#[test]
fn test_assert_ir_matches_assumes_trailing_commas_in_groups() {
- assert_ir_matches!(
- ir_from_cc("").unwrap(),
- quote! {{... , }}
- );
+ assert_ir_matches!(ir_from_cc("").unwrap(), quote! {{... , }});
}
#[test]
diff --git a/rs_bindings_from_cc/ir_testing.rs b/rs_bindings_from_cc/ir_testing.rs
index 4816b3a..87e7bd3 100644
--- a/rs_bindings_from_cc/ir_testing.rs
+++ b/rs_bindings_from_cc/ir_testing.rs
@@ -12,8 +12,8 @@
use ir::{self, make_ir_from_parts, Func, Identifier, Item, Record, IR};
/// Generates `IR` from a header containing `header_source`.
-pub fn ir_from_cc(header_source: &str) -> Result<IR> {
- ir_from_cc_dependency(header_source, "// empty header")
+pub fn ir_from_cc(platform: multiplatform_testing::Platform, header_source: &str) -> Result<IR> {
+ ir_from_cc_dependency(platform, header_source, "// empty header")
}
/// Prepends definitions for lifetime annotation macros to the code.
@@ -70,11 +70,16 @@
/// `header_source` of the header will be updated to contain the `#include` line
/// for the header with `dependency_header_source`. The name of the dependency
/// target is exposed as `DEPENDENCY_TARGET`.
-pub fn ir_from_cc_dependency(header_source: &str, dependency_header_source: &str) -> Result<IR> {
+pub fn ir_from_cc_dependency(
+ platform: multiplatform_testing::Platform,
+ header_source: &str,
+ dependency_header_source: &str,
+) -> Result<IR> {
const DEPENDENCY_HEADER_NAME: &str = "test/dependency_header.h";
extern "C" {
fn json_from_cc_dependency(
+ target_triple: FfiU8Slice,
header_source: FfiU8Slice,
dependency_header_source: FfiU8Slice,
) -> FfiU8SliceBox;
@@ -86,6 +91,7 @@
let dependency_header_source_u8 = dependency_header_source.as_bytes();
let json_utf8 = unsafe {
json_from_cc_dependency(
+ FfiU8Slice::from_slice(platform.target_triple().as_ref()),
FfiU8Slice::from_slice(header_source_with_include_u8),
FfiU8Slice::from_slice(dependency_header_source_u8),
)
@@ -102,8 +108,8 @@
}
/// Creates a simple `Item::Record` with a given name.
-pub fn ir_record(name: &str) -> Record {
- let ir = ir_from_cc("struct REPLACEME final {};").unwrap();
+pub fn ir_record(platform: multiplatform_testing::Platform, name: &str) -> Record {
+ let ir = ir_from_cc(platform, "struct REPLACEME final {};").unwrap();
for item in ir.items() {
if let Item::Record(record) = item {
let mut record = (**record).clone();
@@ -146,7 +152,7 @@
#[test]
fn test_features_ir_from_cc() -> Result<()> {
assert_ir_matches!(
- ir_from_cc("")?,
+ ir_from_cc(multiplatform_testing::Platform::X86Linux, "")?,
quote! {
crubit_features: hash_map!{
...
diff --git a/rs_bindings_from_cc/json_from_cc.cc b/rs_bindings_from_cc/json_from_cc.cc
index e840e91..77376f1 100644
--- a/rs_bindings_from_cc/json_from_cc.cc
+++ b/rs_bindings_from_cc/json_from_cc.cc
@@ -24,19 +24,25 @@
// This is intended to be called from Rust tests.
extern "C" FfiU8SliceBox json_from_cc_dependency(
- FfiU8Slice header_source, FfiU8Slice dependency_header_source) {
+ FfiU8Slice target_triple, FfiU8Slice header_source,
+ FfiU8Slice dependency_header_source) {
absl::StatusOr<IR> ir = IrFromCc(
StringViewFromFfiU8Slice(header_source),
BazelLabel{"//test:testing_target"},
/* public_headers= */ {},
+ /* virtual_headers_contents_for_testing=*/
{{HeaderName(std::string(kDependencyHeaderName)),
std::string(StringViewFromFfiU8Slice(dependency_header_source))}},
+ /*headers_to_targets=*/
{{HeaderName(std::string(kDependencyHeaderName)),
- BazelLabel{std::string(kDependencyTarget)}}});
- // TODO(forster): For now it is good enough to just exit: We are just using
- // this from tests, which are ok to just fail. Clang has already printed error
- // messages. If we start using this for production, then we should bridge the
- // error code into Rust.
+ BazelLabel{std::string(kDependencyTarget)}}},
+ /*extra_rs_srcs=*/{},
+ /*clang_args=*/{"-target", StringViewFromFfiU8Slice(target_triple)});
+
+ // TODO(forster): For now it is good enough to just exit: We are just
+ // using this from tests, which are ok to just fail. Clang has already
+ // printed error messages. If we start using this for production, then we
+ // should bridge the error code into Rust.
if (!ir.ok()) {
llvm::report_fatal_error(llvm::formatv("IrFromCc reported an error: {0}",
ir.status().message()));
diff --git a/rs_bindings_from_cc/src_code_gen.rs b/rs_bindings_from_cc/src_code_gen.rs
index 6a7fbc2..111ef1c 100644
--- a/rs_bindings_from_cc/src_code_gen.rs
+++ b/rs_bindings_from_cc/src_code_gen.rs
@@ -4192,16 +4192,27 @@
#[cfg(test)]
mod tests {
use super::*;
- use ir_testing::{
- ir_from_cc, ir_from_cc_dependency, ir_record, make_ir_from_items, retrieve_func,
- with_lifetime_macros,
- };
+ use ir_testing::{make_ir_from_items, retrieve_func, with_lifetime_macros};
use static_assertions::{assert_impl_all, assert_not_impl_any};
use token_stream_matchers::{
assert_cc_matches, assert_cc_not_matches, assert_rs_matches, assert_rs_not_matches,
};
use token_stream_printer::rs_tokens_to_formatted_string_for_tests;
+ fn ir_from_cc(header: &str) -> Result<IR> {
+ ir_testing::ir_from_cc(multiplatform_testing::test_platform(), header)
+ }
+ fn ir_from_cc_dependency(header: &str, dep_header: &str) -> Result<IR> {
+ ir_testing::ir_from_cc_dependency(
+ multiplatform_testing::test_platform(),
+ header,
+ dep_header,
+ )
+ }
+ fn ir_record(name: &str) -> Record {
+ ir_testing::ir_record(multiplatform_testing::test_platform(), name)
+ }
+
fn generate_bindings_tokens(ir: IR) -> Result<BindingsTokens> {
super::generate_bindings_tokens(
Rc::new(ir),
@@ -5296,12 +5307,14 @@
Ok(())
}
- #[cfg(target_arch = "x86_64")] // vectorcall only exists on x86_64, not e.g. aarch64
mod custom_abi_tests {
use super::*;
use ir_matchers::assert_ir_matches;
#[test]
fn test_func_ptr_with_custom_abi() -> Result<()> {
+ if multiplatform_testing::test_platform() != multiplatform_testing::Platform::X86Linux {
+ return Ok(());
+ }
let ir =
ir_from_cc(r#" int (*get_ptr_to_func())(float, double) [[clang::vectorcall]]; "#)?;
@@ -5364,6 +5377,9 @@
#[test]
fn test_func_ptr_with_custom_abi_thunk() -> Result<()> {
+ if multiplatform_testing::test_platform() != multiplatform_testing::Platform::X86Linux {
+ return Ok(());
+ }
// Using an `inline` keyword forces generation of a C++ thunk in
// `rs_api_impl` (i.e. exercises `format_cc_type`,
// `format_cc_call_conv_as_clang_attribute` and similar code).
@@ -5415,6 +5431,9 @@
#[test]
fn test_custom_abi_thunk() -> Result<()> {
+ if multiplatform_testing::test_platform() != multiplatform_testing::Platform::X86Linux {
+ return Ok(());
+ }
let ir = ir_from_cc(
r#"
float f_vectorcall_calling_convention(float p1, float p2) [[clang::vectorcall]];
