From 0a2e648842a3cf4b2391bc5ae5bf53afc2c38d80 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 31 Mar 2023 03:50:50 +0900 Subject: [PATCH] Use portable-atomic and remove our own logic for now Adopt the third idea of https://github.com/tokio-rs/bytes/pull/573#issuecomment-1271567631. --- .github/workflows/ci.yml | 37 ++++++++++--------------------------- Cargo.toml | 4 ++++ build.rs | 31 ------------------------------- ci/no_atomic_cas.sh | 27 --------------------------- no_atomic_cas.rs | 13 ------------- src/buf/buf_impl.rs | 2 -- src/buf/chain.rs | 1 - src/buf/take.rs | 1 - src/lib.rs | 7 ------- src/loom.rs | 4 +++- 10 files changed, 17 insertions(+), 110 deletions(-) delete mode 100644 build.rs delete mode 100755 ci/no_atomic_cas.sh delete mode 100644 no_atomic_cas.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 12ee4d62d..e5d3b61a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,16 +113,6 @@ jobs: # Build for no_std environment. no-std: - strategy: - fail-fast: false - matrix: - # 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. - target: - - thumbv7m-none-eabi - - thumbv6m-none-eabi - - riscv32i-unknown-none-elf runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 @@ -130,23 +120,16 @@ jobs: run: rustup update stable - name: Install cargo-hack run: cargo install cargo-hack - - run: rustup target add ${{ matrix.target }} - # * --optional-deps is needed for serde feature - # * --no-dev-deps is needed to avoid https://github.com/rust-lang/cargo/issues/4866 - - run: cargo hack build --target ${{ matrix.target }} --feature-powerset --skip std,default --optional-deps --no-dev-deps - - # When this job failed, run ci/no_atomic_cas.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_cas.sh - - run: git diff --exit-code + # thumbv7m-none-eabi supports atomic CAS. + # thumbv6m-none-eabi supports atomic, but not atomic CAS. + - run: rustup target add thumbv7m-none-eabi + - run: rustup target add thumbv6m-none-eabi + # * --optional-deps is needed for serde feature + # * --no-dev-deps is needed to avoid https://github.com/rust-lang/cargo/issues/4866 + - run: cargo hack build --target thumbv7m-none-eabi --feature-powerset --skip std,default --optional-deps --no-dev-deps + # A sound way to provide atomic CAS on platforms without native atomic CAS is system-dependent. + # portable-atomic provides major ways via cfgs and accepts user-defined implementations via critical-section features. + - run: cargo hack build --target thumbv6m-none-eabi --feature-powerset --skip std,default --optional-deps --no-dev-deps --features extra-platforms,portable-atomic/critical-section # Sanitizers tsan: diff --git a/Cargo.toml b/Cargo.toml index 4a96ec1ed..2765fd064 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,9 +20,13 @@ edition = "2018" [features] default = ["std"] std = [] +# Use portable-atomic crate to support platforms without atomic CAS. +# See https://docs.rs/portable-atomic for more information. +extra-platforms = ["portable-atomic"] [dependencies] serde = { version = "1.0.60", optional = true, default-features = false, features = ["alloc"] } +portable-atomic = { version = "1", optional = true, default-features = false } [dev-dependencies] serde_test = "1.0" diff --git a/build.rs b/build.rs deleted file mode 100644 index b71e3cab3..000000000 --- a/build.rs +++ /dev/null @@ -1,31 +0,0 @@ -#![warn(rust_2018_idioms)] - -use std::env; - -include!("no_atomic_cas.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=bytes: unable to get TARGET environment variable: {}", - 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=bytes_no_atomic_cas"); - } - - println!("cargo:rerun-if-changed=no_atomic_cas.rs"); -} diff --git a/ci/no_atomic_cas.sh b/ci/no_atomic_cas.sh deleted file mode 100755 index bc2e3500f..000000000 --- a/ci/no_atomic_cas.sh +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/bash - -# Update the list of targets that do not support atomic CAS operations. -# -# Usage: -# ./ci/no_atomic_cas.sh - -set -euo pipefail -IFS=$'\n\t' - -cd "$(cd "$(dirname "$0")" && pwd)"/.. - -file="no_atomic_cas.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" diff --git a/no_atomic_cas.rs b/no_atomic_cas.rs deleted file mode 100644 index 9b05d4b9f..000000000 --- a/no_atomic_cas.rs +++ /dev/null @@ -1,13 +0,0 @@ -// This file is @generated by no_atomic_cas.sh. -// It is not intended for manual editing. - -const NO_ATOMIC_CAS: &[&str] = &[ - "avr-unknown-gnu-atmega328", - "bpfeb-unknown-none", - "bpfel-unknown-none", - "msp430-none-elf", - "riscv32i-unknown-none-elf", - "riscv32imc-unknown-none-elf", - "thumbv4t-none-eabi", - "thumbv6m-none-eabi", -]; diff --git a/src/buf/buf_impl.rs b/src/buf/buf_impl.rs index 939d517cb..366cfc989 100644 --- a/src/buf/buf_impl.rs +++ b/src/buf/buf_impl.rs @@ -1100,7 +1100,6 @@ pub trait Buf { /// let bytes = (&b"hello world"[..]).copy_to_bytes(5); /// assert_eq!(&bytes[..], &b"hello"[..]); /// ``` - #[cfg(not(bytes_no_atomic_cas))] fn copy_to_bytes(&mut self, len: usize) -> crate::Bytes { use super::BufMut; @@ -1325,7 +1324,6 @@ macro_rules! deref_forward_buf { (**self).get_int_ne(nbytes) } - #[cfg(not(bytes_no_atomic_cas))] fn copy_to_bytes(&mut self, len: usize) -> crate::Bytes { (**self).copy_to_bytes(len) } diff --git a/src/buf/chain.rs b/src/buf/chain.rs index 8b058d263..2011e9edc 100644 --- a/src/buf/chain.rs +++ b/src/buf/chain.rs @@ -171,7 +171,6 @@ where n } - #[cfg(not(bytes_no_atomic_cas))] fn copy_to_bytes(&mut self, len: usize) -> crate::Bytes { let a_rem = self.a.remaining(); if a_rem >= len { diff --git a/src/buf/take.rs b/src/buf/take.rs index 6ca190c9d..c4436f288 100644 --- a/src/buf/take.rs +++ b/src/buf/take.rs @@ -145,7 +145,6 @@ impl Buf for Take { self.limit -= cnt; } - #[cfg(not(bytes_no_atomic_cas))] fn copy_to_bytes(&mut self, len: usize) -> crate::Bytes { assert!(len <= self.remaining(), "`len` greater than remaining"); diff --git a/src/lib.rs b/src/lib.rs index d3e753055..af436b316 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -84,24 +84,17 @@ extern crate std; pub mod buf; pub use crate::buf::{Buf, BufMut}; -#[cfg(not(bytes_no_atomic_cas))] mod bytes; -#[cfg(not(bytes_no_atomic_cas))] mod bytes_mut; -#[cfg(not(bytes_no_atomic_cas))] mod fmt; mod loom; -#[cfg(not(bytes_no_atomic_cas))] pub use crate::bytes::Bytes; -#[cfg(not(bytes_no_atomic_cas))] pub use crate::bytes_mut::BytesMut; // Optional Serde support -#[cfg(not(bytes_no_atomic_cas))] #[cfg(feature = "serde")] mod serde; -#[cfg(not(bytes_no_atomic_cas))] #[inline(never)] #[cold] fn abort() -> ! { diff --git a/src/loom.rs b/src/loom.rs index e30555c3d..e2820b9ed 100644 --- a/src/loom.rs +++ b/src/loom.rs @@ -2,9 +2,11 @@ #[cfg(not(all(test, loom)))] pub(crate) mod sync { - #[cfg(not(bytes_no_atomic_cas))] pub(crate) mod atomic { + #[cfg(not(feature = "extra-platforms"))] pub(crate) use core::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; + #[cfg(feature = "extra-platforms")] + pub(crate) use portable_atomic::{AtomicPtr, AtomicUsize, Ordering}; pub(crate) trait AtomicMut { fn with_mut(&mut self, f: F) -> R