Skip to content

Commit

Permalink
Auto merge of #127261 - jhpratt:rollup-cpoayvr, r=jhpratt
Browse files Browse the repository at this point in the history
Rollup of 8 pull requests

Successful merges:

 - #123588 (Stabilize `hint::assert_unchecked`)
 - #126403 (Actually report normalization-based type errors correctly for alias-relate obligations in new solver)
 - #126917 (Disable rmake test `inaccessible-temp-dir` on riscv64)
 - #127115 (unreferenced-used-static: run test everywhere)
 - #127204 (Stabilize atomic_bool_fetch_not)
 - #127239 (remove unnecessary ignore-endian-big from stack-overflow-trait-infer …)
 - #127245 (Add a test for `generic_const_exprs`)
 - #127246 (Give remote-test-client a longer timeout)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 3, 2024
2 parents d68fe4e + d801730 commit 2db4ff4
Show file tree
Hide file tree
Showing 25 changed files with 266 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1586,60 +1586,113 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}

self.probe(|_| {
let ocx = ObligationCtxt::new(self);

// try to find the mismatched types to report the error with.
//
// this can fail if the problem was higher-ranked, in which
// cause I have no idea for a good error message.
let bound_predicate = predicate.kind();
let (values, err) = if let ty::PredicateKind::Clause(ty::ClauseKind::Projection(data)) =
bound_predicate.skip_binder()
{
let data = self.instantiate_binder_with_fresh_vars(
obligation.cause.span,
infer::BoundRegionConversionTime::HigherRankedType,
bound_predicate.rebind(data),
);
let unnormalized_term = data.projection_term.to_term(self.tcx);
// FIXME(-Znext-solver): For diagnostic purposes, it would be nice
// to deeply normalize this type.
let normalized_term =
ocx.normalize(&obligation.cause, obligation.param_env, unnormalized_term);

debug!(?obligation.cause, ?obligation.param_env);

debug!(?normalized_term, data.ty = ?data.term);
let (values, err) = match bound_predicate.skip_binder() {
ty::PredicateKind::Clause(ty::ClauseKind::Projection(data)) => {
let ocx = ObligationCtxt::new(self);

let data = self.instantiate_binder_with_fresh_vars(
obligation.cause.span,
infer::BoundRegionConversionTime::HigherRankedType,
bound_predicate.rebind(data),
);
let unnormalized_term = data.projection_term.to_term(self.tcx);
// FIXME(-Znext-solver): For diagnostic purposes, it would be nice
// to deeply normalize this type.
let normalized_term =
ocx.normalize(&obligation.cause, obligation.param_env, unnormalized_term);

let is_normalized_term_expected = !matches!(
obligation.cause.code().peel_derives(),
ObligationCauseCode::WhereClause(..)
| ObligationCauseCode::WhereClauseInExpr(..)
| ObligationCauseCode::Coercion { .. }
);

let is_normalized_term_expected = !matches!(
obligation.cause.code().peel_derives(),
|ObligationCauseCode::WhereClause(..)| ObligationCauseCode::WhereClauseInExpr(
..
) | ObligationCauseCode::Coercion { .. }
);
let (expected, actual) = if is_normalized_term_expected {
(normalized_term, data.term)
} else {
(data.term, normalized_term)
};

let (expected, actual) = if is_normalized_term_expected {
(normalized_term, data.term)
} else {
(data.term, normalized_term)
};
// constrain inference variables a bit more to nested obligations from normalize so
// we can have more helpful errors.
//
// we intentionally drop errors from normalization here,
// since the normalization is just done to improve the error message.
let _ = ocx.select_where_possible();

// constrain inference variables a bit more to nested obligations from normalize so
// we can have more helpful errors.
//
// we intentionally drop errors from normalization here,
// since the normalization is just done to improve the error message.
let _ = ocx.select_where_possible();
if let Err(new_err) =
ocx.eq(&obligation.cause, obligation.param_env, expected, actual)
{
(
Some((
data.projection_term,
is_normalized_term_expected,
self.resolve_vars_if_possible(normalized_term),
data.term,
)),
new_err,
)
} else {
(None, error.err)
}
}
ty::PredicateKind::AliasRelate(lhs, rhs, _) => {
let derive_better_type_error =
|alias_term: ty::AliasTerm<'tcx>, expected_term: ty::Term<'tcx>| {
let ocx = ObligationCtxt::new(self);
let normalized_term = match expected_term.unpack() {
ty::TermKind::Ty(_) => self.next_ty_var(DUMMY_SP).into(),
ty::TermKind::Const(_) => self.next_const_var(DUMMY_SP).into(),
};
ocx.register_obligation(Obligation::new(
self.tcx,
ObligationCause::dummy(),
obligation.param_env,
ty::PredicateKind::NormalizesTo(ty::NormalizesTo {
alias: alias_term,
term: normalized_term,
}),
));
let _ = ocx.select_where_possible();
if let Err(terr) = ocx.eq(
&ObligationCause::dummy(),
obligation.param_env,
expected_term,
normalized_term,
) {
Some((terr, self.resolve_vars_if_possible(normalized_term)))
} else {
None
}
};

if let Err(new_err) =
ocx.eq(&obligation.cause, obligation.param_env, expected, actual)
{
(Some((data, is_normalized_term_expected, normalized_term, data.term)), new_err)
} else {
(None, error.err)
if let Some(lhs) = lhs.to_alias_term()
&& let Some((better_type_err, expected_term)) =
derive_better_type_error(lhs, rhs)
{
(
Some((lhs, true, self.resolve_vars_if_possible(expected_term), rhs)),
better_type_err,
)
} else if let Some(rhs) = rhs.to_alias_term()
&& let Some((better_type_err, expected_term)) =
derive_better_type_error(rhs, lhs)
{
(
Some((rhs, true, self.resolve_vars_if_possible(expected_term), lhs)),
better_type_err,
)
} else {
(None, error.err)
}
}
} else {
(None, error.err)
_ => (None, error.err),
};

let msg = values
Expand Down Expand Up @@ -1737,15 +1790,15 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {

fn maybe_detailed_projection_msg(
&self,
pred: ty::ProjectionPredicate<'tcx>,
projection_term: ty::AliasTerm<'tcx>,
normalized_ty: ty::Term<'tcx>,
expected_ty: ty::Term<'tcx>,
) -> Option<String> {
let trait_def_id = pred.projection_term.trait_def_id(self.tcx);
let self_ty = pred.projection_term.self_ty();
let trait_def_id = projection_term.trait_def_id(self.tcx);
let self_ty = projection_term.self_ty();

with_forced_trimmed_paths! {
if self.tcx.is_lang_item(pred.projection_term.def_id,LangItem::FnOnceOutput) {
if self.tcx.is_lang_item(projection_term.def_id, LangItem::FnOnceOutput) {
let fn_kind = self_ty.prefix_string(self.tcx);
let item = match self_ty.kind() {
ty::FnDef(def, _) => self.tcx.item_name(*def).to_string(),
Expand Down
1 change: 0 additions & 1 deletion library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@
#![feature(fmt_internals)]
#![feature(fn_traits)]
#![feature(hasher_prefixfree_extras)]
#![feature(hint_assert_unchecked)]
#![feature(inplace_iteration)]
#![feature(iter_advance_by)]
#![feature(iter_next_chunk)]
Expand Down
97 changes: 74 additions & 23 deletions library/core/src/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,41 +111,92 @@ pub const unsafe fn unreachable_unchecked() -> ! {

/// Makes a *soundness* promise to the compiler that `cond` holds.
///
/// This may allow the optimizer to simplify things,
/// but it might also make the generated code slower.
/// Either way, calling it will most likely make compilation take longer.
/// This may allow the optimizer to simplify things, but it might also make the generated code
/// slower. Either way, calling it will most likely make compilation take longer.
///
/// This is a situational tool for micro-optimization, and is allowed to do nothing.
/// Any use should come with a repeatable benchmark to show the value
/// and allow removing it later should the optimizer get smarter and no longer need it.
/// You may know this from other places as
/// [`llvm.assume`](https://llvm.org/docs/LangRef.html#llvm-assume-intrinsic) or, in C,
/// [`__builtin_assume`](https://clang.llvm.org/docs/LanguageExtensions.html#builtin-assume).
///
/// The more complicated the condition the less likely this is to be fruitful.
/// For example, `assert_unchecked(foo.is_sorted())` is a complex enough value
/// that the compiler is unlikely to be able to take advantage of it.
/// This promotes a correctness requirement to a soundness requirement. Don't do that without
/// very good reason.
///
/// There's also no need to `assert_unchecked` basic properties of things. For
/// example, the compiler already knows the range of `count_ones`, so there's no
/// benefit to `let n = u32::count_ones(x); assert_unchecked(n <= u32::BITS);`.
/// # Usage
///
/// If ever you're tempted to write `assert_unchecked(false)`, then you're
/// actually looking for [`unreachable_unchecked()`].
/// This is a situational tool for micro-optimization, and is allowed to do nothing. Any use
/// should come with a repeatable benchmark to show the value, with the expectation to drop it
/// later should the optimizer get smarter and no longer need it.
///
/// You may know this from other places
/// as [`llvm.assume`](https://llvm.org/docs/LangRef.html#llvm-assume-intrinsic)
/// or [`__builtin_assume`](https://clang.llvm.org/docs/LanguageExtensions.html#builtin-assume).
/// The more complicated the condition, the less likely this is to be useful. For example,
/// `assert_unchecked(foo.is_sorted())` is a complex enough value that the compiler is unlikely
/// to be able to take advantage of it.
///
/// This promotes a correctness requirement to a soundness requirement.
/// Don't do that without very good reason.
/// There's also no need to `assert_unchecked` basic properties of things. For example, the
/// compiler already knows the range of `count_ones`, so there is no benefit to
/// `let n = u32::count_ones(x); assert_unchecked(n <= u32::BITS);`.
///
/// `assert_unchecked` is logically equivalent to `if !cond { unreachable_unchecked(); }`. If
/// ever you are tempted to write `assert_unchecked(false)`, you should instead use
/// [`unreachable_unchecked()`] directly.
///
/// # Safety
///
/// `cond` must be `true`. It's immediate UB to call this with `false`.
/// `cond` must be `true`. It is immediate UB to call this with `false`.
///
/// # Example
///
/// ```
/// use core::hint;
///
/// /// # Safety
/// ///
/// /// `p` must be nonnull and valid
/// pub unsafe fn next_value(p: *const i32) -> i32 {
/// // SAFETY: caller invariants guarantee that `p` is not null
/// unsafe { hint::assert_unchecked(!p.is_null()) }
///
/// if p.is_null() {
/// return -1;
/// } else {
/// // SAFETY: caller invariants guarantee that `p` is valid
/// unsafe { *p + 1 }
/// }
/// }
/// ```
///
/// Without the `assert_unchecked`, the above function produces the following with optimizations
/// enabled:
///
/// ```asm
/// next_value:
/// test rdi, rdi
/// je .LBB0_1
/// mov eax, dword ptr [rdi]
/// inc eax
/// ret
/// .LBB0_1:
/// mov eax, -1
/// ret
/// ```
///
/// Adding the assertion allows the optimizer to remove the extra check:
///
/// ```asm
/// next_value:
/// mov eax, dword ptr [rdi]
/// inc eax
/// ret
/// ```
///
/// This example is quite unlike anything that would be used in the real world: it is redundant
/// to put an an assertion right next to code that checks the same thing, and dereferencing a
/// pointer already has the builtin assumption that it is nonnull. However, it illustrates the
/// kind of changes the optimizer can make even when the behavior is less obviously related.
#[track_caller]
#[inline(always)]
#[doc(alias = "assume")]
#[track_caller]
#[unstable(feature = "hint_assert_unchecked", issue = "119131")]
#[rustc_const_unstable(feature = "const_hint_assert_unchecked", issue = "119131")]
#[stable(feature = "hint_assert_unchecked", since = "CURRENT_RUSTC_VERSION")]
#[rustc_const_stable(feature = "hint_assert_unchecked", since = "CURRENT_RUSTC_VERSION")]
pub const unsafe fn assert_unchecked(cond: bool) {
// SAFETY: The caller promised `cond` is true.
unsafe {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ extern "rust-intrinsic" {
/// not be used if the invariant can be discovered by the optimizer on its
/// own, or if it does not enable any significant optimizations.
///
/// This intrinsic does not have a stable counterpart.
/// The stabilized version of this intrinsic is [`core::hint::assert_unchecked`].
#[rustc_const_stable(feature = "const_assume", since = "1.77.0")]
#[rustc_nounwind]
#[unstable(feature = "core_intrinsics", issue = "none")]
Expand Down
1 change: 0 additions & 1 deletion library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@
#![feature(const_fmt_arguments_new)]
#![feature(const_hash)]
#![feature(const_heap)]
#![feature(const_hint_assert_unchecked)]
#![feature(const_index_range_slice_index)]
#![feature(const_int_from_str)]
#![feature(const_intrinsic_copy)]
Expand Down
3 changes: 1 addition & 2 deletions library/core/src/sync/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,6 @@ impl AtomicBool {
/// # Examples
///
/// ```
/// #![feature(atomic_bool_fetch_not)]
/// use std::sync::atomic::{AtomicBool, Ordering};
///
/// let foo = AtomicBool::new(true);
Expand All @@ -1081,7 +1080,7 @@ impl AtomicBool {
/// assert_eq!(foo.load(Ordering::SeqCst), true);
/// ```
#[inline]
#[unstable(feature = "atomic_bool_fetch_not", issue = "98485")]
#[stable(feature = "atomic_bool_fetch_not", since = "CURRENT_RUSTC_VERSION")]
#[cfg(target_has_atomic = "8")]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub fn fetch_not(&self, order: Ordering) -> bool {
Expand Down
1 change: 0 additions & 1 deletion library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,6 @@
#![feature(fmt_internals)]
#![feature(hasher_prefixfree_extras)]
#![feature(hashmap_internals)]
#![feature(hint_assert_unchecked)]
#![feature(ip)]
#![feature(maybe_uninit_slice)]
#![feature(maybe_uninit_write_slice)]
Expand Down
2 changes: 1 addition & 1 deletion src/tools/remote-test-client/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fn spawn_emulator(target: &str, server: &Path, tmpdir: &Path, rootfs: Option<Pat

// Wait for the emulator to come online
loop {
let dur = Duration::from_millis(100);
let dur = Duration::from_millis(2000);
if let Ok(mut client) = TcpStream::connect(&device_address) {
t!(client.set_read_timeout(Some(dur)));
t!(client.set_write_timeout(Some(dur)));
Expand Down
8 changes: 6 additions & 2 deletions tests/run-make/inaccessible-temp-dir/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@
// use a directory with non-existing parent like `/does-not-exist/output`.
// See https://github.com/rust-lang/rust/issues/66530

//@ ignore-riscv64
// FIXME: The riscv build container runs as root, and can always write
// into `inaccessible/tmp`. Ideally, the riscv64-gnu docker container
// would use a non-root user, but this leads to issues with
// `mkfs.ext4 -d`, as well as mounting a loop device for the rootfs.
//@ ignore-arm
// Reason: linker error on `armhf-gnu`
//@ ignore-windows
// Reason: `set_readonly` has no effect on directories
// and does not prevent modification.

use run_make_support::{fs_wrapper, rustc, target, test_while_readonly};
use run_make_support::{fs_wrapper, rustc, test_while_readonly};

fn main() {
// Create an inaccessible directory.
Expand All @@ -28,7 +33,6 @@ fn main() {
// Run rustc with `-Z temps-dir` set to a directory *inside* the inaccessible one,
// so that it can't create `tmp`.
rustc()
.target(target())
.input("program.rs")
.arg("-Ztemps-dir=inaccessible/tmp")
.run_fail()
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/const-generics/generic_const_exprs/adt_wf_hang.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(generic_const_exprs)]
#![feature(adt_const_params)]
#![allow(incomplete_features)]
#![allow(dead_code)]

#[derive(PartialEq, Eq)]
struct U;

struct S<const N: U>()
where
S<{ U }>:;
//~^ ERROR: overflow evaluating the requirement `S<{ U }> well-formed`

fn main() {}
Loading

0 comments on commit 2db4ff4

Please sign in to comment.