Skip to content

Rollup of 7 pull requests #135095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Jan 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
69942f0
core: implement `bool::select_unpredictable`
joboet Dec 6, 2024
13c77ba
core: improve comments
joboet Dec 9, 2024
49d76b8
core: use public method instead of instrinsic
joboet Dec 9, 2024
ab5446c
Actually use self-contained lld in bootstrap when `use-lld = "self-co…
Kobzol Jan 1, 2025
3d69dd1
Do not pass LLD flags to targets flags of compiletest
Kobzol Jan 1, 2025
c298388
Report impl has stricter requirements even when RPITIT inference gets…
compiler-errors Jan 3, 2025
6628c4b
bootstrap: support `./x check run-make-support`
jieyouxu Jan 3, 2025
e839d05
remove unused function params
matthiaskrgr Jan 3, 2025
b806dcc
const-in-pattern: test that the PartialEq impl does not need to be const
RalfJung Jan 3, 2025
8f3aa35
add codegen test for `bool::select_unpredictable`
joboet Jan 3, 2025
ed00524
Update carrying_mul_add test to tolerate `nuw`
maurer Jan 3, 2025
695da5b
Rollup merge of #133964 - joboet:select_unpredictable, r=tgross35
matthiaskrgr Jan 4, 2025
8744b44
Rollup merge of #135001 - Kobzol:bootstrap-mcp-510, r=onur-ozkan
matthiaskrgr Jan 4, 2025
b0b54f2
Rollup merge of #135055 - compiler-errors:rpitit-infer-in-stricter-im…
matthiaskrgr Jan 4, 2025
966a5be
Rollup merge of #135064 - RalfJung:const-in-pat-partial-eq-not-const,…
matthiaskrgr Jan 4, 2025
e4cc2db
Rollup merge of #135066 - jieyouxu:check-run-make-support, r=clubby789
matthiaskrgr Jan 4, 2025
725b799
Rollup merge of #135069 - matthiaskrgr:param_rec_usage, r=jieyouxu
matthiaskrgr Jan 4, 2025
75e412b
Rollup merge of #135084 - maurer:nuw, r=nikic
matthiaskrgr Jan 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,26 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
let infcx = &tcx.infer_ctxt().build(TypingMode::non_body_analysis());
let ocx = ObligationCtxt::new_with_diagnostics(infcx);

// Check that the where clauses of the impl are satisfied by the hybrid param env.
// You might ask -- what does this have to do with RPITIT inference? Nothing.
// We check these because if the where clauses of the signatures do not match
// up, then we don't want to give spurious other errors that point at the RPITITs.
// They're not necessary to check, though, because we already check them in
// `compare_method_predicate_entailment`.
let impl_m_own_bounds = tcx.predicates_of(impl_m_def_id).instantiate_own_identity();
for (predicate, span) in impl_m_own_bounds {
let normalize_cause = traits::ObligationCause::misc(span, impl_m_def_id);
let predicate = ocx.normalize(&normalize_cause, param_env, predicate);

let cause =
ObligationCause::new(span, impl_m_def_id, ObligationCauseCode::CompareImplItem {
impl_item_def_id: impl_m_def_id,
trait_item_def_id: trait_m.def_id,
kind: impl_m.kind,
});
ocx.register_obligation(traits::Obligation::new(tcx, cause, param_env, predicate));
}

// Normalize the impl signature with fresh variables for lifetime inference.
let misc_cause = ObligationCause::misc(return_span, impl_m_def_id);
let impl_sig = ocx.normalize(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ impl<'tcx> Ty<'tcx> {
/// description in error messages. This is used in the primary span label. Beyond what
/// `is_simple_ty` includes, it also accepts ADTs with no type arguments and references to
/// ADTs with no type arguments.
pub fn is_simple_text(self, tcx: TyCtxt<'tcx>) -> bool {
pub fn is_simple_text(self) -> bool {
match self.kind() {
Adt(_, args) => args.non_erasable_generics().next().is_none(),
Ref(_, ty, _) => ty.is_simple_text(tcx),
Ref(_, ty, _) => ty.is_simple_text(),
_ => self.is_simple_ty(),
}
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,10 @@ fn type_has_partial_eq_impl<'tcx>(
// `PartialEq` for some lifetime but *not* for `'static`? If this ever becomes a problem
// we'll need to leave some sort of trace of this requirement in the MIR so that borrowck
// can ensure that the type really implements `PartialEq`.
// We also do *not* require `const PartialEq`, not even in `const fn`. This violates the model
// that patterns can only do things that the code could also do without patterns, but it is
// needed for backwards compatibility. The actual pattern matching compares primitive values,
// `PartialEq::eq` never gets invoked, so there's no risk of us running non-const code.
(
infcx.predicate_must_hold_modulo_regions(&partial_eq_obligation),
automatically_derived,
Expand Down
28 changes: 9 additions & 19 deletions compiler/rustc_mir_transform/src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1822,9 +1822,6 @@ impl<'tcx> Visitor<'tcx> for EnsureCoroutineFieldAssignmentsNeverAlias<'_> {
fn check_suspend_tys<'tcx>(tcx: TyCtxt<'tcx>, layout: &CoroutineLayout<'tcx>, body: &Body<'tcx>) {
let mut linted_tys = FxHashSet::default();

// We want a user-facing param-env.
let param_env = tcx.param_env(body.source.def_id());

for (variant, yield_source_info) in
layout.variant_fields.iter().zip(&layout.variant_source_info)
{
Expand All @@ -1838,7 +1835,7 @@ fn check_suspend_tys<'tcx>(tcx: TyCtxt<'tcx>, layout: &CoroutineLayout<'tcx>, bo
continue;
};

check_must_not_suspend_ty(tcx, decl.ty, hir_id, param_env, SuspendCheckData {
check_must_not_suspend_ty(tcx, decl.ty, hir_id, SuspendCheckData {
source_span: decl.source_info.span,
yield_span: yield_source_info.span,
plural_len: 1,
Expand Down Expand Up @@ -1868,7 +1865,6 @@ fn check_must_not_suspend_ty<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
hir_id: hir::HirId,
param_env: ty::ParamEnv<'tcx>,
data: SuspendCheckData<'_>,
) -> bool {
if ty.is_unit() {
Expand All @@ -1883,16 +1879,13 @@ fn check_must_not_suspend_ty<'tcx>(
ty::Adt(_, args) if ty.is_box() => {
let boxed_ty = args.type_at(0);
let allocator_ty = args.type_at(1);
check_must_not_suspend_ty(tcx, boxed_ty, hir_id, param_env, SuspendCheckData {
check_must_not_suspend_ty(tcx, boxed_ty, hir_id, SuspendCheckData {
descr_pre: &format!("{}boxed ", data.descr_pre),
..data
}) || check_must_not_suspend_ty(
tcx,
allocator_ty,
hir_id,
param_env,
SuspendCheckData { descr_pre: &format!("{}allocator ", data.descr_pre), ..data },
)
}) || check_must_not_suspend_ty(tcx, allocator_ty, hir_id, SuspendCheckData {
descr_pre: &format!("{}allocator ", data.descr_pre),
..data
})
}
ty::Adt(def, _) => check_must_not_suspend_def(tcx, def.did(), hir_id, data),
// FIXME: support adding the attribute to TAITs
Expand Down Expand Up @@ -1937,7 +1930,7 @@ fn check_must_not_suspend_ty<'tcx>(
let mut has_emitted = false;
for (i, ty) in fields.iter().enumerate() {
let descr_post = &format!(" in tuple element {i}");
if check_must_not_suspend_ty(tcx, ty, hir_id, param_env, SuspendCheckData {
if check_must_not_suspend_ty(tcx, ty, hir_id, SuspendCheckData {
descr_post,
..data
}) {
Expand All @@ -1948,7 +1941,7 @@ fn check_must_not_suspend_ty<'tcx>(
}
ty::Array(ty, len) => {
let descr_pre = &format!("{}array{} of ", data.descr_pre, plural_suffix);
check_must_not_suspend_ty(tcx, ty, hir_id, param_env, SuspendCheckData {
check_must_not_suspend_ty(tcx, ty, hir_id, SuspendCheckData {
descr_pre,
// FIXME(must_not_suspend): This is wrong. We should handle printing unevaluated consts.
plural_len: len.try_to_target_usize(tcx).unwrap_or(0) as usize + 1,
Expand All @@ -1959,10 +1952,7 @@ fn check_must_not_suspend_ty<'tcx>(
// may not be considered live across the await point.
ty::Ref(_region, ty, _mutability) => {
let descr_pre = &format!("{}reference{} to ", data.descr_pre, plural_suffix);
check_must_not_suspend_ty(tcx, ty, hir_id, param_env, SuspendCheckData {
descr_pre,
..data
})
check_must_not_suspend_ty(tcx, ty, hir_id, SuspendCheckData { descr_pre, ..data })
}
_ => false,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1496,8 +1496,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
ValuePairs::Terms(ExpectedFound { expected, found }) => {
match (expected.unpack(), found.unpack()) {
(ty::TermKind::Ty(expected), ty::TermKind::Ty(found)) => {
let is_simple_err = expected.is_simple_text(self.tcx)
&& found.is_simple_text(self.tcx);
let is_simple_err =
expected.is_simple_text() && found.is_simple_text();
OpaqueTypesVisitor::visit_expected_found(
self.tcx, expected, found, span,
)
Expand Down Expand Up @@ -1736,8 +1736,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
(true, _) => format!(" ({})", ty.sort_string(self.tcx)),
(false, _) => "".to_string(),
};
if !(values.expected.is_simple_text(self.tcx)
&& values.found.is_simple_text(self.tcx))
if !(values.expected.is_simple_text() && values.found.is_simple_text())
|| (exp_found.is_some_and(|ef| {
// This happens when the type error is a subset of the expectation,
// like when you have two references but one is `usize` and the other
Expand Down
48 changes: 48 additions & 0 deletions library/core/src/bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,52 @@ impl bool {
pub fn then<T, F: FnOnce() -> T>(self, f: F) -> Option<T> {
if self { Some(f()) } else { None }
}

/// Returns either `true_val` or `false_val` depending on the value of
/// `self`, with a hint to the compiler that `self` is unlikely
/// to be correctly predicted by a CPU’s branch predictor.
///
/// This method is functionally equivalent to
/// ```ignore (this is just for illustrative purposes)
/// fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
/// if b { true_val } else { false_val }
/// }
/// ```
/// but might generate different assembly. In particular, on platforms with
/// a conditional move or select instruction (like `cmov` on x86 or `csel`
/// on ARM) the optimizer might use these instructions to avoid branches,
/// which can benefit performance if the branch predictor is struggling
/// with predicting `condition`, such as in an implementation of binary
/// search.
///
/// Note however that this lowering is not guaranteed (on any platform) and
/// should not be relied upon when trying to write constant-time code. Also
/// be aware that this lowering might *decrease* performance if `condition`
/// is well-predictable. It is advisable to perform benchmarks to tell if
/// this function is useful.
///
/// # Examples
///
/// Distribute values evenly between two buckets:
/// ```
/// #![feature(select_unpredictable)]
///
/// use std::hash::BuildHasher;
///
/// fn append<H: BuildHasher>(hasher: &H, v: i32, bucket_one: &mut Vec<i32>, bucket_two: &mut Vec<i32>) {
/// let hash = hasher.hash_one(&v);
/// let bucket = (hash % 2 == 0).select_unpredictable(bucket_one, bucket_two);
/// bucket.push(v);
/// }
/// # let hasher = std::collections::hash_map::RandomState::new();
/// # let mut bucket_one = Vec::new();
/// # let mut bucket_two = Vec::new();
/// # append(&hasher, 42, &mut bucket_one, &mut bucket_two);
/// # assert_eq!(bucket_one.len() + bucket_two.len(), 1);
/// ```
#[inline(always)]
#[unstable(feature = "select_unpredictable", issue = "133962")]
pub fn select_unpredictable<T>(self, true_val: T, false_val: T) -> T {
crate::intrinsics::select_unpredictable(self, true_val, false_val)
}
}
2 changes: 1 addition & 1 deletion library/core/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1545,7 +1545,7 @@ pub const fn unlikely(b: bool) -> bool {
/// Therefore, implementations must not require the user to uphold
/// any safety invariants.
///
/// This intrinsic does not have a stable counterpart.
/// The public form of this instrinsic is [`bool::select_unpredictable`].
#[unstable(feature = "core_intrinsics", issue = "none")]
#[rustc_intrinsic]
#[rustc_nounwind]
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#![stable(feature = "rust1", since = "1.0.0")]

use crate::cmp::Ordering::{self, Equal, Greater, Less};
use crate::intrinsics::{exact_div, select_unpredictable, unchecked_sub};
use crate::intrinsics::{exact_div, unchecked_sub};
use crate::mem::{self, SizedTypeProperties};
use crate::num::NonZero;
use crate::ops::{Bound, OneSidedRange, Range, RangeBounds, RangeInclusive};
Expand Down Expand Up @@ -2835,7 +2835,7 @@ impl<T> [T] {
// Binary search interacts poorly with branch prediction, so force
// the compiler to use conditional moves if supported by the target
// architecture.
base = select_unpredictable(cmp == Greater, base, mid);
base = (cmp == Greater).select_unpredictable(base, mid);

// This is imprecise in the case where `size` is odd and the
// comparison returns Greater: the mid element still gets included
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/core/build_steps/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,11 @@ tool_check_step!(MiroptTestTools { path: "src/tools/miropt-test-tools" });
tool_check_step!(TestFloatParse { path: "src/etc/test-float-parse" });

tool_check_step!(Bootstrap { path: "src/bootstrap", default: false });

// `run-make-support` will be built as part of suitable run-make compiletest test steps, but support
// check to make it easier to work on.
tool_check_step!(RunMakeSupport { path: "src/tools/run-make-support", default: false });

// Compiletest is implicitly "checked" when it gets built in order to run tests,
// so this is mainly for people working on compiletest to run locally.
tool_check_step!(Compiletest { path: "src/tools/compiletest", default: false });
Expand Down
1 change: 0 additions & 1 deletion src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,6 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the

let mut targetflags = flags;
targetflags.push(format!("-Lnative={}", builder.test_helpers_out(target).display()));
targetflags.extend(linker_flags(builder, compiler.host, LldThreads::No));
for flag in targetflags {
cmd.arg("--target-rustcflags").arg(flag);
}
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/src/core/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,7 @@ impl<'a> Builder<'a> {
check::RustAnalyzer,
check::TestFloatParse,
check::Bootstrap,
check::RunMakeSupport,
check::Compiletest,
),
Kind::Test => describe!(
Expand Down
15 changes: 14 additions & 1 deletion src/bootstrap/src/utils/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,20 @@ pub fn linker_flags(
) -> Vec<String> {
let mut args = vec![];
if !builder.is_lld_direct_linker(target) && builder.config.lld_mode.is_used() {
args.push(String::from("-Clink-arg=-fuse-ld=lld"));
match builder.config.lld_mode {
LldMode::External => {
args.push("-Clinker-flavor=gnu-lld-cc".to_string());
// FIXME(kobzol): remove this flag once MCP510 gets stabilized
args.push("-Zunstable-options".to_string());
}
LldMode::SelfContained => {
args.push("-Clinker-flavor=gnu-lld-cc".to_string());
args.push("-Clink-self-contained=+linker".to_string());
// FIXME(kobzol): remove this flag once MCP510 gets stabilized
args.push("-Zunstable-options".to_string());
}
LldMode::Unused => unreachable!(),
};

if matches!(lld_threads, LldThreads::No) {
args.push(format!(
Expand Down
35 changes: 35 additions & 0 deletions tests/codegen/bool-select-unpredictable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//@ compile-flags: -O

#![feature(select_unpredictable)]
#![crate_type = "lib"]

#[no_mangle]
pub fn test_int(p: bool, a: u64, b: u64) -> u64 {
// CHECK-LABEL: define{{.*}} @test_int
// CHECK: select i1 %p, i64 %a, i64 %b, !unpredictable
p.select_unpredictable(a, b)
}

#[no_mangle]
pub fn test_pair(p: bool, a: (u64, u64), b: (u64, u64)) -> (u64, u64) {
// CHECK-LABEL: define{{.*}} @test_pair
// CHECK: select i1 %p, {{.*}}, !unpredictable
p.select_unpredictable(a, b)
}

struct Large {
e: [u64; 100],
}

#[no_mangle]
pub fn test_struct(p: bool, a: Large, b: Large) -> Large {
// CHECK-LABEL: define{{.*}} @test_struct
// CHECK: select i1 %p, {{.*}}, !unpredictable
p.select_unpredictable(a, b)
}

#[no_mangle]
pub fn test_zst(p: bool, a: (), b: ()) -> () {
// CHECK-LABEL: define{{.*}} @test_zst
p.select_unpredictable(a, b)
}
4 changes: 2 additions & 2 deletions tests/codegen/intrinsics/carrying_mul_add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub unsafe fn cma_u128(a: u128, b: u128, c: u128, d: u128) -> (u128, u128) {
// RAW: [[PAIR0:%.+]] = insertvalue { i128, i128 } poison, i128 [[LOW]], 0
// RAW: [[PAIR1:%.+]] = insertvalue { i128, i128 } [[PAIR0]], i128 [[HIGH]], 1
// OPT: store i128 [[LOW]], ptr %_0
// OPT: [[P1:%.+]] = getelementptr inbounds i8, ptr %_0, {{i32|i64}} 16
// OPT: [[P1:%.+]] = getelementptr inbounds{{( nuw)?}} i8, ptr %_0, {{i32|i64}} 16
// OPT: store i128 [[HIGH]], ptr [[P1]]
// CHECK: ret void
carrying_mul_add(a, b, c, d)
Expand All @@ -111,7 +111,7 @@ pub unsafe fn cma_i128(a: i128, b: i128, c: i128, d: i128) -> (u128, i128) {
// RAW: [[PAIR0:%.+]] = insertvalue { i128, i128 } poison, i128 [[LOW]], 0
// RAW: [[PAIR1:%.+]] = insertvalue { i128, i128 } [[PAIR0]], i128 [[HIGH]], 1
// OPT: store i128 [[LOW]], ptr %_0
// OPT: [[P1:%.+]] = getelementptr inbounds i8, ptr %_0, {{i32|i64}} 16
// OPT: [[P1:%.+]] = getelementptr inbounds{{( nuw)?}} i8, ptr %_0, {{i32|i64}} 16
// OPT: store i128 [[HIGH]], ptr [[P1]]
// CHECK: ret void
carrying_mul_add(a, b, c, d)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ struct Baz {}

impl Foo for Baz {
async fn bar<F>(&mut self, _func: F) -> ()
//~^ ERROR `F` cannot be sent between threads safely
where
F: FnMut() + Send,
//~^ impl has stricter requirements than trait
{
()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
error[E0277]: `F` cannot be sent between threads safely
--> $DIR/remove-invalid-type-bound-suggest-issue-127555.rs:13:5
error[E0276]: impl has stricter requirements than trait
--> $DIR/remove-invalid-type-bound-suggest-issue-127555.rs:15:22
|
LL | / async fn bar<F>(&mut self, _func: F) -> ()
LL | |
LL | / fn bar<F>(&mut self, func: F) -> impl std::future::Future<Output = ()> + Send
LL | | where
LL | | F: FnMut() + Send,
| |__________________________^ `F` cannot be sent between threads safely
|
note: required by a bound in `<Baz as Foo>::bar`
--> $DIR/remove-invalid-type-bound-suggest-issue-127555.rs:16:22
|
LL | async fn bar<F>(&mut self, _func: F) -> ()
| --- required by a bound in this associated function
LL | | F: FnMut();
| |___________________- definition of `bar` from trait
...
LL | F: FnMut() + Send,
| ^^^^ required by this bound in `<Baz as Foo>::bar`
LL | F: FnMut() + Send,
| ^^^^ impl has extra requirement `F: Send`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
For more information about this error, try `rustc --explain E0276`.
Loading
Loading