Skip to content

Commit

Permalink
Auto merge of #116742 - GuillaumeGomez:rollup-xjxs0mr, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
Rollup of 3 pull requests

Successful merges:

 - #116540 (Implement `OnceCell/Lock::try_insert()`)
 - #116576 (const-eval: allow calling functions with targat features disabled at compile time in WASM)
 - #116661 (Make "request changes" reviews apply `S-waiting-on-author`)

Failed merges:

 - #116643 (x.py zsh completion support)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Oct 14, 2023
2 parents c543b6f + 31b86ea commit e292fec
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 18 deletions.
10 changes: 6 additions & 4 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -890,11 +890,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

fn check_fn_target_features(&self, instance: ty::Instance<'tcx>) -> InterpResult<'tcx, ()> {
// Calling functions with `#[target_feature]` is not unsafe on WASM, see #84988
let attrs = self.tcx.codegen_fn_attrs(instance.def_id());
if attrs
.target_features
.iter()
.any(|feature| !self.tcx.sess.target_features.contains(feature))
if !self.tcx.sess.target.is_like_wasm
&& attrs
.target_features
.iter()
.any(|feature| !self.tcx.sess.target_features.contains(feature))
{
throw_ub_custom!(
fluent::const_eval_unavailable_target_features_for_fn,
Expand Down
46 changes: 37 additions & 9 deletions library/core/src/cell/once.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,48 @@ impl<T> OnceCell<T> {
#[inline]
#[stable(feature = "once_cell", since = "1.70.0")]
pub fn set(&self, value: T) -> Result<(), T> {
// SAFETY: Safe because we cannot have overlapping mutable borrows
let slot = unsafe { &*self.inner.get() };
if slot.is_some() {
return Err(value);
match self.try_insert(value) {
Ok(_) => Ok(()),
Err((_, value)) => Err(value),
}
}

/// Sets the contents of the cell to `value` if the cell was empty, then
/// returns a reference to it.
///
/// # Errors
///
/// This method returns `Ok(&value)` if the cell was empty and
/// `Err(&current_value, value)` if it was full.
///
/// # Examples
///
/// ```
/// #![feature(once_cell_try_insert)]
///
/// use std::cell::OnceCell;
///
/// let cell = OnceCell::new();
/// assert!(cell.get().is_none());
///
/// assert_eq!(cell.try_insert(92), Ok(&92));
/// assert_eq!(cell.try_insert(62), Err((&92, 62)));
///
/// assert!(cell.get().is_some());
/// ```
#[inline]
#[unstable(feature = "once_cell_try_insert", issue = "116693")]
pub fn try_insert(&self, value: T) -> Result<&T, (&T, T)> {
if let Some(old) = self.get() {
return Err((old, value));
}

// SAFETY: This is the only place where we set the slot, no races
// due to reentrancy/concurrency are possible, and we've
// checked that slot is currently `None`, so this write
// maintains the `inner`'s invariant.
let slot = unsafe { &mut *self.inner.get() };
*slot = Some(value);
Ok(())
Ok(slot.insert(value))
}

/// Gets the contents of the cell, initializing it with `f`
Expand Down Expand Up @@ -183,10 +212,9 @@ impl<T> OnceCell<T> {
let val = outlined_call(f)?;
// Note that *some* forms of reentrant initialization might lead to
// UB (see `reentrant_init` test). I believe that just removing this
// `assert`, while keeping `set/get` would be sound, but it seems
// `panic`, while keeping `try_insert` would be sound, but it seems
// better to panic, rather than to silently use an old value.
assert!(self.set(val).is_ok(), "reentrant init");
Ok(self.get().unwrap())
if let Ok(val) = self.try_insert(val) { Ok(val) } else { panic!("reentrant init") }
}

/// Consumes the cell, returning the wrapped value.
Expand Down
43 changes: 40 additions & 3 deletions library/std/src/sync/once_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,48 @@ impl<T> OnceLock<T> {
#[inline]
#[stable(feature = "once_cell", since = "1.70.0")]
pub fn set(&self, value: T) -> Result<(), T> {
match self.try_insert(value) {
Ok(_) => Ok(()),
Err((_, value)) => Err(value),
}
}

/// Sets the contents of this cell to `value` if the cell was empty, then
/// returns a reference to it.
///
/// May block if another thread is currently attempting to initialize the cell. The cell is
/// guaranteed to contain a value when set returns, though not necessarily the one provided.
///
/// Returns `Ok(&value)` if the cell was empty and `Err(&current_value, value)` if it was full.
///
/// # Examples
///
/// ```
/// #![feature(once_cell_try_insert)]
///
/// use std::sync::OnceLock;
///
/// static CELL: OnceLock<i32> = OnceLock::new();
///
/// fn main() {
/// assert!(CELL.get().is_none());
///
/// std::thread::spawn(|| {
/// assert_eq!(CELL.try_insert(92), Ok(&92));
/// }).join().unwrap();
///
/// assert_eq!(CELL.try_insert(62), Err((&92, 62)));
/// assert_eq!(CELL.get(), Some(&92));
/// }
/// ```
#[inline]
#[unstable(feature = "once_cell_try_insert", issue = "116693")]
pub fn try_insert(&self, value: T) -> Result<&T, (&T, T)> {
let mut value = Some(value);
self.get_or_init(|| value.take().unwrap());
let res = self.get_or_init(|| value.take().unwrap());
match value {
None => Ok(()),
Some(value) => Err(value),
None => Ok(res),
Some(value) => Err((res, value)),
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ case $HOST_TARGET in
MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests
MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple atomic data_race env/var
MIRI_TEST_TARGET=aarch64-linux-android run_tests_minimal hello integer vec panic/panic
MIRI_TEST_TARGET=wasm32-wasi run_tests_minimal no_std integer strings
MIRI_TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std integer strings
MIRI_TEST_TARGET=wasm32-wasi run_tests_minimal no_std integer strings wasm
MIRI_TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std integer strings wasm
MIRI_TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std # no_std embedded architecture
MIRI_TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std # JSON target file
;;
Expand Down
11 changes: 11 additions & 0 deletions src/tools/miri/tests/pass/function_calls/target_feature_wasm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@only-target-wasm32: tests WASM-specific behavior
//@compile-flags: -C target-feature=-simd128

fn main() {
// Calling functions with `#[target_feature]` is not unsound on WASM, see #84988
assert!(!cfg!(target_feature = "simd128"));
simd128_fn();
}

#[target_feature(enable = "simd128")]
fn simd128_fn() {}
14 changes: 14 additions & 0 deletions tests/ui/consts/const-eval/const_fn_target_feature_wasm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// only-wasm32
// compile-flags:-C target-feature=-simd128
// build-pass

#![crate_type = "lib"]

#[cfg(target_feature = "simd128")]
compile_error!("simd128 target feature should be disabled");

// Calling functions with `#[target_feature]` is not unsound on WASM, see #84988
const A: () = simd128_fn();

#[target_feature(enable = "simd128")]
const fn simd128_fn() {}
6 changes: 6 additions & 0 deletions triagebot.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ allow-unauthenticated = [
"needs-triage",
]

[review-submitted]
# This label is added when a "request changes" review is submitted.
reviewed_label = "S-waiting-on-author"
# These labels are removed when a "request changes" review is submitted.
review_labels = ["S-waiting-on-review"]

[glacier]

[ping.icebreakers-llvm]
Expand Down

0 comments on commit e292fec

Please sign in to comment.