Skip to content

Commit

Permalink
Merge #698 #704
Browse files Browse the repository at this point in the history
698: Remove uses of unstable feature(cfg_target_has_atomic) r=taiki-e a=taiki-e

Some no-std targets (e.g., ARMv6-M) do not support atomic CAS operations and cannot use Arc, etc.

Currently, we are using an unstable feature to detect them, but it has caused breakage in the past (#435).
Also, users of stable Rust are not able to compile crossbeam on those targets.

Instead of depending on unstable features of the compiler, this patch detects those targets using the TARGET environment variables provided by cargo for the build script, and a list of targets that do not support atomic CAS operations.

This way is the same as the way we recently adopted in [futures](rust-lang/futures-rs#2400) and [valuable](tokio-rs/valuable#12), and was originally inspired by the way [heapless](rust-embedded/heapless@44c66a7) and [defmt](https://github.com/knurling-rs/defmt/blob/963152f0fc530fca64ba4ff1492d9c4b7bf76062/build.rs#L42-L51) do, but this doesn't maintain the target list manually. (It's not really fully automated, but [it's very easy to update](https://github.com/crossbeam-rs/crossbeam/blob/a42dbed87a5739228b576f526b1e2fd80260a29b/.github/workflows/ci.yml#L89).)

Also, this completely removes the dependency on unstable features from crates other than crossbeam-epoch.

refs: rust-lang/rust#51953, rust-lang/futures-rs#2400, tokio-rs/valuable#12

704: Add AtomicCell::fetch_update r=taiki-e a=taiki-e

Equivalent of [`std::sync::atomic::AtomicN::fetch_update`](https://doc.rust-lang.org/nightly/core/sync/atomic/struct.AtomicUsize.html#method.fetch_update) that stabilized in Rust 1.45.



Co-authored-by: Taiki Endo <[email protected]>
  • Loading branch information
bors[bot] and taiki-e authored May 29, 2021
3 parents 0e7d89a + 02a41ef + 9e5ac90 commit 9a9dd75
Show file tree
Hide file tree
Showing 29 changed files with 436 additions and 134 deletions.
24 changes: 20 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ jobs:
name: test
env:
RUST_VERSION: ${{ matrix.rust }}
TARGET: ${{ matrix.target }}
RUST_TARGET: ${{ matrix.target }}
strategy:
fail-fast: false
matrix:
include:
- rust: 1.36.0
Expand Down Expand Up @@ -63,6 +64,7 @@ jobs:
env:
RUST_VERSION: ${{ matrix.rust }}
strategy:
fail-fast: false
matrix:
rust:
- 1.36.0
Expand All @@ -86,14 +88,27 @@ jobs:
- name: dependency tree check
run: ./ci/dependencies.sh

# When this job failed, run ci/no_atomic.sh and commit result changes.
# TODO(taiki-e): Ideally, this should be automated using a bot that creates
# PR when failed, but there is no bandwidth to implement it
# right now...
codegen:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update nightly && rustup default nightly
- run: ci/no_atomic.sh
- run: git diff --exit-code

# Check formatting.
rustfmt:
name: rustfmt
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update stable && rustup default stable
run: rustup update stable
- name: rustfmt
run: ./ci/rustfmt.sh

Expand All @@ -104,7 +119,7 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update stable && rustup default stable
run: rustup update stable
- name: clippy
run: ./ci/clippy.sh

Expand All @@ -126,7 +141,7 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update stable && rustup default stable
run: rustup update stable
- name: loom
run: ./ci/crossbeam-epoch-loom.sh

Expand Down Expand Up @@ -154,6 +169,7 @@ jobs:
- test
- features
- dependencies
- codegen
- rustfmt
- clippy
- san
Expand Down
5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ alloc = ["crossbeam-epoch/alloc", "crossbeam-queue/alloc"]

# Enable to use of unstable functionality.
# This is disabled by default and requires recent nightly compiler.
# Note that this is outside of the normal semver guarantees and minor versions
# of crossbeam may make breaking changes to them at any time.
#
# NOTE: This feature is outside of the normal semver guarantees and minor or
# patch versions of crossbeam may make breaking changes to them at any time.
nightly = ["crossbeam-epoch/nightly", "crossbeam-utils/nightly", "crossbeam-queue/nightly"]

[dependencies]
Expand Down
17 changes: 10 additions & 7 deletions ci/check-features.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,19 @@ if [[ "$RUST_VERSION" != "nightly"* ]]; then
# * `--no-dev-deps` - build without dev-dependencies to avoid https://github.com/rust-lang/cargo/issues/4866
# * `--exclude benchmarks` - benchmarks doesn't published.
# * `--skip nightly` - skip `nightly` feature as requires nightly compilers.
cargo hack check --all --feature-powerset --no-dev-deps --exclude benchmarks --skip nightly
cargo hack build --all --feature-powerset --no-dev-deps --exclude benchmarks --skip nightly
else
# On nightly, all feature combinations should work.
cargo hack check --all --feature-powerset --no-dev-deps --exclude benchmarks
cargo hack build --all --feature-powerset --no-dev-deps --exclude benchmarks

# Check for no_std environment.
# Build for no_std environment.
# thumbv7m-none-eabi supports atomic CAS.
# thumbv6m-none-eabi supports atomic, but not atomic CAS.
# riscv32i-unknown-none-elf does not support atomic at all.
rustup target add thumbv7m-none-eabi
rustup target add thumbv6m-none-eabi
cargo hack check --all --feature-powerset --no-dev-deps --exclude benchmarks --target thumbv7m-none-eabi --skip std,default
# * `--features nightly` is required for enable `cfg_target_has_atomic`.
# * `--ignore-unknown-features` - some crates doesn't have 'nightly' feature
cargo hack check --all --feature-powerset --no-dev-deps --exclude benchmarks --target thumbv6m-none-eabi --skip std,default --features nightly --ignore-unknown-features
rustup target add riscv32i-unknown-none-elf
cargo hack build --all --feature-powerset --no-dev-deps --exclude benchmarks --target thumbv7m-none-eabi --skip std,default
cargo hack build --all --feature-powerset --no-dev-deps --exclude benchmarks --target thumbv6m-none-eabi --skip std,default
cargo hack build --all --feature-powerset --no-dev-deps --exclude benchmarks --target riscv32i-unknown-none-elf --skip std,default
fi
65 changes: 65 additions & 0 deletions ci/no_atomic.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/bin/bash

# Update the list of targets that do not support atomic/CAS operations.
#
# Usage:
# ./ci/no_atomic.sh

set -euo pipefail
IFS=$'\n\t'

cd "$(cd "$(dirname "$0")" && pwd)"/..

file="no_atomic.rs"

{
echo "// This file is @generated by $(basename "$0")."
echo "// It is not intended for manual editing."
echo ""
} >"$file"

echo "const NO_ATOMIC_CAS: &[&str] = &[" >>"$file"
for target in $(rustc --print target-list); do
res=$(rustc --print target-spec-json -Z unstable-options --target "$target" \
| jq -r "select(.\"atomic-cas\" == false)")
[[ -z "$res" ]] || echo " \"$target\"," >>"$file"
done
echo "];" >>"$file"

{
# Only crossbeam-utils actually uses this const.
echo "#[allow(dead_code)]"
echo "const NO_ATOMIC_64: &[&str] = &["
} >>"$file"
for target in $(rustc --print target-list); do
res=$(rustc --print target-spec-json -Z unstable-options --target "$target" \
| jq -r "select(.\"max-atomic-width\" == 32)")
[[ -z "$res" ]] || echo " \"$target\"," >>"$file"
done
# It is not clear exactly what `"max-atomic-width" == null` means, but they
# actually seem to have the same max-atomic-width as the target-pointer-width.
# The targets currently included in this group are "mipsel-sony-psp",
# "thumbv4t-none-eabi", "thumbv6m-none-eabi", all of which are
# `"target-pointer-width" == "32"`, so assuming them `"max-atomic-width" == 32`
# for now.
for target in $(rustc --print target-list); do
res=$(rustc --print target-spec-json -Z unstable-options --target "$target" \
| jq -r "select(.\"max-atomic-width\" == null)")
[[ -z "$res" ]] || echo " \"$target\"," >>"$file"
done
echo "];" >>"$file"

# There is no `"max-atomic-width" == 16` or `"max-atomic-width" == 8` targets.

# `"max-atomic-width" == 0` means that atomic is not supported at all.
{
# Only crossbeam-utils actually uses this const.
echo "#[allow(dead_code)]"
echo "const NO_ATOMIC: &[&str] = &["
} >>"$file"
for target in $(rustc --print target-list); do
res=$(rustc --print target-spec-json -Z unstable-options --target "$target" \
| jq -r "select(.\"max-atomic-width\" == 0)")
[[ -z "$res" ]] || echo " \"$target\"," >>"$file"
done
echo "];" >>"$file"
6 changes: 3 additions & 3 deletions ci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ set -ex

export RUSTFLAGS="-D warnings"

if [[ -n "$TARGET" ]]; then
# If TARGET is specified, use cross for testing.
if [[ -n "$RUST_TARGET" ]]; then
# If RUST_TARGET is specified, use cross for testing.
cargo install cross
cross test --all --target "$TARGET" --exclude benchmarks -- --test-threads=1
cross test --all --target "$RUST_TARGET" --exclude benchmarks -- --test-threads=1

# For now, the non-host target only runs tests.
exit 0
Expand Down
2 changes: 2 additions & 0 deletions crossbeam-channel/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ default = ["std"]

# Enable to use APIs that require `std`.
# This is enabled by default.
#
# NOTE: Disabling `std` feature is not supported yet.
std = ["crossbeam-utils/std"]

[dependencies]
Expand Down
2 changes: 2 additions & 0 deletions crossbeam-deque/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ default = ["std"]

# Enable to use APIs that require `std`.
# This is enabled by default.
#
# NOTE: Disabling `std` feature is not supported yet.
std = ["crossbeam-epoch/std", "crossbeam-utils/std"]

[dependencies]
Expand Down
15 changes: 9 additions & 6 deletions crossbeam-epoch/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,21 @@ std = ["alloc", "crossbeam-utils/std", "lazy_static"]

# Enable to use APIs that require `alloc`.
# This is enabled by default and also enabled if the `std` feature is enabled.
#
# NOTE: Disabling both `std` *and* `alloc` features is not supported yet.
alloc = []

# Enable to use of unstable functionality.
# This is disabled by default and requires recent nightly compiler.
# Note that this is outside of the normal semver guarantees and minor versions
# of crossbeam may make breaking changes to them at any time.
#
# NOTE: This feature is outside of the normal semver guarantees and minor or
# patch versions of crossbeam may make breaking changes to them at any time.
nightly = ["crossbeam-utils/nightly", "const_fn"]

# Enable the use of loom for concurrency testing.
#
# This configuration option is outside of the normal semver guarantees: minor
# versions of crossbeam may make breaking changes to it at any time.
# NOTE: This feature is outside of the normal semver guarantees and minor or
# patch versions of crossbeam may make breaking changes to them at any time.
loom = ["loom-crate", "crossbeam-utils/loom"]

[dependencies]
Expand All @@ -45,8 +48,8 @@ memoffset = "0.6"

# Enable the use of loom for concurrency testing.
#
# This configuration option is outside of the normal semver guarantees: minor
# versions of crossbeam may make breaking changes to it at any time.
# NOTE: This feature is outside of the normal semver guarantees and minor or
# patch versions of crossbeam may make breaking changes to them at any time.
[target.'cfg(crossbeam_loom)'.dependencies]
loom-crate = { package = "loom", version = "0.5", optional = true }

Expand Down
32 changes: 32 additions & 0 deletions crossbeam-epoch/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#![warn(rust_2018_idioms)]

use std::env;

include!("no_atomic.rs");

// The rustc-cfg strings below are *not* public API. Please let us know by
// opening a GitHub issue if your build environment requires some way to enable
// these cfgs other than by executing our build script.
fn main() {
let target = match env::var("TARGET") {
Ok(target) => target,
Err(e) => {
println!(
"cargo:warning={}: unable to get TARGET environment variable: {}",
env!("CARGO_PKG_NAME"),
e
);
return;
}
};

// Note that this is `no_*`, not `has_*`. This allows treating
// `cfg(target_has_atomic = "ptr")` as true when the build script doesn't
// run. This is needed for compatibility with non-cargo build systems that
// don't run the build script.
if NO_ATOMIC_CAS.contains(&&*target) {
println!("cargo:rustc-cfg=crossbeam_no_atomic_cas");
}

println!("cargo:rerun-if-changed=no_atomic.rs");
}
1 change: 1 addition & 0 deletions crossbeam-epoch/no_atomic.rs
9 changes: 4 additions & 5 deletions crossbeam-epoch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
unreachable_pub
)]
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(feature = "nightly", feature(cfg_target_has_atomic))]
#![cfg_attr(feature = "nightly", feature(const_fn_trait_bound))]

#[cfg(crossbeam_loom)]
Expand Down Expand Up @@ -105,10 +104,11 @@ mod primitive {
pub(crate) use loom::lazy_static;
pub(crate) use loom::thread_local;
}
#[cfg(not(crossbeam_no_atomic_cas))]
#[cfg(not(crossbeam_loom))]
#[allow(unused_imports, dead_code)]
mod primitive {
#[cfg(any(feature = "alloc", feature = "std"))]
#[cfg(feature = "alloc")]
pub(crate) mod cell {
#[derive(Debug)]
#[repr(transparent)]
Expand Down Expand Up @@ -136,14 +136,13 @@ mod primitive {
}
}
}
#[cfg(any(feature = "alloc", feature = "std"))]
#[cfg(feature = "alloc")]
pub(crate) mod sync {
pub(crate) mod atomic {
pub(crate) use core::sync::atomic::compiler_fence;
pub(crate) use core::sync::atomic::fence;
pub(crate) use core::sync::atomic::AtomicUsize;
}
#[cfg_attr(feature = "nightly", cfg(target_has_atomic = "ptr"))]
pub(crate) use alloc::sync::Arc;
}

Expand All @@ -154,7 +153,7 @@ mod primitive {
pub(crate) use lazy_static::lazy_static;
}

#[cfg_attr(feature = "nightly", cfg(target_has_atomic = "ptr"))]
#[cfg(not(crossbeam_no_atomic_cas))]
cfg_if! {
if #[cfg(feature = "alloc")] {
extern crate alloc;
Expand Down
9 changes: 7 additions & 2 deletions crossbeam-queue/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,17 @@ std = ["alloc", "crossbeam-utils/std"]

# Enable to use APIs that require `alloc`.
# This is enabled by default and also enabled if the `std` feature is enabled.
#
# NOTE: Disabling both `std` *and* `alloc` features is not supported yet.
alloc = []

# These features are no longer used.
# TODO: remove in the next major version.
# Enable to use of unstable functionality.
# This is disabled by default and requires recent nightly compiler.
# Note that this is outside of the normal semver guarantees and minor versions
# of crossbeam may make breaking changes to them at any time.
#
# NOTE: This feature is outside of the normal semver guarantees and minor or
# patch versions of crossbeam may make breaking changes to them at any time.
nightly = ["crossbeam-utils/nightly"]

[dependencies]
Expand Down
32 changes: 32 additions & 0 deletions crossbeam-queue/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#![warn(rust_2018_idioms)]

use std::env;

include!("no_atomic.rs");

// The rustc-cfg strings below are *not* public API. Please let us know by
// opening a GitHub issue if your build environment requires some way to enable
// these cfgs other than by executing our build script.
fn main() {
let target = match env::var("TARGET") {
Ok(target) => target,
Err(e) => {
println!(
"cargo:warning={}: unable to get TARGET environment variable: {}",
env!("CARGO_PKG_NAME"),
e
);
return;
}
};

// Note that this is `no_*`, not `has_*`. This allows treating
// `cfg(target_has_atomic = "ptr")` as true when the build script doesn't
// run. This is needed for compatibility with non-cargo build systems that
// don't run the build script.
if NO_ATOMIC_CAS.contains(&&*target) {
println!("cargo:rustc-cfg=crossbeam_no_atomic_cas");
}

println!("cargo:rerun-if-changed=no_atomic.rs");
}
1 change: 1 addition & 0 deletions crossbeam-queue/no_atomic.rs
3 changes: 1 addition & 2 deletions crossbeam-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
unreachable_pub
)]
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(feature = "nightly", feature(cfg_target_has_atomic))]

#[cfg_attr(feature = "nightly", cfg(target_has_atomic = "ptr"))]
#[cfg(not(crossbeam_no_atomic_cas))]
cfg_if::cfg_if! {
if #[cfg(feature = "alloc")] {
extern crate alloc;
Expand Down
Loading

0 comments on commit 9a9dd75

Please sign in to comment.