Skip to content

Commit

Permalink
Merge pull request #4151 from tgross35/tests-and-const-extern
Browse files Browse the repository at this point in the history
Ensure all tests in the workspace are run and that `const extern fn` is always enabled
  • Loading branch information
tgross35 authored Nov 26, 2024
2 parents 42e5708 + e6d7b51 commit 3cce47f
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 39 deletions.
6 changes: 1 addition & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,10 @@ cargo-args = ["-Zbuild-std=core"]
rustc-std-workspace-core = { version = "1.0.0", optional = true }

[features]
default = ["const-extern-fn", "std"]
default = ["std"]
std = []
rustc-dep-of-std = ["rustc-std-workspace-core"]
extra_traits = []

# `const-extern-function` is deprecated and no longer does anything
# FIXME(1.0): remove this completely
const-extern-fn = []

[workspace]
members = ["libc-test"]
5 changes: 5 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const ALLOWED_CFGS: &'static [&'static str] = &[
"freebsd13",
"freebsd14",
"freebsd15",
// FIXME(ctest): this config shouldn't be needed but ctest can't parse `const extern fn`
"libc_const_extern_fn",
"libc_deny_warnings",
"libc_ctest",
];
Expand Down Expand Up @@ -74,6 +76,9 @@ fn main() {
set_cfg("libc_deny_warnings");
}

// Set unconditionally when ctest is not being invoked.
set_cfg("libc_const_extern_fn");

// check-cfg is a nightly cargo/rustc feature to warn when unknown cfgs are used across the
// codebase. libc can configure it if the appropriate environment variable is passed. Since
// rust-lang/rust enforces it, this is useful when using a custom libc fork there.
Expand Down
11 changes: 5 additions & 6 deletions ci/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,15 @@ cmd="cargo test --target $target ${LIBC_CI_ZBUILD_STD+"-Zbuild-std"}"

# Run tests in the `libc` crate
case "$target" in
# Only run `libc-test`
# FIXME(android): unit tests fail to start on Android
# FIXME(s390x): unit tests fail to locate glibc
*android*) ;;
*s390x*) ;;
*) $cmd
*android*) cmd="$cmd --manifest-path libc-test/Cargo.toml" ;;
*s390x*) cmd="$cmd --manifest-path libc-test/Cargo.toml" ;;
# For all other platforms, test everything in the workspace
*) cmd="$cmd --workspace"
esac

# Everything else is in `libc-test`
cmd="$cmd --manifest-path libc-test/Cargo.toml"

if [ "$target" = "s390x-unknown-linux-gnu" ]; then
# FIXME: s390x-unknown-linux-gnu often fails to test due to timeout,
# so we retry this N times.
Expand Down
1 change: 0 additions & 1 deletion ci/verify-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ test_target() {

# Test with expected combinations of features
$cmd
$cmd --features const-extern-fn
$cmd --features extra_traits

# Test again without default features, i.e. without "std"
Expand Down
45 changes: 18 additions & 27 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,40 +167,31 @@ macro_rules! e {
)*);
}

// This is a pretty horrible hack to allow us to conditionally mark
// some functions as 'const', without requiring users of this macro
// to care about the "const-extern-fn" feature.
// This is a pretty horrible hack to allow us to conditionally mark some functions as 'const',
// without requiring users of this macro to care "libc_const_extern_fn".
//
// When 'const-extern-fn' is enabled, we emit the captured 'const' keyword
// in the expanded function.
// When 'libc_const_extern_fn' is enabled, we emit the captured 'const' keyword in the expanded
// function.
//
// When 'const-extern-fn' is disabled, we always emit a plain 'pub unsafe extern fn'.
// When 'libc_const_extern_fn' is disabled, we always emit a plain 'pub unsafe extern fn'.
// Note that the expression matched by the macro is exactly the same - this allows
// users of this macro to work whether or not 'const-extern-fn' is enabled
// users of this macro to work whether or not 'libc_const_extern_fn' is enabled
//
// Unfortunately, we need to duplicate most of this macro between the 'cfg_if' blocks.
// This is because 'const unsafe extern fn' won't even parse on older compilers,
// so we need to avoid emitting it at all of 'const-extern-fn'.
// so we need to avoid emitting it at all of 'libc_const_extern_fn'.
//
// Specifically, moving the 'cfg_if' into the macro body will *not* work.
// Doing so would cause the '#[cfg(feature = "const-extern-fn")]' to be emitted
// into user code. The 'cfg' gate will not stop Rust from trying to parse the
// 'pub const unsafe extern fn', so users would get a compiler error even when
// the 'const-extern-fn' feature is disabled
//
// Note that users of this macro need to place 'const' in a weird position
// (after the closing ')' for the arguments, but before the return type).
// This was the only way I could satisfy the following two requirements:
// 1. Avoid ambiguity errors from 'macro_rules!' (which happen when writing '$foo:ident fn'
// 2. Allow users of this macro to mix 'pub fn foo' and 'pub const fn bar' within the same
// 'f!' block
// Specifically, moving the 'cfg_if' into the macro body will *not* work. Doing so would cause the
// '#[cfg(libc_const_extern_fn)]' to be emitted into user code. The 'cfg' gate will not stop Rust
// from trying to parse the 'pub const unsafe extern fn', so users would get a compiler error even
// when the 'libc_const_extern_fn' feature is disabled.

// FIXME(ctest): ctest can't handle `const extern` functions, we should be able to remove this
// cfg completely.
// FIXME(ctest): ctest can't handle `$(,)?` so we use `$(,)*` which isn't quite correct.
cfg_if! {
if #[cfg(feature = "const-extern-fn")] {
/// Define an `unsafe` function that is const as long as `const-extern-fn` is enabled.
if #[cfg(libc_const_extern_fn)] {
/// Define an `unsafe` function that is const as long as `libc_const_extern_fn` is enabled.
macro_rules! f {
($(
$(#[$attr:meta])*
Expand All @@ -214,7 +205,7 @@ cfg_if! {
)*)
}

/// Define a safe function that is const as long as `const-extern-fn` is enabled.
/// Define a safe function that is const as long as `libc_const_extern_fn` is enabled.
macro_rules! safe_f {
($(
$(#[$attr:meta])*
Expand All @@ -228,7 +219,7 @@ cfg_if! {
)*)
}

/// A nonpublic function that is const as long as `const-extern-fn` is enabled.
/// A nonpublic function that is const as long as `libc_const_extern_fn` is enabled.
macro_rules! const_fn {
($(
$(#[$attr:meta])*
Expand All @@ -242,7 +233,7 @@ cfg_if! {
)*)
}
} else {
/// Define an `unsafe` function that is const as long as `const-extern-fn` is enabled.
/// Define an `unsafe` function that is const as long as `libc_const_extern_fn` is enabled.
macro_rules! f {
($(
$(#[$attr:meta])*
Expand All @@ -256,7 +247,7 @@ cfg_if! {
)*)
}

/// Define a safe function that is const as long as `const-extern-fn` is enabled.
/// Define a safe function that is const as long as `libc_const_extern_fn` is enabled.
macro_rules! safe_f {
($(
$(#[$attr:meta])*
Expand All @@ -270,7 +261,7 @@ cfg_if! {
)*)
}

/// A nonpublic function that is const as long as `const-extern-fn` is enabled.
/// A nonpublic function that is const as long as `libc_const_extern_fn` is enabled.
macro_rules! const_fn {
($(
$(#[$attr:meta])*
Expand Down

0 comments on commit 3cce47f

Please sign in to comment.