Skip to content

Commit

Permalink
Auto merge of #104455 - the8472:dont-drain-on-drop, r=Amanieu
Browse files Browse the repository at this point in the history
Don't drain-on-drop in DrainFilter impls of various collections.

This removes drain-on-drop behavior from various unstable DrainFilter impls (not yet for HashSet/Map) because that behavior [is problematic](#43244 (comment)) (because it can lead to panic-in-drop when user closures panic) and may become forbidden if [this draft RFC passes](rust-lang/rfcs#3288).

closes #101122

[ACP](rust-lang/libs-team#136)

affected tracking issues
* #43244
* #70530
* #59618

Related hashbrown update: rust-lang/hashbrown#374
  • Loading branch information
bors committed Jun 15, 2023
2 parents 8c74a5d + 0c5f442 commit 6ee4265
Show file tree
Hide file tree
Showing 43 changed files with 449 additions and 577 deletions.
17 changes: 16 additions & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ dependencies = [
"rand_xorshift",
]

[[package]]
name = "allocator-api2"
version = "0.2.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c4f263788a35611fba42eb41ff811c5d0360c58b97402570312a350736e2542e"

[[package]]
name = "ammonia"
version = "3.2.0"
Expand Down Expand Up @@ -1522,6 +1528,15 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "33ff8ae62cd3a9102e5637afc8452c55acf3844001bd5374e0b0bd7b6616c038"
dependencies = [
"ahash 0.8.2",
]

[[package]]
name = "hashbrown"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2c6201b9ff9fd90a5a3bac2e56a830d0caa509576f0e503818ee82c181b3437a"
dependencies = [
"allocator-api2",
"compiler_builtins",
"rustc-std-workspace-alloc",
"rustc-std-workspace-core",
Expand Down Expand Up @@ -4633,7 +4648,7 @@ dependencies = [
"core",
"dlmalloc",
"fortanix-sgx-abi",
"hashbrown 0.13.1",
"hashbrown 0.14.0",
"hermit-abi 0.3.0",
"libc",
"miniz_oxide",
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(array_windows)]
#![feature(drain_filter)]
#![feature(extract_if)]
#![feature(if_let_guard)]
#![feature(let_chains)]
#![feature(never_type)]
Expand Down Expand Up @@ -1399,7 +1399,7 @@ impl HandlerInner {
!self.emitted_diagnostics.insert(diagnostic_hash)
};

diagnostic.children.drain_filter(already_emitted_sub).for_each(|_| {});
diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {});

self.emitter.emit_diagnostic(diagnostic);
if diagnostic.is_error() {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_hir_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ This API is completely unstable and subject to change.
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(box_patterns)]
#![feature(control_flow_enum)]
#![feature(drain_filter)]
#![feature(hash_drain_filter)]
#![feature(if_let_guard)]
#![feature(is_sorted)]
#![feature(iter_intersperse)]
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,20 +753,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return;
}

errors.drain_filter(|error| {
errors.retain(|error| {
let Error::Invalid(
provided_idx,
expected_idx,
Compatibility::Incompatible(Some(e)),
) = error else { return false };
) = error else { return true };
let (provided_ty, provided_span) = provided_arg_tys[*provided_idx];
let trace =
mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308) {
self.err_ctxt().report_and_explain_type_error(trace, *e).emit();
return true;
return false;
}
false
true
});

// We're done if we found errors, but we already emitted them.
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#![feature(box_patterns)]
#![feature(min_specialization)]
#![feature(control_flow_enum)]
#![feature(drain_filter)]
#![feature(option_as_slice)]
#![allow(rustc::potential_query_instability)]
#![recursion_limit = "256"]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(decl_macro)]
#![feature(drain_filter)]
#![feature(extract_if)]
#![feature(generators)]
#![feature(iter_from_generator)]
#![feature(let_chains)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/native_libs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ impl<'tcx> Collector<'tcx> {
// can move them to the end of the list below.
let mut existing = self
.libs
.drain_filter(|lib| {
.extract_if(|lib| {
if lib.name.as_str() == passed_lib.name {
// FIXME: This whole logic is questionable, whether modifiers are
// involved or not, library reordering and kind overriding without
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
#![feature(try_reserve_kind)]
#![feature(nonzero_ops)]
#![feature(decl_macro)]
#![feature(drain_filter)]
#![feature(extract_if)]
#![feature(intra_doc_pointers)]
#![feature(yeet_expr)]
#![feature(result_option_inspect)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ pub fn suggest_constraining_type_params<'a>(

{
let mut sized_constraints =
constraints.drain_filter(|(_, def_id)| *def_id == tcx.lang_items().sized_trait());
constraints.extract_if(|(_, def_id)| *def_id == tcx.lang_items().sized_trait());
if let Some((constraint, def_id)) = sized_constraints.next() {
applicability = Applicability::MaybeIncorrect;

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
#![feature(box_patterns)]
#![feature(drain_filter)]
#![feature(is_sorted)]
#![feature(let_chains)]
#![feature(map_try_insert)]
Expand Down
14 changes: 6 additions & 8 deletions compiler/rustc_mir_transform/src/sroa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,26 +436,24 @@ impl<'tcx, 'll> MutVisitor<'tcx> for ReplacementVisitor<'tcx, 'll> {
VarDebugInfoContents::Composite { ty: _, ref mut fragments } => {
let mut new_fragments = Vec::new();
debug!(?fragments);
fragments
.drain_filter(|fragment| {
if let Some(repl) =
fragments.retain_mut(|fragment| {
if let Some(repl) =
self.replacements.replace_place(self.tcx, fragment.contents.as_ref())
{
fragment.contents = repl;
false
true
} else if let Some(local) = fragment.contents.as_local()
&& let Some(frg) = self.gather_debug_info_fragments(local)
{
new_fragments.extend(frg.into_iter().map(|mut f| {
f.projection.splice(0..0, fragment.projection.iter().copied());
f
}));
true
} else {
false
} else {
true
}
})
.for_each(drop);
});
debug!(?fragments);
debug!(?new_fragments);
fragments.extend(new_fragments);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2494,7 +2494,7 @@ fn show_candidates(
for path_strings in [&mut accessible_path_strings, &mut inaccessible_path_strings] {
path_strings.sort_by(|a, b| a.0.cmp(&b.0));
let core_path_strings =
path_strings.drain_filter(|p| p.0.starts_with("core::")).collect::<Vec<_>>();
path_strings.extract_if(|p| p.0.starts_with("core::")).collect::<Vec<_>>();
path_strings.extend(core_path_strings);
path_strings.dedup_by(|a, b| a.0 == b.0);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
// Try to filter out intrinsics candidates, as long as we have
// some other candidates to suggest.
let intrinsic_candidates: Vec<_> = candidates
.drain_filter(|sugg| {
.extract_if(|sugg| {
let path = path_names_to_string(&sugg.path);
path.starts_with("core::intrinsics::") || path.starts_with("std::intrinsics::")
})
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(assert_matches)]
#![feature(box_patterns)]
#![feature(drain_filter)]
#![feature(extract_if)]
#![feature(if_let_guard)]
#![feature(iter_intersperse)]
#![feature(let_chains)]
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_trait_selection/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
#![feature(associated_type_bounds)]
#![feature(box_patterns)]
#![feature(control_flow_enum)]
#![feature(drain_filter)]
#![feature(hash_drain_filter)]
#![feature(extract_if)]
#![feature(let_chains)]
#![feature(if_let_guard)]
#![feature(never_type)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ pub fn normalize_param_env_or_error<'tcx>(
// This works fairly well because trait matching does not actually care about param-env
// TypeOutlives predicates - these are normally used by regionck.
let outlives_predicates: Vec<_> = predicates
.drain_filter(|predicate| {
.extract_if(|predicate| {
matches!(
predicate.kind().skip_binder(),
ty::PredicateKind::Clause(ty::Clause::TypeOutlives(..))
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1170,11 +1170,11 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
};

let mut deduped: SsoHashSet<_> = Default::default();
result.obligations.drain_filter(|projected_obligation| {
result.obligations.retain(|projected_obligation| {
if !deduped.insert(projected_obligation.clone()) {
return true;
return false;
}
false
true
});

if use_cache {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_traits/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
#![feature(let_chains)]
#![feature(drain_filter)]
#![recursion_limit = "256"]

#[macro_use]
Expand Down
12 changes: 6 additions & 6 deletions library/alloc/benches/btree/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,15 +385,15 @@ pub fn clone_slim_100_and_clear(b: &mut Bencher) {
#[bench]
pub fn clone_slim_100_and_drain_all(b: &mut Bencher) {
let src = slim_map(100);
b.iter(|| src.clone().drain_filter(|_, _| true).count())
b.iter(|| src.clone().extract_if(|_, _| true).count())
}

#[bench]
pub fn clone_slim_100_and_drain_half(b: &mut Bencher) {
let src = slim_map(100);
b.iter(|| {
let mut map = src.clone();
assert_eq!(map.drain_filter(|i, _| i % 2 == 0).count(), 100 / 2);
assert_eq!(map.extract_if(|i, _| i % 2 == 0).count(), 100 / 2);
assert_eq!(map.len(), 100 / 2);
})
}
Expand Down Expand Up @@ -456,15 +456,15 @@ pub fn clone_slim_10k_and_clear(b: &mut Bencher) {
#[bench]
pub fn clone_slim_10k_and_drain_all(b: &mut Bencher) {
let src = slim_map(10_000);
b.iter(|| src.clone().drain_filter(|_, _| true).count())
b.iter(|| src.clone().extract_if(|_, _| true).count())
}

#[bench]
pub fn clone_slim_10k_and_drain_half(b: &mut Bencher) {
let src = slim_map(10_000);
b.iter(|| {
let mut map = src.clone();
assert_eq!(map.drain_filter(|i, _| i % 2 == 0).count(), 10_000 / 2);
assert_eq!(map.extract_if(|i, _| i % 2 == 0).count(), 10_000 / 2);
assert_eq!(map.len(), 10_000 / 2);
})
}
Expand Down Expand Up @@ -527,15 +527,15 @@ pub fn clone_fat_val_100_and_clear(b: &mut Bencher) {
#[bench]
pub fn clone_fat_val_100_and_drain_all(b: &mut Bencher) {
let src = fat_val_map(100);
b.iter(|| src.clone().drain_filter(|_, _| true).count())
b.iter(|| src.clone().extract_if(|_, _| true).count())
}

#[bench]
pub fn clone_fat_val_100_and_drain_half(b: &mut Bencher) {
let src = fat_val_map(100);
b.iter(|| {
let mut map = src.clone();
assert_eq!(map.drain_filter(|i, _| i % 2 == 0).count(), 100 / 2);
assert_eq!(map.extract_if(|i, _| i % 2 == 0).count(), 100 / 2);
assert_eq!(map.len(), 100 / 2);
})
}
Expand Down
8 changes: 4 additions & 4 deletions library/alloc/benches/btree/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ pub fn clone_100_and_clear(b: &mut Bencher) {
#[bench]
pub fn clone_100_and_drain_all(b: &mut Bencher) {
let src = slim_set(100);
b.iter(|| src.clone().drain_filter(|_| true).count())
b.iter(|| src.clone().extract_if(|_| true).count())
}

#[bench]
pub fn clone_100_and_drain_half(b: &mut Bencher) {
let src = slim_set(100);
b.iter(|| {
let mut set = src.clone();
assert_eq!(set.drain_filter(|i| i % 2 == 0).count(), 100 / 2);
assert_eq!(set.extract_if(|i| i % 2 == 0).count(), 100 / 2);
assert_eq!(set.len(), 100 / 2);
})
}
Expand Down Expand Up @@ -140,15 +140,15 @@ pub fn clone_10k_and_clear(b: &mut Bencher) {
#[bench]
pub fn clone_10k_and_drain_all(b: &mut Bencher) {
let src = slim_set(10_000);
b.iter(|| src.clone().drain_filter(|_| true).count())
b.iter(|| src.clone().extract_if(|_| true).count())
}

#[bench]
pub fn clone_10k_and_drain_half(b: &mut Bencher) {
let src = slim_set(10_000);
b.iter(|| {
let mut set = src.clone();
assert_eq!(set.drain_filter(|i| i % 2 == 0).count(), 10_000 / 2);
assert_eq!(set.extract_if(|i| i % 2 == 0).count(), 10_000 / 2);
assert_eq!(set.len(), 10_000 / 2);
})
}
Expand Down
2 changes: 1 addition & 1 deletion library/alloc/benches/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Disabling on android for the time being
// See https://github.com/rust-lang/rust/issues/73535#event-3477699747
#![cfg(not(target_os = "android"))]
#![feature(btree_drain_filter)]
#![feature(btree_extract_if)]
#![feature(iter_next_chunk)]
#![feature(repr_simd)]
#![feature(slice_partition_dedup)]
Expand Down
Loading

0 comments on commit 6ee4265

Please sign in to comment.