From 7731135af25495bf8eb1cd2f599755f7a0bba37d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 Mar 2024 20:18:46 +0100 Subject: [PATCH 01/22] alloc::Layout: explicitly document size invariant on the type level --- library/core/src/alloc/layout.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/core/src/alloc/layout.rs b/library/core/src/alloc/layout.rs index 2a02870e30be9..0b92767c93205 100644 --- a/library/core/src/alloc/layout.rs +++ b/library/core/src/alloc/layout.rs @@ -25,7 +25,9 @@ const fn size_align() -> (usize, usize) { /// An instance of `Layout` describes a particular layout of memory. /// You build a `Layout` up as an input to give to an allocator. /// -/// All layouts have an associated size and a power-of-two alignment. +/// All layouts have an associated size and a power-of-two alignment. The size, when rounded up to +/// the nearest multiple of `align`, does not overflow isize (i.e., the rounded value will always be +/// less than or equal to `isize::MAX`). /// /// (Note that layouts are *not* required to have non-zero size, /// even though `GlobalAlloc` requires that all memory requests From aa1653e5be13cdcb818ae1f933bec7ad15e33b70 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 15 Apr 2024 19:00:47 -0400 Subject: [PATCH 02/22] Rename coroutine_stalled_predicates --- compiler/rustc_hir_analysis/src/check/check.rs | 6 +++--- compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs | 2 +- compiler/rustc_hir_typeck/src/writeback.rs | 4 ++-- compiler/rustc_middle/src/ty/typeck_results.rs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 216b89fd4f150..ae3623aa329df 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -1552,8 +1552,8 @@ pub(super) fn check_coroutine_obligations( let typeck = tcx.typeck(def_id); let param_env = tcx.param_env(typeck.hir_owner.def_id); - let coroutine_interior_predicates = &typeck.coroutine_interior_predicates[&def_id]; - debug!(?coroutine_interior_predicates); + let coroutine_stalled_predicates = &typeck.coroutine_stalled_predicates[&def_id]; + debug!(?coroutine_stalled_predicates); let infcx = tcx .infer_ctxt() @@ -1566,7 +1566,7 @@ pub(super) fn check_coroutine_obligations( .build(); let mut fulfillment_cx = >::new(&infcx); - for (predicate, cause) in coroutine_interior_predicates { + for (predicate, cause) in coroutine_stalled_predicates { let obligation = Obligation::new(tcx, cause.clone(), param_env, *predicate); fulfillment_cx.register_predicate_obligation(&infcx, obligation); } diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 2d5ba447e4e68..1024dec1d9fe8 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -579,7 +579,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { debug!(?obligations); self.typeck_results .borrow_mut() - .coroutine_interior_predicates + .coroutine_stalled_predicates .insert(expr_def_id, obligations); } } diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index 6604bf094c14a..73613923e69c2 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -552,11 +552,11 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner); self.tcx().with_stable_hashing_context(move |ref hcx| { for (&expr_def_id, predicates) in - fcx_typeck_results.coroutine_interior_predicates.to_sorted(hcx, false).into_iter() + fcx_typeck_results.coroutine_stalled_predicates.to_sorted(hcx, false).into_iter() { let predicates = self.resolve(predicates.clone(), &self.fcx.tcx.def_span(expr_def_id)); - self.typeck_results.coroutine_interior_predicates.insert(expr_def_id, predicates); + self.typeck_results.coroutine_stalled_predicates.insert(expr_def_id, predicates); } }) } diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs index 995feeaa1487f..aa905466fbc87 100644 --- a/compiler/rustc_middle/src/ty/typeck_results.rs +++ b/compiler/rustc_middle/src/ty/typeck_results.rs @@ -201,7 +201,7 @@ pub struct TypeckResults<'tcx> { /// Stores the predicates that apply on coroutine witness types. /// formatting modified file tests/ui/coroutine/retain-resume-ref.rs - pub coroutine_interior_predicates: + pub coroutine_stalled_predicates: LocalDefIdMap, ObligationCause<'tcx>)>>, /// We sometimes treat byte string literals (which are of type `&[u8; N]`) @@ -243,7 +243,7 @@ impl<'tcx> TypeckResults<'tcx> { closure_min_captures: Default::default(), closure_fake_reads: Default::default(), rvalue_scopes: Default::default(), - coroutine_interior_predicates: Default::default(), + coroutine_stalled_predicates: Default::default(), treat_byte_string_as_slice: Default::default(), closure_size_eval: Default::default(), offset_of_data: Default::default(), From d29178c2efe7846fdb8c442219d7b70ae83e18e7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 15 Apr 2024 19:44:23 -0400 Subject: [PATCH 03/22] Do check_coroutine_obligations once per typeck root --- .../rustc_hir_analysis/src/check/check.rs | 47 +++--------- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 8 +-- compiler/rustc_hir_typeck/src/writeback.rs | 13 ++-- compiler/rustc_interface/src/passes.rs | 4 +- .../rustc_middle/src/ty/typeck_results.rs | 9 +-- compiler/rustc_mir_transform/src/coroutine.rs | 40 +++++++++++ tests/ui/coroutine/clone-impl.rs | 24 +++++-- tests/ui/coroutine/clone-impl.stderr | 72 +++++++++---------- 8 files changed, 118 insertions(+), 99 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index ae3623aa329df..cb0ed68edff3e 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -10,10 +10,9 @@ use rustc_hir as hir; use rustc_hir::def::{CtorKind, DefKind}; use rustc_hir::Node; use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt}; -use rustc_infer::traits::{Obligation, TraitEngineExt as _}; +use rustc_infer::traits::Obligation; use rustc_lint_defs::builtin::REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS; use rustc_middle::middle::stability::EvalResult; -use rustc_middle::traits::ObligationCauseCode; use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES}; use rustc_middle::ty::util::{Discr, InspectCoroutineFields, IntTypeExt}; @@ -26,7 +25,7 @@ use rustc_target::abi::FieldIdx; use rustc_trait_selection::traits::error_reporting::on_unimplemented::OnUnimplementedDirective; use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _; -use rustc_trait_selection::traits::{self, TraitEngine, TraitEngineExt as _}; +use rustc_trait_selection::traits::{self}; use rustc_type_ir::fold::TypeFoldable; use std::cell::LazyCell; @@ -1541,55 +1540,31 @@ fn opaque_type_cycle_error( err.emit() } -// FIXME(@lcnr): This should not be computed per coroutine, but instead once for -// each typeck root. pub(super) fn check_coroutine_obligations( tcx: TyCtxt<'_>, def_id: LocalDefId, ) -> Result<(), ErrorGuaranteed> { - debug_assert!(tcx.is_coroutine(def_id.to_def_id())); + debug_assert!(!tcx.is_typeck_child(def_id.to_def_id())); - let typeck = tcx.typeck(def_id); - let param_env = tcx.param_env(typeck.hir_owner.def_id); + let typeck_results = tcx.typeck(def_id); + let param_env = tcx.param_env(def_id); - let coroutine_stalled_predicates = &typeck.coroutine_stalled_predicates[&def_id]; - debug!(?coroutine_stalled_predicates); + debug!(?typeck_results.coroutine_stalled_predicates); let infcx = tcx .infer_ctxt() // typeck writeback gives us predicates with their regions erased. // As borrowck already has checked lifetimes, we do not need to do it again. .ignoring_regions() - // Bind opaque types to type checking root, as they should have been checked by borrowck, - // but may show up in some cases, like when (root) obligations are stalled in the new solver. - .with_opaque_type_inference(typeck.hir_owner.def_id) + .with_opaque_type_inference(def_id) .build(); - let mut fulfillment_cx = >::new(&infcx); - for (predicate, cause) in coroutine_stalled_predicates { - let obligation = Obligation::new(tcx, cause.clone(), param_env, *predicate); - fulfillment_cx.register_predicate_obligation(&infcx, obligation); - } - - if (tcx.features().unsized_locals || tcx.features().unsized_fn_params) - && let Some(coroutine) = tcx.mir_coroutine_witnesses(def_id) - { - for field_ty in coroutine.field_tys.iter() { - fulfillment_cx.register_bound( - &infcx, - param_env, - field_ty.ty, - tcx.require_lang_item(hir::LangItem::Sized, Some(field_ty.source_info.span)), - ObligationCause::new( - field_ty.source_info.span, - def_id, - ObligationCauseCode::SizedCoroutineInterior(def_id), - ), - ); - } + let ocx = ObligationCtxt::new(&infcx); + for (predicate, cause) in &typeck_results.coroutine_stalled_predicates { + ocx.register_obligation(Obligation::new(tcx, cause.clone(), param_env, *predicate)); } - let errors = fulfillment_cx.select_all_or_error(&infcx); + let errors = ocx.select_all_or_error(); debug!(?errors); if !errors.is_empty() { return Err(infcx.err_ctxt().report_fulfillment_errors(errors)); diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 1024dec1d9fe8..14e4236ad7859 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -575,12 +575,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { obligations .extend(self.fulfillment_cx.borrow_mut().drain_unstalled_obligations(&self.infcx)); - let obligations = obligations.into_iter().map(|o| (o.predicate, o.cause)).collect(); - debug!(?obligations); - self.typeck_results - .borrow_mut() - .coroutine_stalled_predicates - .insert(expr_def_id, obligations); + let obligations = obligations.into_iter().map(|o| (o.predicate, o.cause)); + self.typeck_results.borrow_mut().coroutine_stalled_predicates.extend(obligations); } } diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index 73613923e69c2..fa2bbc3dbd3a1 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -550,15 +550,10 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_coroutine_interior(&mut self) { let fcx_typeck_results = self.fcx.typeck_results.borrow(); assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner); - self.tcx().with_stable_hashing_context(move |ref hcx| { - for (&expr_def_id, predicates) in - fcx_typeck_results.coroutine_stalled_predicates.to_sorted(hcx, false).into_iter() - { - let predicates = - self.resolve(predicates.clone(), &self.fcx.tcx.def_span(expr_def_id)); - self.typeck_results.coroutine_stalled_predicates.insert(expr_def_id, predicates); - } - }) + for (predicate, cause) in &fcx_typeck_results.coroutine_stalled_predicates { + let (predicate, cause) = self.resolve((*predicate, cause.clone()), &cause.span); + self.typeck_results.coroutine_stalled_predicates.insert((predicate, cause)); + } } #[instrument(skip(self), level = "debug")] diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 91cef02c7d17d..b593c41a8eafa 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -759,7 +759,9 @@ fn run_required_analyses(tcx: TyCtxt<'_>) { tcx.hir().par_body_owners(|def_id| { if tcx.is_coroutine(def_id.to_def_id()) { tcx.ensure().mir_coroutine_witnesses(def_id); - tcx.ensure().check_coroutine_obligations(def_id); + tcx.ensure().check_coroutine_obligations( + tcx.typeck_root_def_id(def_id.to_def_id()).expect_local(), + ); } }); sess.time("layout_testing", || layout_test::test_layout(tcx)); diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs index aa905466fbc87..904d56c516d8e 100644 --- a/compiler/rustc_middle/src/ty/typeck_results.rs +++ b/compiler/rustc_middle/src/ty/typeck_results.rs @@ -7,10 +7,8 @@ use crate::{ GenericArgs, GenericArgsRef, Ty, UserArgs, }, }; -use rustc_data_structures::{ - fx::FxIndexMap, - unord::{ExtendUnord, UnordItems, UnordSet}, -}; +use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; +use rustc_data_structures::unord::{ExtendUnord, UnordItems, UnordSet}; use rustc_errors::ErrorGuaranteed; use rustc_hir::{ self as hir, @@ -201,8 +199,7 @@ pub struct TypeckResults<'tcx> { /// Stores the predicates that apply on coroutine witness types. /// formatting modified file tests/ui/coroutine/retain-resume-ref.rs - pub coroutine_stalled_predicates: - LocalDefIdMap, ObligationCause<'tcx>)>>, + pub coroutine_stalled_predicates: FxIndexSet<(ty::Predicate<'tcx>, ObligationCause<'tcx>)>, /// We sometimes treat byte string literals (which are of type `&[u8; N]`) /// as `&[u8]`, depending on the pattern in which they are used. diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index e2a911f0dc7d9..b745d97567d6f 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -80,6 +80,10 @@ use rustc_span::symbol::sym; use rustc_span::Span; use rustc_target::abi::{FieldIdx, VariantIdx}; use rustc_target::spec::PanicStrategy; +use rustc_trait_selection::infer::TyCtxtInferExt as _; +use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; +use rustc_trait_selection::traits::ObligationCtxt; +use rustc_trait_selection::traits::{ObligationCause, ObligationCauseCode}; use std::{iter, ops}; pub struct StateTransform; @@ -1584,10 +1588,46 @@ pub(crate) fn mir_coroutine_witnesses<'tcx>( let (_, coroutine_layout, _) = compute_layout(liveness_info, body); check_suspend_tys(tcx, &coroutine_layout, body); + check_field_tys_sized(tcx, &coroutine_layout, def_id); Some(coroutine_layout) } +fn check_field_tys_sized<'tcx>( + tcx: TyCtxt<'tcx>, + coroutine_layout: &CoroutineLayout<'tcx>, + def_id: LocalDefId, +) { + // No need to check if unsized_locals/unsized_fn_params is disabled, + // since we will error during typeck. + if !tcx.features().unsized_locals && !tcx.features().unsized_fn_params { + return; + } + + let infcx = tcx.infer_ctxt().ignoring_regions().build(); + let param_env = tcx.param_env(def_id); + + let ocx = ObligationCtxt::new(&infcx); + for field_ty in &coroutine_layout.field_tys { + ocx.register_bound( + ObligationCause::new( + field_ty.source_info.span, + def_id, + ObligationCauseCode::SizedCoroutineInterior(def_id), + ), + param_env, + field_ty.ty, + tcx.require_lang_item(hir::LangItem::Sized, Some(field_ty.source_info.span)), + ); + } + + let errors = ocx.select_all_or_error(); + debug!(?errors); + if !errors.is_empty() { + infcx.err_ctxt().report_fulfillment_errors(errors); + } +} + impl<'tcx> MirPass<'tcx> for StateTransform { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let Some(old_yield_ty) = body.yield_ty() else { diff --git a/tests/ui/coroutine/clone-impl.rs b/tests/ui/coroutine/clone-impl.rs index eed6f851bd028..fffdae632ef7d 100644 --- a/tests/ui/coroutine/clone-impl.rs +++ b/tests/ui/coroutine/clone-impl.rs @@ -6,18 +6,18 @@ struct NonClone; -fn main() { +fn test1() { let copyable: u32 = 123; - let clonable_0: Vec = Vec::new(); - let clonable_1: Vec = Vec::new(); - let non_clonable: NonClone = NonClone; - let gen_copy_0 = move || { yield; drop(copyable); }; check_copy(&gen_copy_0); check_clone(&gen_copy_0); +} + +fn test2() { + let copyable: u32 = 123; let gen_copy_1 = move || { /* let v = vec!['a']; @@ -33,6 +33,10 @@ fn main() { }; check_copy(&gen_copy_1); check_clone(&gen_copy_1); +} + +fn test3() { + let clonable_0: Vec = Vec::new(); let gen_clone_0 = move || { let v = vec!['a']; yield; @@ -43,6 +47,10 @@ fn main() { //~^ ERROR the trait bound `Vec: Copy` is not satisfied //~| ERROR the trait bound `Vec: Copy` is not satisfied check_clone(&gen_clone_0); +} + +fn test4() { + let clonable_1: Vec = Vec::new(); let gen_clone_1 = move || { let v = vec!['a']; /* @@ -59,6 +67,10 @@ fn main() { //~^ ERROR the trait bound `Vec: Copy` is not satisfied //~| ERROR the trait bound `Vec: Copy` is not satisfied check_clone(&gen_clone_1); +} + +fn test5() { + let non_clonable: NonClone = NonClone; let gen_non_clone = move || { yield; drop(non_clonable); @@ -71,3 +83,5 @@ fn main() { fn check_copy(_x: &T) {} fn check_clone(_x: &T) {} + +fn main() {} diff --git a/tests/ui/coroutine/clone-impl.stderr b/tests/ui/coroutine/clone-impl.stderr index 1d4804501d8bc..b454846faac9b 100644 --- a/tests/ui/coroutine/clone-impl.stderr +++ b/tests/ui/coroutine/clone-impl.stderr @@ -1,76 +1,76 @@ -error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}` - --> $DIR/clone-impl.rs:42:5 +error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}` + --> $DIR/clone-impl.rs:46:5 | LL | let gen_clone_0 = move || { - | ------- within this `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}` + | ------- within this `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}` ... LL | check_copy(&gen_clone_0); - | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}: Copy` + | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}: Copy` | note: captured value does not implement `Copy` - --> $DIR/clone-impl.rs:40:14 + --> $DIR/clone-impl.rs:44:14 | LL | drop(clonable_0); | ^^^^^^^^^^ has type `Vec` which does not implement `Copy` note: required by a bound in `check_copy` - --> $DIR/clone-impl.rs:72:18 + --> $DIR/clone-impl.rs:84:18 | LL | fn check_copy(_x: &T) {} | ^^^^ required by this bound in `check_copy` -error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}` - --> $DIR/clone-impl.rs:42:5 +error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}` + --> $DIR/clone-impl.rs:46:5 | LL | let gen_clone_0 = move || { - | ------- within this `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}` + | ------- within this `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}` ... LL | check_copy(&gen_clone_0); - | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:36:23: 36:30}: Copy` + | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:40:23: 40:30}: Copy` | note: coroutine does not implement `Copy` as this value is used across a yield - --> $DIR/clone-impl.rs:38:9 + --> $DIR/clone-impl.rs:42:9 | LL | let v = vec!['a']; | - has type `Vec` which does not implement `Copy` LL | yield; | ^^^^^ yield occurs here, with `v` maybe used later note: required by a bound in `check_copy` - --> $DIR/clone-impl.rs:72:18 + --> $DIR/clone-impl.rs:84:18 | LL | fn check_copy(_x: &T) {} | ^^^^ required by this bound in `check_copy` -error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}` - --> $DIR/clone-impl.rs:58:5 +error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}` + --> $DIR/clone-impl.rs:66:5 | LL | let gen_clone_1 = move || { - | ------- within this `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}` + | ------- within this `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}` ... LL | check_copy(&gen_clone_1); - | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}: Copy` + | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}: Copy` | note: captured value does not implement `Copy` - --> $DIR/clone-impl.rs:56:14 + --> $DIR/clone-impl.rs:64:14 | LL | drop(clonable_1); | ^^^^^^^^^^ has type `Vec` which does not implement `Copy` note: required by a bound in `check_copy` - --> $DIR/clone-impl.rs:72:18 + --> $DIR/clone-impl.rs:84:18 | LL | fn check_copy(_x: &T) {} | ^^^^ required by this bound in `check_copy` -error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}` - --> $DIR/clone-impl.rs:58:5 +error[E0277]: the trait bound `Vec: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}` + --> $DIR/clone-impl.rs:66:5 | LL | let gen_clone_1 = move || { - | ------- within this `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}` + | ------- within this `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}` ... LL | check_copy(&gen_clone_1); - | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:46:23: 46:30}: Copy` + | ^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}`, the trait `Copy` is not implemented for `Vec`, which is required by `{coroutine@$DIR/clone-impl.rs:54:23: 54:30}: Copy` | note: coroutine does not implement `Copy` as this value is used across a yield - --> $DIR/clone-impl.rs:52:9 + --> $DIR/clone-impl.rs:60:9 | LL | let v = vec!['a']; | - has type `Vec` which does not implement `Copy` @@ -78,27 +78,27 @@ LL | let v = vec!['a']; LL | yield; | ^^^^^ yield occurs here, with `v` maybe used later note: required by a bound in `check_copy` - --> $DIR/clone-impl.rs:72:18 + --> $DIR/clone-impl.rs:84:18 | LL | fn check_copy(_x: &T) {} | ^^^^ required by this bound in `check_copy` -error[E0277]: the trait bound `NonClone: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}` - --> $DIR/clone-impl.rs:66:5 +error[E0277]: the trait bound `NonClone: Copy` is not satisfied in `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}` + --> $DIR/clone-impl.rs:78:5 | LL | let gen_non_clone = move || { - | ------- within this `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}` + | ------- within this `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}` ... LL | check_copy(&gen_non_clone); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}`, the trait `Copy` is not implemented for `NonClone`, which is required by `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}: Copy` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}`, the trait `Copy` is not implemented for `NonClone`, which is required by `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}: Copy` | note: captured value does not implement `Copy` - --> $DIR/clone-impl.rs:64:14 + --> $DIR/clone-impl.rs:76:14 | LL | drop(non_clonable); | ^^^^^^^^^^^^ has type `NonClone` which does not implement `Copy` note: required by a bound in `check_copy` - --> $DIR/clone-impl.rs:72:18 + --> $DIR/clone-impl.rs:84:18 | LL | fn check_copy(_x: &T) {} | ^^^^ required by this bound in `check_copy` @@ -108,22 +108,22 @@ LL + #[derive(Copy)] LL | struct NonClone; | -error[E0277]: the trait bound `NonClone: Clone` is not satisfied in `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}` - --> $DIR/clone-impl.rs:68:5 +error[E0277]: the trait bound `NonClone: Clone` is not satisfied in `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}` + --> $DIR/clone-impl.rs:80:5 | LL | let gen_non_clone = move || { - | ------- within this `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}` + | ------- within this `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}` ... LL | check_clone(&gen_non_clone); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}`, the trait `Clone` is not implemented for `NonClone`, which is required by `{coroutine@$DIR/clone-impl.rs:62:25: 62:32}: Clone` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ within `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}`, the trait `Clone` is not implemented for `NonClone`, which is required by `{coroutine@$DIR/clone-impl.rs:74:25: 74:32}: Clone` | note: captured value does not implement `Clone` - --> $DIR/clone-impl.rs:64:14 + --> $DIR/clone-impl.rs:76:14 | LL | drop(non_clonable); | ^^^^^^^^^^^^ has type `NonClone` which does not implement `Clone` note: required by a bound in `check_clone` - --> $DIR/clone-impl.rs:73:19 + --> $DIR/clone-impl.rs:85:19 | LL | fn check_clone(_x: &T) {} | ^^^^^ required by this bound in `check_clone` From 06501156d15120ff4a14c8f5b714c8c46563b58d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 15 Apr 2024 19:54:51 -0400 Subject: [PATCH 04/22] redundant ::{self} --- compiler/rustc_hir_analysis/src/check/check.rs | 2 +- compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs | 2 +- compiler/rustc_span/src/source_map.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index cb0ed68edff3e..3fd1cd3910c3f 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -22,10 +22,10 @@ use rustc_middle::ty::{ }; use rustc_session::lint::builtin::{UNINHABITED_STATIC, UNSUPPORTED_CALLING_CONVENTIONS}; use rustc_target::abi::FieldIdx; +use rustc_trait_selection::traits; use rustc_trait_selection::traits::error_reporting::on_unimplemented::OnUnimplementedDirective; use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _; -use rustc_trait_selection::traits::{self}; use rustc_type_ir::fold::TypeFoldable; use std::cell::LazyCell; diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index ce203eae95fd0..b911c76da5e88 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -23,7 +23,7 @@ use rustc_hir::{ }; use rustc_hir_analysis::collect::suggest_impl_trait; use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer; -use rustc_infer::traits::{self}; +use rustc_infer::traits; use rustc_middle::lint::in_external_macro; use rustc_middle::middle::stability::EvalResult; use rustc_middle::ty::print::with_no_trimmed_paths; diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 93d5f06a16736..d9aa5300ed8cb 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -14,7 +14,7 @@ use rustc_data_structures::sync::{IntoDynSyncSend, MappedReadGuard, ReadGuard, R use rustc_data_structures::unhash::UnhashMap; use std::fs; use std::io::{self, BorrowedBuf, Read}; -use std::path::{self}; +use std::path; #[cfg(test)] mod tests; From a8c9a0bd8167c99557c9d475631c844775883336 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 15 Apr 2024 22:21:42 -0400 Subject: [PATCH 05/22] crash -> test --- tests/crashes/122552.rs | 10 ---------- .../coroutine-in-orphaned-anon-const.rs | 10 ++++++++++ .../coroutine-in-orphaned-anon-const.stderr | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 10 deletions(-) delete mode 100644 tests/crashes/122552.rs create mode 100644 tests/ui/coroutine/coroutine-in-orphaned-anon-const.rs create mode 100644 tests/ui/coroutine/coroutine-in-orphaned-anon-const.stderr diff --git a/tests/crashes/122552.rs b/tests/crashes/122552.rs deleted file mode 100644 index 29566941e22b2..0000000000000 --- a/tests/crashes/122552.rs +++ /dev/null @@ -1,10 +0,0 @@ -//@ known-bug: #122552 -//@ edition:2021 - -trait X { - fn line_stream<'a, Repr>() -> Self::LineStreamFut<{ async {} }, Repr>; -} - -struct Y; - -pub fn main() {} diff --git a/tests/ui/coroutine/coroutine-in-orphaned-anon-const.rs b/tests/ui/coroutine/coroutine-in-orphaned-anon-const.rs new file mode 100644 index 0000000000000..07c13239a2ce2 --- /dev/null +++ b/tests/ui/coroutine/coroutine-in-orphaned-anon-const.rs @@ -0,0 +1,10 @@ +//@ edition:2021 + +trait X { + fn test() -> Self::Assoc<{ async {} }>; + //~^ ERROR associated type `Assoc` not found for `Self` + //~| ERROR associated type `Assoc` not found for `Self` + +} + +pub fn main() {} diff --git a/tests/ui/coroutine/coroutine-in-orphaned-anon-const.stderr b/tests/ui/coroutine/coroutine-in-orphaned-anon-const.stderr new file mode 100644 index 0000000000000..864c6556d7990 --- /dev/null +++ b/tests/ui/coroutine/coroutine-in-orphaned-anon-const.stderr @@ -0,0 +1,17 @@ +error[E0220]: associated type `Assoc` not found for `Self` + --> $DIR/coroutine-in-orphaned-anon-const.rs:4:24 + | +LL | fn test() -> Self::Assoc<{ async {} }>; + | ^^^^^ associated type `Assoc` not found + +error[E0220]: associated type `Assoc` not found for `Self` + --> $DIR/coroutine-in-orphaned-anon-const.rs:4:24 + | +LL | fn test() -> Self::Assoc<{ async {} }>; + | ^^^^^ associated type `Assoc` not found + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0220`. From f7ebad494ca7238047d3e64b59da356c5b268985 Mon Sep 17 00:00:00 2001 From: Gurinder Singh Date: Tue, 16 Apr 2024 11:11:50 +0530 Subject: [PATCH 06/22] Emit suggestions when equality constraints are wrongly used --- .../src/hir_ty_lowering/errors.rs | 115 +++++- .../src/hir_ty_lowering/generics.rs | 2 +- .../src/hir_ty_lowering/mod.rs | 8 +- .../invalid_associated_const.stderr | 9 + tests/rustdoc-ui/issue-102467.stderr | 9 + .../issue-102335-const.stderr | 9 + .../associated-type-bounds/issue-102335-ty.rs | 8 +- .../issue-102335-ty.stderr | 38 +- .../associated-types/associated-types-eq-2.rs | 120 +++++- .../associated-types-eq-2.stderr | 366 +++++++++++++++++- .../issue-89013-no-kw.stderr | 5 + .../parser-error-recovery/issue-89013.stderr | 5 + tests/ui/error-codes/E0229.stderr | 13 + .../issue-102335-gat.stderr | 9 + ...lid-associated-type-supertrait-hrtb.stderr | 5 + tests/ui/suggestions/issue-85347.rs | 6 +- tests/ui/suggestions/issue-85347.stderr | 9 + 17 files changed, 707 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs index 7a0890e50dac8..3d5be4f7e5507 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs @@ -17,6 +17,7 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_infer::traits::FulfillmentError; use rustc_middle::query::Key; +use rustc_middle::ty::GenericParamDefKind; use rustc_middle::ty::{self, suggest_constraining_type_param}; use rustc_middle::ty::{AdtDef, Ty, TyCtxt, TypeVisitableExt}; use rustc_middle::ty::{Binder, TraitRef}; @@ -1200,12 +1201,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { /// Emits an error regarding forbidden type binding associations pub fn prohibit_assoc_item_binding( tcx: TyCtxt<'_>, - span: Span, - segment: Option<(&hir::PathSegment<'_>, Span)>, + binding: &hir::TypeBinding<'_>, + segment: Option<(DefId, &hir::PathSegment<'_>, Span)>, ) -> ErrorGuaranteed { - tcx.dcx().emit_err(AssocTypeBindingNotAllowed { - span, - fn_trait_expansion: if let Some((segment, span)) = segment + let mut err = tcx.dcx().create_err(AssocTypeBindingNotAllowed { + span: binding.span, + fn_trait_expansion: if let Some((_, segment, span)) = segment && segment.args().parenthesized == hir::GenericArgsParentheses::ParenSugar { Some(ParenthesizedFnTraitExpansion { @@ -1215,7 +1216,109 @@ pub fn prohibit_assoc_item_binding( } else { None }, - }) + }); + + // Emit a suggestion to turn the assoc item binding into a generic arg + // if the relevant item has a generic param whose name matches the binding name; + // otherwise suggest the removal of the binding. + if let Some((def_id, segment, _)) = segment + && segment.args().parenthesized == hir::GenericArgsParentheses::No + && let hir::TypeBindingKind::Equality { term } = binding.kind + { + // Suggests removal of the offending binding + let suggest_removal = |e: &mut Diag<'_>| { + let bindings = segment.args().bindings; + let args = segment.args().args; + let binding_span = binding.span; + + // Compute the span to remove based on the position + // of the binding. We do that as follows: + // 1. Find the index of the binding in the list of bindings + // 2. Locate the spans preceding and following the binding. + // If it's the first binding the preceding span would be + // that of the last arg + // 3. Using this information work out whether the span + // to remove will start from the end of the preceding span, + // the start of the next span or will simply be the + // span encomassing everything within the generics brackets + + let Some(binding_index) = bindings.iter().position(|b| b.hir_id == binding.hir_id) + else { + bug!("a type binding exists but its HIR ID not found in generics"); + }; + + let preceding_span = if binding_index > 0 { + Some(bindings[binding_index - 1].span) + } else { + args.last().map(|a| a.span()) + }; + + let next_span = if binding_index < bindings.len() - 1 { + Some(bindings[binding_index + 1].span) + } else { + None + }; + + let removal_span = match (preceding_span, next_span) { + (Some(prec), _) => binding_span.with_lo(prec.hi()), + (None, Some(next)) => binding_span.with_hi(next.lo()), + (None, None) => { + let Some(generics_span) = segment.args().span_ext() else { + bug!("a type binding exists but generic span is empty"); + }; + + generics_span + } + }; + + // Now emit the suggestion + if let Ok(suggestion) = tcx.sess.source_map().span_to_snippet(removal_span) { + e.span_suggestion_verbose( + removal_span, + "consider removing this type binding", + suggestion, + Applicability::MaybeIncorrect, + ); + } + }; + + // Suggest replacing the associated item binding with a generic argument. + // i.e., replacing `<..., T = A, ...>` with `<..., A, ...>`. + let suggest_direct_use = |e: &mut Diag<'_>, sp: Span| { + if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(sp) { + e.span_suggestion_verbose( + binding.span, + format!("to use `{snippet}` as a generic argument specify it directly"), + snippet, + Applicability::MaybeIncorrect, + ); + } + }; + + // Check if the type has a generic param with the + // same name as the assoc type name in type binding + let generics = tcx.generics_of(def_id); + let matching_param = + generics.params.iter().find(|p| p.name.as_str() == binding.ident.as_str()); + + // Now emit the appropriate suggestion + if let Some(matching_param) = matching_param { + match (&matching_param.kind, term) { + (GenericParamDefKind::Type { .. }, hir::Term::Ty(ty)) => { + suggest_direct_use(&mut err, ty.span); + } + (GenericParamDefKind::Const { .. }, hir::Term::Const(c)) => { + let span = tcx.hir().span(c.hir_id); + suggest_direct_use(&mut err, span); + } + _ => suggest_removal(&mut err), + } + } else { + suggest_removal(&mut err); + } + } + + err.emit() } pub(crate) fn fn_trait_to_string( diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs index d340a08ee79b1..1957849c5aaad 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs @@ -454,7 +454,7 @@ pub(crate) fn check_generic_arg_count( if gen_pos != GenericArgPosition::Type && let Some(b) = gen_args.bindings.first() { - prohibit_assoc_item_binding(tcx, b.span, None); + prohibit_assoc_item_binding(tcx, b, None); } let explicit_late_bound = diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index f726f2a7b8908..f8596c67d422f 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -322,7 +322,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { ty::BoundConstness::NotConst, ); if let Some(b) = item_segment.args().bindings.first() { - prohibit_assoc_item_binding(self.tcx(), b.span, Some((item_segment, span))); + prohibit_assoc_item_binding(self.tcx(), b, Some((def_id, item_segment, span))); } args } @@ -619,7 +619,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { ty::BoundConstness::NotConst, ); if let Some(b) = item_segment.args().bindings.first() { - prohibit_assoc_item_binding(self.tcx(), b.span, Some((item_segment, span))); + prohibit_assoc_item_binding(self.tcx(), b, Some((item_def_id, item_segment, span))); } args } @@ -764,7 +764,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { constness, ); if let Some(b) = trait_segment.args().bindings.first() { - prohibit_assoc_item_binding(self.tcx(), b.span, Some((trait_segment, span))); + prohibit_assoc_item_binding(self.tcx(), b, Some((trait_def_id, trait_segment, span))); } ty::TraitRef::new(self.tcx(), trait_def_id, generic_args) } @@ -1556,7 +1556,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { for segment in segments { // Only emit the first error to avoid overloading the user with error messages. if let Some(b) = segment.args().bindings.first() { - return Err(prohibit_assoc_item_binding(self.tcx(), b.span, None)); + return Err(prohibit_assoc_item_binding(self.tcx(), b, None)); } } diff --git a/tests/rustdoc-ui/invalid_associated_const.stderr b/tests/rustdoc-ui/invalid_associated_const.stderr index 1eb6d2714e31b..5eaddc2b8c984 100644 --- a/tests/rustdoc-ui/invalid_associated_const.stderr +++ b/tests/rustdoc-ui/invalid_associated_const.stderr @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here | LL | type A: S = 34>; | ^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | type A: S = 34>; + | ~~~~~~~~~~ error[E0229]: associated type bindings are not allowed here --> $DIR/invalid_associated_const.rs:4:17 @@ -11,6 +16,10 @@ LL | type A: S = 34>; | ^^^^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | type A: S = 34>; + | ~~~~~~~~~~ error: aborting due to 2 previous errors diff --git a/tests/rustdoc-ui/issue-102467.stderr b/tests/rustdoc-ui/issue-102467.stderr index f54a50a4e1952..119ca949e999a 100644 --- a/tests/rustdoc-ui/issue-102467.stderr +++ b/tests/rustdoc-ui/issue-102467.stderr @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here | LL | type A: S = 34>; | ^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | type A: S = 34>; + | ~~~~~~~~~~ error[E0229]: associated type bindings are not allowed here --> $DIR/issue-102467.rs:7:17 @@ -11,6 +16,10 @@ LL | type A: S = 34>; | ^^^^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | type A: S = 34>; + | ~~~~~~~~~~ error: aborting due to 2 previous errors diff --git a/tests/ui/associated-consts/issue-102335-const.stderr b/tests/ui/associated-consts/issue-102335-const.stderr index 2a70425a3cc87..905d7c75c2000 100644 --- a/tests/ui/associated-consts/issue-102335-const.stderr +++ b/tests/ui/associated-consts/issue-102335-const.stderr @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here | LL | type A: S = 34>; | ^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | type A: S = 34>; + | ~~~~~~~~~~ error[E0229]: associated type bindings are not allowed here --> $DIR/issue-102335-const.rs:4:17 @@ -11,6 +16,10 @@ LL | type A: S = 34>; | ^^^^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | type A: S = 34>; + | ~~~~~~~~~~ error: aborting due to 2 previous errors diff --git a/tests/ui/associated-type-bounds/issue-102335-ty.rs b/tests/ui/associated-type-bounds/issue-102335-ty.rs index 5fd8b71e679b2..b2df68b18ae4d 100644 --- a/tests/ui/associated-type-bounds/issue-102335-ty.rs +++ b/tests/ui/associated-type-bounds/issue-102335-ty.rs @@ -1,5 +1,11 @@ trait T { - type A: S = ()>; + type A: S = ()>; // Just one erroneous equality constraint + //~^ ERROR associated type bindings are not allowed here + //~| ERROR associated type bindings are not allowed here +} + +trait T2 { + type A: S = ()>; // More than one erroneous equality constraints //~^ ERROR associated type bindings are not allowed here //~| ERROR associated type bindings are not allowed here } diff --git a/tests/ui/associated-type-bounds/issue-102335-ty.stderr b/tests/ui/associated-type-bounds/issue-102335-ty.stderr index 3bd7566ad1e0a..cf30b0a4f6cad 100644 --- a/tests/ui/associated-type-bounds/issue-102335-ty.stderr +++ b/tests/ui/associated-type-bounds/issue-102335-ty.stderr @@ -1,17 +1,49 @@ error[E0229]: associated type bindings are not allowed here --> $DIR/issue-102335-ty.rs:2:17 | -LL | type A: S = ()>; +LL | type A: S = ()>; // Just one erroneous equality constraint | ^^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | type A: S = ()>; // Just one erroneous equality constraint + | ~~~~~~~~~~~ error[E0229]: associated type bindings are not allowed here --> $DIR/issue-102335-ty.rs:2:17 | -LL | type A: S = ()>; +LL | type A: S = ()>; // Just one erroneous equality constraint + | ^^^^^^^^^ associated type not allowed here + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | type A: S = ()>; // Just one erroneous equality constraint + | ~~~~~~~~~~~ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/issue-102335-ty.rs:8:17 + | +LL | type A: S = ()>; // More than one erroneous equality constraints + | ^^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | type A: S = ()>; // More than one erroneous equality constraints + | ~~~~~~~~~~ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/issue-102335-ty.rs:8:17 + | +LL | type A: S = ()>; // More than one erroneous equality constraints | ^^^^^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | type A: S = ()>; // More than one erroneous equality constraints + | ~~~~~~~~~~ -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0229`. diff --git a/tests/ui/associated-types/associated-types-eq-2.rs b/tests/ui/associated-types/associated-types-eq-2.rs index 18e38d4466702..d71697e9a8325 100644 --- a/tests/ui/associated-types/associated-types-eq-2.rs +++ b/tests/ui/associated-types/associated-types-eq-2.rs @@ -1,19 +1,125 @@ // Test equality constraints on associated types. Check we get an error when an -// equality constraint is used in a qualified path. +// equality constraint is used in an invalid context -pub trait Foo { +struct Bar; +struct Qux; + +// Tests for a a non generic trait +pub trait Tr1 { type A; - fn boo(&self) -> ::A; + fn boo(&self) -> ::A; } -struct Bar; - -impl Foo for isize { +impl Tr1 for isize { type A = usize; fn boo(&self) -> usize { 42 } } -fn baz(x: &>::A) {} +// Test for when the assoc type is +// specified as an equality constraint +impl Tr1 for usize { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR not all trait items implemented, missing: `A` + fn boo(&self) -> usize { 42 } +} + +// Test for a wronngly used equality constraint in a func arg +fn baz(_x: &>::A) {} +//~^ ERROR associated type bindings are not allowed here + + + +// Tests for a generic trait +trait Tr2 { +} + +// Test for when wrongly specifed equality constraint's ident +// matches some generic param's ident +// (Note: E0229 is emitted only for the first erroneous equality +// constraint (T2) not for any subequent ones (e.g. T3)) +impl Tr2 for Bar { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR trait takes 3 generic arguments but 1 generic argument was supplied +} + +// Test for when equality constraint's ident matches a +// generic param's ident but has different case +impl Tr2 for Qux { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR trait takes 3 generic arguments but 1 generic argument was supplied +} + +// Test for when equality constraint's ident +// matches none of the generic param idents +impl Tr2 for Bar { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR trait takes 3 generic arguments but 1 generic argument was supplied +} + +// Test for when the term in equality constraint is itself generic +struct GenericTerm { _t: T } +impl Tr2> for Bar { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR trait takes 3 generic arguments but 2 generic arguments were supplied +} + + + +// Tests for a trait with a const param +trait Tr3 { +} + +// Test for when equality constraint's ident +// matches the const param's ident +// (Deliberately spread over multiple lines to test that +// our suggestion spans are kosher in the face of such formatting) +impl Tr3 for Bar { +} + +// Test for when equality constraint's ident +// matches the const param's ident but has a different case +impl Tr3 for Qux { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR associated const equality is incomplete +//~| ERROR trait takes 3 generic arguments but 0 generic arguments were supplied +} + +// Test for when equality constraint's ident +// matches the const param ident but the constraint is a type arg +impl Tr3 for Bar { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR trait takes 3 generic arguments but 0 generic arguments were supplied +} + +// Test for when equality constraint's ident +// matches a type param ident but the constraint is a const arg +impl Tr3<42, T2 = 42, T3 = usize> for Bar { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR associated const equality is incomplete +//~| ERROR trait takes 3 generic arguments but 1 generic argument was supplied +} + +// Test for when equality constraint's ident +// matches none of the param idents +impl Tr3 for Bar { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR associated const equality is incomplete +//~| ERROR trait takes 3 generic arguments but 0 generic arguments were supplied +} + + + +// Test for the case when lifetimes are present +struct St<'a, T> { v: &'a T } + +impl<'a, T> St<'a , T = Qux> { +//~^ ERROR associated type bindings are not allowed here +//~| ERROR struct takes 1 generic argument but 0 generic arguments were supplied +} + pub fn main() {} diff --git a/tests/ui/associated-types/associated-types-eq-2.stderr b/tests/ui/associated-types/associated-types-eq-2.stderr index 447b8413ee294..b68c82f590c14 100644 --- a/tests/ui/associated-types/associated-types-eq-2.stderr +++ b/tests/ui/associated-types/associated-types-eq-2.stderr @@ -1,9 +1,365 @@ +error[E0658]: associated const equality is incomplete + --> $DIR/associated-types-eq-2.rs:76:10 + | +LL | impl Tr3 for Bar { + | |____^ + | + = note: see issue #92827 for more information + = help: add `#![feature(associated_const_equality)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0658]: associated const equality is incomplete + --> $DIR/associated-types-eq-2.rs:85:10 + | +LL | impl Tr3 for Qux { + | ^^^^^^ + | + = note: see issue #92827 for more information + = help: add `#![feature(associated_const_equality)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0658]: associated const equality is incomplete + --> $DIR/associated-types-eq-2.rs:100:14 + | +LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar { + | ^^^^^^^ + | + = note: see issue #92827 for more information + = help: add `#![feature(associated_const_equality)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0658]: associated const equality is incomplete + --> $DIR/associated-types-eq-2.rs:108:10 + | +LL | impl Tr3 for Bar { + | ^^^^^^ + | + = note: see issue #92827 for more information + = help: add `#![feature(associated_const_equality)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + error[E0229]: associated type bindings are not allowed here - --> $DIR/associated-types-eq-2.rs:16:30 + --> $DIR/associated-types-eq-2.rs:20:10 + | +LL | impl Tr1 for usize { + | ^^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | impl Tr1 for usize { + | ~~~~~~~~~~~ + +error[E0046]: not all trait items implemented, missing: `A` + --> $DIR/associated-types-eq-2.rs:20:1 + | +LL | type A; + | ------ `A` from trait +... +LL | impl Tr1 for usize { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `A` in implementation + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:27:31 + | +LL | fn baz(_x: &>::A) {} + | ^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | fn baz(_x: &>::A) {} + | ~~~~~~~ + +error[E0107]: trait takes 3 generic arguments but 1 generic argument was supplied + --> $DIR/associated-types-eq-2.rs:40:6 + | +LL | impl Tr2 for Bar { + | ^^^ --- supplied 1 generic argument + | | + | expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `T1`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:33:7 + | +LL | trait Tr2 { + | ^^^ -- -- -- +help: add missing generic arguments + | +LL | impl Tr2 for Bar { + | ++++++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:40:15 + | +LL | impl Tr2 for Bar { + | ^^^^^^^^ associated type not allowed here + | +help: to use `Qux` as a generic argument specify it directly + | +LL | impl Tr2 for Bar { + | ~~~ + +error[E0107]: trait takes 3 generic arguments but 1 generic argument was supplied + --> $DIR/associated-types-eq-2.rs:47:6 + | +LL | impl Tr2 for Qux { + | ^^^ --- supplied 1 generic argument + | | + | expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `T1`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:33:7 + | +LL | trait Tr2 { + | ^^^ -- -- -- +help: add missing generic arguments + | +LL | impl Tr2 for Qux { + | ++++++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:47:15 + | +LL | impl Tr2 for Qux { + | ^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | impl Tr2 for Qux { + | ~~~~~~~~~~ + +error[E0107]: trait takes 3 generic arguments but 1 generic argument was supplied + --> $DIR/associated-types-eq-2.rs:54:6 + | +LL | impl Tr2 for Bar { + | ^^^ --- supplied 1 generic argument + | | + | expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `T1`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:33:7 + | +LL | trait Tr2 { + | ^^^ -- -- -- +help: add missing generic arguments + | +LL | impl Tr2 for Bar { + | ++++++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:54:15 + | +LL | impl Tr2 for Bar { + | ^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | impl Tr2 for Bar { + | ~~~~~~~~~ + +error[E0107]: trait takes 3 generic arguments but 2 generic arguments were supplied + --> $DIR/associated-types-eq-2.rs:61:6 + | +LL | impl Tr2> for Bar { + | ^^^ --- --- supplied 2 generic arguments + | | + | expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `T1`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:33:7 + | +LL | trait Tr2 { + | ^^^ -- -- -- +help: add missing generic argument + | +LL | impl Tr2> for Bar { + | ++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:61:20 + | +LL | impl Tr2> for Bar { + | ^^^^^^^^^^^^^^^^^^^^^ associated type not allowed here + | +help: to use `GenericTerm` as a generic argument specify it directly + | +LL | impl Tr2> for Bar { + | ~~~~~~~~~~~~~~~~ + +error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied + --> $DIR/associated-types-eq-2.rs:76:6 + | +LL | impl Tr3 $DIR/associated-types-eq-2.rs:69:7 + | +LL | trait Tr3 { + | ^^^ ------------ -- -- +help: add missing generic arguments + | +LL | impl Tr3 $DIR/associated-types-eq-2.rs:76:10 + | +LL | impl Tr3 for Bar { + | |____^ associated type not allowed here + | +help: to use `42` as a generic argument specify it directly + | +LL | impl Tr3<42, T2 = Qux, T3 = usize> for Bar { + | ~~ + +error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied + --> $DIR/associated-types-eq-2.rs:85:6 + | +LL | impl Tr3 for Qux { + | ^^^ expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `N`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:69:7 + | +LL | trait Tr3 { + | ^^^ ------------ -- -- +help: add missing generic arguments + | +LL | impl Tr3 for Qux { + | ++++++++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:85:10 + | +LL | impl Tr3 for Qux { + | ^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | impl Tr3 for Qux { + | ~~~~~~~ + +error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied + --> $DIR/associated-types-eq-2.rs:93:6 + | +LL | impl Tr3 for Bar { + | ^^^ expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `N`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:69:7 + | +LL | trait Tr3 { + | ^^^ ------------ -- -- +help: add missing generic arguments + | +LL | impl Tr3 for Bar { + | ++++++++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:93:10 + | +LL | impl Tr3 for Bar { + | ^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | impl Tr3 for Bar { + | ~~~~~~~~ + +error[E0107]: trait takes 3 generic arguments but 1 generic argument was supplied + --> $DIR/associated-types-eq-2.rs:100:6 + | +LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar { + | ^^^ -- supplied 1 generic argument + | | + | expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `N`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:69:7 + | +LL | trait Tr3 { + | ^^^ ------------ -- -- +help: add missing generic arguments + | +LL | impl Tr3<42, T2, T3, T2 = 42, T3 = usize> for Bar { + | ++++++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:100:14 + | +LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar { + | ^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar { + | ~~~~~~~~~ + +error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied + --> $DIR/associated-types-eq-2.rs:108:6 + | +LL | impl Tr3 for Bar { + | ^^^ expected 3 generic arguments + | +note: trait defined here, with 3 generic parameters: `N`, `T2`, `T3` + --> $DIR/associated-types-eq-2.rs:69:7 + | +LL | trait Tr3 { + | ^^^ ------------ -- -- +help: add missing generic arguments + | +LL | impl Tr3 for Bar { + | ++++++++++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:108:10 + | +LL | impl Tr3 for Bar { + | ^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | impl Tr3 for Bar { + | ~~~~~~~ + +error[E0107]: struct takes 1 generic argument but 0 generic arguments were supplied + --> $DIR/associated-types-eq-2.rs:119:13 + | +LL | impl<'a, T> St<'a , T = Qux> { + | ^^ expected 1 generic argument + | +note: struct defined here, with 1 generic parameter: `T` + --> $DIR/associated-types-eq-2.rs:117:8 + | +LL | struct St<'a, T> { v: &'a T } + | ^^ - +help: add missing generic argument + | +LL | impl<'a, T> St<'a, T , T = Qux> { + | +++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/associated-types-eq-2.rs:119:21 + | +LL | impl<'a, T> St<'a , T = Qux> { + | ^^^^^^^ associated type not allowed here + | +help: to use `Qux` as a generic argument specify it directly | -LL | fn baz(x: &>::A) {} - | ^^^^^ associated type not allowed here +LL | impl<'a, T> St<'a , Qux> { + | ~~~ -error: aborting due to 1 previous error +error: aborting due to 27 previous errors -For more information about this error, try `rustc --explain E0229`. +Some errors have detailed explanations: E0046, E0107, E0229, E0658. +For more information about an error, try `rustc --explain E0046`. diff --git a/tests/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr b/tests/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr index 92dedd74feb50..941764a575e6b 100644 --- a/tests/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr +++ b/tests/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr @@ -29,6 +29,11 @@ error[E0229]: associated type bindings are not allowed here | LL | impl Foo for Bar { | ^^^^^ associated type not allowed here + | +help: to use `3` as a generic argument specify it directly + | +LL | impl Foo<3> for Bar { + | ~ error: aborting due to 3 previous errors diff --git a/tests/ui/const-generics/parser-error-recovery/issue-89013.stderr b/tests/ui/const-generics/parser-error-recovery/issue-89013.stderr index 801d14b395054..a4c9e065c15ca 100644 --- a/tests/ui/const-generics/parser-error-recovery/issue-89013.stderr +++ b/tests/ui/const-generics/parser-error-recovery/issue-89013.stderr @@ -41,6 +41,11 @@ error[E0229]: associated type bindings are not allowed here | LL | impl Foo for Bar { | ^^^^^^^^^^^ associated type not allowed here + | +help: to use `3` as a generic argument specify it directly + | +LL | impl Foo<3> for Bar { + | ~ error: aborting due to 4 previous errors diff --git a/tests/ui/error-codes/E0229.stderr b/tests/ui/error-codes/E0229.stderr index bd8e1955ac684..ae7dc9ac26599 100644 --- a/tests/ui/error-codes/E0229.stderr +++ b/tests/ui/error-codes/E0229.stderr @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here | LL | fn baz(x: &>::A) {} | ^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | fn baz(x: &>::A) {} + | ~~~~~~~ error[E0229]: associated type bindings are not allowed here --> $DIR/E0229.rs:13:25 @@ -11,6 +16,10 @@ LL | fn baz(x: &>::A) {} | ^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | fn baz(x: &>::A) {} + | ~~~~~~~ error[E0229]: associated type bindings are not allowed here --> $DIR/E0229.rs:13:25 @@ -19,6 +28,10 @@ LL | fn baz(x: &>::A) {} | ^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | fn baz(x: &>::A) {} + | ~~~~~~~ error[E0277]: the trait bound `I: Foo` is not satisfied --> $DIR/E0229.rs:13:15 diff --git a/tests/ui/generic-associated-types/issue-102335-gat.stderr b/tests/ui/generic-associated-types/issue-102335-gat.stderr index f5e782e92fc6a..23b114a3a55e2 100644 --- a/tests/ui/generic-associated-types/issue-102335-gat.stderr +++ b/tests/ui/generic-associated-types/issue-102335-gat.stderr @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here | LL | type A: S = ()>; | ^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | type A: S = ()>; + | ~~~~~~~~~~ error[E0229]: associated type bindings are not allowed here --> $DIR/issue-102335-gat.rs:2:21 @@ -11,6 +16,10 @@ LL | type A: S = ()>; | ^^^^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | type A: S = ()>; + | ~~~~~~~~~~ error: aborting due to 2 previous errors diff --git a/tests/ui/lifetimes/issue-83753-invalid-associated-type-supertrait-hrtb.stderr b/tests/ui/lifetimes/issue-83753-invalid-associated-type-supertrait-hrtb.stderr index d99ea6a0c309f..d6da842e6abae 100644 --- a/tests/ui/lifetimes/issue-83753-invalid-associated-type-supertrait-hrtb.stderr +++ b/tests/ui/lifetimes/issue-83753-invalid-associated-type-supertrait-hrtb.stderr @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here | LL | fn bar(foo: Foo) {} | ^^^^^^^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | fn bar(foo: Foo) {} + | ~~~~~~~~~~~~~~~~ error: aborting due to 1 previous error diff --git a/tests/ui/suggestions/issue-85347.rs b/tests/ui/suggestions/issue-85347.rs index d14cf07d915ed..95e76e76cfa8a 100644 --- a/tests/ui/suggestions/issue-85347.rs +++ b/tests/ui/suggestions/issue-85347.rs @@ -2,11 +2,13 @@ use std::ops::Deref; trait Foo { type Bar<'a>: Deref::Bar>; //~^ ERROR associated type takes 1 lifetime argument but 0 lifetime arguments were supplied - //~| ERROR associated type bindings are not allowed here //~| HELP add missing - //~| ERROR associated type takes 1 lifetime argument but 0 lifetime arguments were supplied //~| ERROR associated type bindings are not allowed here + //~| HELP consider removing this type binding + //~| ERROR associated type takes 1 lifetime argument but 0 lifetime arguments were supplied //~| HELP add missing + //~| ERROR associated type bindings are not allowed here + //~| HELP consider removing this type binding } fn main() {} diff --git a/tests/ui/suggestions/issue-85347.stderr b/tests/ui/suggestions/issue-85347.stderr index 45f87e539b4e6..de0aa09ce49c7 100644 --- a/tests/ui/suggestions/issue-85347.stderr +++ b/tests/ui/suggestions/issue-85347.stderr @@ -19,6 +19,11 @@ error[E0229]: associated type bindings are not allowed here | LL | type Bar<'a>: Deref::Bar>; | ^^^^^^^^^^^^^ associated type not allowed here + | +help: consider removing this type binding + | +LL | type Bar<'a>: Deref::Bar>; + | ~~~~~~~~~~~~~~~ error[E0107]: associated type takes 1 lifetime argument but 0 lifetime arguments were supplied --> $DIR/issue-85347.rs:3:42 @@ -44,6 +49,10 @@ LL | type Bar<'a>: Deref::Bar>; | ^^^^^^^^^^^^^ associated type not allowed here | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider removing this type binding + | +LL | type Bar<'a>: Deref::Bar>; + | ~~~~~~~~~~~~~~~ error: aborting due to 4 previous errors From c623319a30d50d88091120fe5ac9354572d56662 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Mar 2024 03:29:52 +0100 Subject: [PATCH 07/22] Lower deref patterns to MIR This handles using deref patterns to choose the correct match arm. This does not handle bindings or guards. Co-authored-by: Deadbeef --- .../src/mem_categorization.rs | 8 +- .../rustc_mir_build/src/build/matches/mod.rs | 7 ++ .../rustc_mir_build/src/build/matches/test.rs | 88 +++++++++++++------ .../rustc_mir_build/src/build/matches/util.rs | 15 ++-- tests/ui/pattern/deref-patterns/bindings.rs | 37 ++++++++ tests/ui/pattern/deref-patterns/branch.rs | 40 +++++++++ .../cant_move_out_of_pattern.rs | 24 +++++ .../cant_move_out_of_pattern.stderr | 27 ++++++ tests/ui/pattern/deref-patterns/typeck.rs | 16 ++-- .../ui/pattern/deref-patterns/typeck_fail.rs | 17 ++++ .../pattern/deref-patterns/typeck_fail.stderr | 19 ++++ 11 files changed, 259 insertions(+), 39 deletions(-) create mode 100644 tests/ui/pattern/deref-patterns/bindings.rs create mode 100644 tests/ui/pattern/deref-patterns/branch.rs create mode 100644 tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs create mode 100644 tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr create mode 100644 tests/ui/pattern/deref-patterns/typeck_fail.rs create mode 100644 tests/ui/pattern/deref-patterns/typeck_fail.stderr diff --git a/compiler/rustc_hir_typeck/src/mem_categorization.rs b/compiler/rustc_hir_typeck/src/mem_categorization.rs index 859877962fe78..15f802560fd35 100644 --- a/compiler/rustc_hir_typeck/src/mem_categorization.rs +++ b/compiler/rustc_hir_typeck/src/mem_categorization.rs @@ -711,13 +711,19 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> { self.cat_pattern_(place_with_id, subpat, op)?; } - PatKind::Box(subpat) | PatKind::Ref(subpat, _) | PatKind::Deref(subpat) => { + PatKind::Box(subpat) | PatKind::Ref(subpat, _) => { // box p1, &p1, &mut p1. we can ignore the mutability of // PatKind::Ref since that information is already contained // in the type. let subplace = self.cat_deref(pat, place_with_id)?; self.cat_pattern_(subplace, subpat, op)?; } + PatKind::Deref(subpat) => { + let ty = self.pat_ty_adjusted(subpat)?; + // A deref pattern generates a temporary. + let place = self.cat_rvalue(pat.hir_id, ty); + self.cat_pattern_(place, subpat, op)?; + } PatKind::Slice(before, ref slice, after) => { let Some(element_ty) = place_with_id.place.ty().builtin_index() else { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 9730473c4288f..f16dddeaa6813 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1163,6 +1163,7 @@ enum TestCase<'pat, 'tcx> { Constant { value: mir::Const<'tcx> }, Range(&'pat PatRange<'tcx>), Slice { len: usize, variable_length: bool }, + Deref { temp: Place<'tcx> }, Or { pats: Box<[FlatPat<'pat, 'tcx>]> }, } @@ -1222,6 +1223,12 @@ enum TestKind<'tcx> { /// Test that the length of the slice is equal to `len`. Len { len: u64, op: BinOp }, + + /// Call `Deref::deref` on the value. + Deref { + /// Temporary to store the result of `deref()`. + temp: Place<'tcx>, + }, } /// A test to perform to determine which [`Candidate`] matches a value. diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 690879b94885a..f6827fd7c5256 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -42,6 +42,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { TestKind::Len { len: len as u64, op } } + TestCase::Deref { temp } => TestKind::Deref { temp }, + TestCase::Or { .. } => bug!("or-patterns should have already been handled"), TestCase::Irrefutable { .. } => span_bug!( @@ -143,35 +145,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } let re_erased = tcx.lifetimes.re_erased; - let ref_string = self.temp(Ty::new_imm_ref(tcx, re_erased, ty), test.span); let ref_str_ty = Ty::new_imm_ref(tcx, re_erased, tcx.types.str_); let ref_str = self.temp(ref_str_ty, test.span); - let deref = tcx.require_lang_item(LangItem::Deref, None); - let method = trait_method(tcx, deref, sym::deref, [ty]); let eq_block = self.cfg.start_new_block(); - self.cfg.push_assign( - block, - source_info, - ref_string, - Rvalue::Ref(re_erased, BorrowKind::Shared, place), - ); - self.cfg.terminate( - block, - source_info, - TerminatorKind::Call { - func: Operand::Constant(Box::new(ConstOperand { - span: test.span, - user_ty: None, - const_: method, - })), - args: vec![Spanned { node: Operand::Move(ref_string), span: DUMMY_SP }], - destination: ref_str, - target: Some(eq_block), - unwind: UnwindAction::Continue, - call_source: CallSource::Misc, - fn_span: source_info.span, - }, - ); + // `let ref_str: &str = ::deref(&place);` + self.call_deref(block, eq_block, place, ty, ref_str, test.span); self.non_scalar_compare( eq_block, success_block, @@ -270,9 +248,57 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Operand::Move(expected), ); } + + TestKind::Deref { temp } => { + let ty = place_ty.ty; + let target = target_block(TestBranch::Success); + self.call_deref(block, target, place, ty, temp, test.span); + } } } + /// Perform `let temp = ::deref(&place)`. + pub(super) fn call_deref( + &mut self, + block: BasicBlock, + target_block: BasicBlock, + place: Place<'tcx>, + ty: Ty<'tcx>, + temp: Place<'tcx>, + span: Span, + ) { + let source_info = self.source_info(span); + let re_erased = self.tcx.lifetimes.re_erased; + let deref = self.tcx.require_lang_item(LangItem::Deref, None); + let method = trait_method(self.tcx, deref, sym::deref, [ty]); + let ref_src = self.temp(Ty::new_imm_ref(self.tcx, re_erased, ty), span); + // `let ref_src = &src_place;` + self.cfg.push_assign( + block, + source_info, + ref_src, + Rvalue::Ref(re_erased, BorrowKind::Shared, place), + ); + // `let temp = ::deref(ref_src);` + self.cfg.terminate( + block, + source_info, + TerminatorKind::Call { + func: Operand::Constant(Box::new(ConstOperand { + span, + user_ty: None, + const_: method, + })), + args: vec![Spanned { node: Operand::Move(ref_src), span }], + destination: temp, + target: Some(target_block), + unwind: UnwindAction::Continue, + call_source: CallSource::Misc, + fn_span: source_info.span, + }, + ); + } + /// Compare using the provided built-in comparison operator fn compare( &mut self, @@ -660,13 +686,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + (TestKind::Deref { temp: test_temp }, TestCase::Deref { temp }) + if test_temp == temp => + { + fully_matched = true; + Some(TestBranch::Success) + } + ( TestKind::Switch { .. } | TestKind::SwitchInt { .. } | TestKind::If | TestKind::Len { .. } | TestKind::Range { .. } - | TestKind::Eq { .. }, + | TestKind::Eq { .. } + | TestKind::Deref { .. }, _, ) => { fully_matched = false; diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index d6376b7b0dcac..4d8248c0e395f 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -5,8 +5,8 @@ use rustc_data_structures::fx::FxIndexSet; use rustc_infer::infer::type_variable::TypeVariableOrigin; use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; -use rustc_middle::ty; use rustc_middle::ty::TypeVisitableExt; +use rustc_middle::ty::{self, Ty}; impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn field_match_pairs<'pat>( @@ -249,10 +249,15 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { default_irrefutable() } - PatKind::DerefPattern { .. } => { - // FIXME(deref_patterns) - // Treat it like a wildcard for now. - default_irrefutable() + PatKind::DerefPattern { ref subpattern } => { + // Create a new temporary for each deref pattern. + // FIXME(deref_patterns): dedup temporaries to avoid multiple `deref()` calls? + let temp = cx.temp( + Ty::new_imm_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty), + pattern.span, + ); + subpairs.push(MatchPair::new(PlaceBuilder::from(temp).deref(), subpattern, cx)); + TestCase::Deref { temp } } }; diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs new file mode 100644 index 0000000000000..75d59c1967ad6 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -0,0 +1,37 @@ +//@ run-pass +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn simple_vec(vec: Vec) -> u32 { + match vec { + deref!([]) => 100, + // FIXME(deref_patterns): fake borrows break guards + // deref!([x]) if x == 4 => x + 4, + deref!([x]) => x, + deref!([1, x]) => x + 200, + deref!(ref slice) => slice.iter().sum(), + _ => 2000, + } +} + +fn nested_vec(vecvec: Vec>) -> u32 { + match vecvec { + deref!([]) => 0, + deref!([deref!([x])]) => x, + deref!([deref!([0, x]) | deref!([1, x])]) => x, + deref!([ref x]) => x.iter().sum(), + deref!([deref!([]), deref!([1, x, y])]) => y - x, + _ => 2000, + } +} + +fn main() { + assert_eq!(simple_vec(vec![1]), 1); + assert_eq!(simple_vec(vec![1, 2]), 202); + assert_eq!(simple_vec(vec![1, 2, 3]), 6); + + assert_eq!(nested_vec(vec![vec![0, 42]]), 42); + assert_eq!(nested_vec(vec![vec![1, 42]]), 42); + assert_eq!(nested_vec(vec![vec![1, 2, 3]]), 6); + assert_eq!(nested_vec(vec![vec![], vec![1, 2, 3]]), 1); +} diff --git a/tests/ui/pattern/deref-patterns/branch.rs b/tests/ui/pattern/deref-patterns/branch.rs new file mode 100644 index 0000000000000..1bac1006d9dc0 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/branch.rs @@ -0,0 +1,40 @@ +//@ run-pass +// Test the execution of deref patterns. +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn branch(vec: Vec) -> u32 { + match vec { + deref!([]) => 0, + deref!([1, _, 3]) => 1, + deref!([2, ..]) => 2, + _ => 1000, + } +} + +fn nested(vec: Vec>) -> u32 { + match vec { + deref!([deref!([]), ..]) => 1, + deref!([deref!([0, ..]), deref!([1, ..])]) => 2, + _ => 1000, + } +} + +fn main() { + assert!(matches!(Vec::::new(), deref!([]))); + assert!(matches!(vec![1], deref!([1]))); + assert!(matches!(&vec![1], deref!([1]))); + assert!(matches!(vec![&1], deref!([1]))); + assert!(matches!(vec![vec![1]], deref!([deref!([1])]))); + + assert_eq!(branch(vec![]), 0); + assert_eq!(branch(vec![1, 2, 3]), 1); + assert_eq!(branch(vec![3, 2, 1]), 1000); + assert_eq!(branch(vec![2]), 2); + assert_eq!(branch(vec![2, 3]), 2); + assert_eq!(branch(vec![3, 2]), 1000); + + assert_eq!(nested(vec![vec![], vec![2]]), 1); + assert_eq!(nested(vec![vec![0], vec![1]]), 2); + assert_eq!(nested(vec![vec![0, 2], vec![1, 2]]), 2); +} diff --git a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs new file mode 100644 index 0000000000000..84b5ec09dc7d8 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs @@ -0,0 +1,24 @@ +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +use std::rc::Rc; + +struct Struct; + +fn cant_move_out_box(b: Box) -> Struct { + match b { + //~^ ERROR: cannot move out of a shared reference + deref!(x) => x, + _ => unreachable!(), + } +} + +fn cant_move_out_rc(rc: Rc) -> Struct { + match rc { + //~^ ERROR: cannot move out of a shared reference + deref!(x) => x, + _ => unreachable!(), + } +} + +fn main() {} diff --git a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr new file mode 100644 index 0000000000000..108db6d9e4b53 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr @@ -0,0 +1,27 @@ +error[E0507]: cannot move out of a shared reference + --> $DIR/cant_move_out_of_pattern.rs:9:11 + | +LL | match b { + | ^ +LL | +LL | deref!(x) => x, + | - + | | + | data moved here + | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + +error[E0507]: cannot move out of a shared reference + --> $DIR/cant_move_out_of_pattern.rs:17:11 + | +LL | match rc { + | ^^ +LL | +LL | deref!(x) => x, + | - + | | + | data moved here + | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0507`. diff --git a/tests/ui/pattern/deref-patterns/typeck.rs b/tests/ui/pattern/deref-patterns/typeck.rs index ead6dcdbaf0f9..f23f7042cd892 100644 --- a/tests/ui/pattern/deref-patterns/typeck.rs +++ b/tests/ui/pattern/deref-patterns/typeck.rs @@ -4,6 +4,8 @@ use std::rc::Rc; +struct Struct; + fn main() { let vec: Vec = Vec::new(); match vec { @@ -22,10 +24,12 @@ fn main() { deref!(1..) => {} _ => {} } - // FIXME(deref_patterns): fails to typecheck because `"foo"` has type &str but deref creates a - // place of type `str`. - // match "foo".to_string() { - // box "foo" => {} - // _ => {} - // } + let _: &Struct = match &Rc::new(Struct) { + deref!(x) => x, + _ => unreachable!(), + }; + let _: &[Struct] = match &Rc::new(vec![Struct]) { + deref!(deref!(x)) => x, + _ => unreachable!(), + }; } diff --git a/tests/ui/pattern/deref-patterns/typeck_fail.rs b/tests/ui/pattern/deref-patterns/typeck_fail.rs new file mode 100644 index 0000000000000..040118449ecca --- /dev/null +++ b/tests/ui/pattern/deref-patterns/typeck_fail.rs @@ -0,0 +1,17 @@ +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn main() { + // FIXME(deref_patterns): fails to typecheck because `"foo"` has type &str but deref creates a + // place of type `str`. + match "foo".to_string() { + deref!("foo") => {} + //~^ ERROR: mismatched types + _ => {} + } + match &"foo".to_string() { + deref!("foo") => {} + //~^ ERROR: mismatched types + _ => {} + } +} diff --git a/tests/ui/pattern/deref-patterns/typeck_fail.stderr b/tests/ui/pattern/deref-patterns/typeck_fail.stderr new file mode 100644 index 0000000000000..1c14802745a0c --- /dev/null +++ b/tests/ui/pattern/deref-patterns/typeck_fail.stderr @@ -0,0 +1,19 @@ +error[E0308]: mismatched types + --> $DIR/typeck_fail.rs:8:16 + | +LL | match "foo".to_string() { + | ----------------- this expression has type `String` +LL | deref!("foo") => {} + | ^^^^^ expected `str`, found `&str` + +error[E0308]: mismatched types + --> $DIR/typeck_fail.rs:13:16 + | +LL | match &"foo".to_string() { + | ------------------ this expression has type `&String` +LL | deref!("foo") => {} + | ^^^^^ expected `str`, found `&str` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 1dabacd059403a7dabf283f4ee02148cb5f69bd4 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Mar 2024 16:10:45 +0100 Subject: [PATCH 08/22] Don't fake borrow inside a deref pattern --- compiler/rustc_mir_build/src/build/matches/mod.rs | 14 ++++++++++++-- tests/ui/pattern/deref-patterns/bindings.rs | 4 ++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index f16dddeaa6813..6094c8e759c0a 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -5,7 +5,7 @@ //! This also includes code for pattern bindings in `let` statements and //! function parameters. -use crate::build::expr::as_place::PlaceBuilder; +use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; use crate::build::scope::DropKind; use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; @@ -438,7 +438,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } if let Some(ref borrows) = fake_borrows { - self.calculate_fake_borrows(borrows, scrutinee_span) + self.calculate_fake_borrows(borrows, scrutinee_place_builder.base(), scrutinee_span) } else { Vec::new() } @@ -1936,6 +1936,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn calculate_fake_borrows<'b>( &mut self, fake_borrows: &'b FxIndexSet>, + scrutinee_base: PlaceBase, temp_span: Span, ) -> Vec<(Place<'tcx>, Local)> { let tcx = self.tcx; @@ -1946,6 +1947,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Insert a Shallow borrow of the prefixes of any fake borrows. for place in fake_borrows { + if let PlaceBase::Local(l) = scrutinee_base + && l != place.local + { + // The base of this place is a temporary created for deref patterns. We don't emit + // fake borrows for these as they are not initialized in all branches. + // FIXME(deref_patterns): is this sound? + continue; + } + let mut cursor = place.projection.as_ref(); while let [proj_base @ .., elem] = cursor { cursor = proj_base; diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index 75d59c1967ad6..c0c8a70dbf0dd 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -5,8 +5,7 @@ fn simple_vec(vec: Vec) -> u32 { match vec { deref!([]) => 100, - // FIXME(deref_patterns): fake borrows break guards - // deref!([x]) if x == 4 => x + 4, + deref!([x]) if x == 4 => x + 4, deref!([x]) => x, deref!([1, x]) => x + 200, deref!(ref slice) => slice.iter().sum(), @@ -29,6 +28,7 @@ fn main() { assert_eq!(simple_vec(vec![1]), 1); assert_eq!(simple_vec(vec![1, 2]), 202); assert_eq!(simple_vec(vec![1, 2, 3]), 6); + assert_eq!(simple_vec(vec![4]), 8); assert_eq!(nested_vec(vec![vec![0, 42]]), 42); assert_eq!(nested_vec(vec![vec![1, 42]]), 42); From 5c4909b8e18eb4fcc7e3b8b6826be5890aa8e529 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 4 Apr 2024 00:25:16 +0200 Subject: [PATCH 09/22] Track mutability of deref patterns --- compiler/rustc_middle/src/thir.rs | 5 +++-- compiler/rustc_middle/src/thir/visit.rs | 2 +- compiler/rustc_mir_build/src/build/matches/mod.rs | 2 +- compiler/rustc_mir_build/src/build/matches/util.rs | 2 +- compiler/rustc_mir_build/src/thir/pattern/mod.rs | 4 +++- compiler/rustc_mir_build/src/thir/print.rs | 2 +- 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 66130a8dde03b..b55fd0a9231d0 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -642,7 +642,7 @@ impl<'tcx> Pat<'tcx> { AscribeUserType { subpattern, .. } | Binding { subpattern: Some(subpattern), .. } | Deref { subpattern } - | DerefPattern { subpattern } + | DerefPattern { subpattern, .. } | InlineConstant { subpattern, .. } => subpattern.walk_(it), Leaf { subpatterns } | Variant { subpatterns, .. } => { subpatterns.iter().for_each(|field| field.pattern.walk_(it)) @@ -760,6 +760,7 @@ pub enum PatKind<'tcx> { /// Deref pattern, written `box P` for now. DerefPattern { subpattern: Box>, + mutability: hir::Mutability, }, /// One of the following: @@ -1163,7 +1164,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> { } write!(f, "{subpattern}") } - PatKind::DerefPattern { ref subpattern } => { + PatKind::DerefPattern { ref subpattern, .. } => { write!(f, "deref!({subpattern})") } PatKind::Constant { value } => write!(f, "{value}"), diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index e42b85530b502..f198881043739 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -229,7 +229,7 @@ pub fn walk_pat<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>( match &pat.kind { AscribeUserType { subpattern, ascription: _ } | Deref { subpattern } - | DerefPattern { subpattern } + | DerefPattern { subpattern, .. } | Binding { subpattern: Some(subpattern), .. } => visitor.visit_pat(subpattern), Binding { .. } | Wild | Never | Error(_) => {} Variant { subpatterns, adt_def: _, args: _, variant_index: _ } | Leaf { subpatterns } => { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 6094c8e759c0a..2ac2c2546dd62 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -938,7 +938,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.visit_primary_bindings(subpattern, pattern_user_ty.deref(), f); } - PatKind::DerefPattern { ref subpattern } => { + PatKind::DerefPattern { ref subpattern, .. } => { self.visit_primary_bindings(subpattern, UserTypeProjections::none(), f); } diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 4d8248c0e395f..3292ab1f1b7e6 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -249,7 +249,7 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { default_irrefutable() } - PatKind::DerefPattern { ref subpattern } => { + PatKind::DerefPattern { ref subpattern, .. } => { // Create a new temporary for each deref pattern. // FIXME(deref_patterns): dedup temporaries to avoid multiple `deref()` calls? let temp = cx.temp( diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index bcb43a0054709..5c016682d8d2d 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -264,7 +264,9 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { } hir::PatKind::Deref(subpattern) => { - PatKind::DerefPattern { subpattern: self.lower_pattern(subpattern) } + let mutable = self.typeck_results.pat_has_ref_mut_binding(subpattern); + let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + PatKind::DerefPattern { subpattern: self.lower_pattern(subpattern), mutability } } hir::PatKind::Ref(subpattern, _) | hir::PatKind::Box(subpattern) => { PatKind::Deref { subpattern: self.lower_pattern(subpattern) } diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index 49e48427b6555..619bfbcf43d3a 100644 --- a/compiler/rustc_mir_build/src/thir/print.rs +++ b/compiler/rustc_mir_build/src/thir/print.rs @@ -688,7 +688,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> { self.print_pat(subpattern, depth_lvl + 2); print_indented!(self, "}", depth_lvl + 1); } - PatKind::DerefPattern { subpattern } => { + PatKind::DerefPattern { subpattern, .. } => { print_indented!(self, "DerefPattern { ", depth_lvl + 1); print_indented!(self, "subpattern:", depth_lvl + 2); self.print_pat(subpattern, depth_lvl + 2); From 377e095371ae99dfdd61d5b50d4a43422b34ca23 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 4 Apr 2024 00:46:35 +0200 Subject: [PATCH 10/22] Allow mutable bindings inside deref patterns --- .../rustc_mir_build/src/build/matches/mod.rs | 7 ++-- .../rustc_mir_build/src/build/matches/test.rs | 35 ++++++++++++++----- .../rustc_mir_build/src/build/matches/util.rs | 6 ++-- tests/ui/pattern/deref-patterns/bindings.rs | 15 ++++++++ 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 2ac2c2546dd62..265d80ed8bc59 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1163,7 +1163,7 @@ enum TestCase<'pat, 'tcx> { Constant { value: mir::Const<'tcx> }, Range(&'pat PatRange<'tcx>), Slice { len: usize, variable_length: bool }, - Deref { temp: Place<'tcx> }, + Deref { temp: Place<'tcx>, mutability: Mutability }, Or { pats: Box<[FlatPat<'pat, 'tcx>]> }, } @@ -1224,10 +1224,11 @@ enum TestKind<'tcx> { /// Test that the length of the slice is equal to `len`. Len { len: u64, op: BinOp }, - /// Call `Deref::deref` on the value. + /// Call `Deref::deref[_mut]` on the value. Deref { - /// Temporary to store the result of `deref()`. + /// Temporary to store the result of `deref()`/`deref_mut()`. temp: Place<'tcx>, + mutability: Mutability, }, } diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index f6827fd7c5256..5dd478aa4224a 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -42,7 +42,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { TestKind::Len { len: len as u64, op } } - TestCase::Deref { temp } => TestKind::Deref { temp }, + TestCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability }, TestCase::Or { .. } => bug!("or-patterns should have already been handled"), @@ -149,7 +149,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let ref_str = self.temp(ref_str_ty, test.span); let eq_block = self.cfg.start_new_block(); // `let ref_str: &str = ::deref(&place);` - self.call_deref(block, eq_block, place, ty, ref_str, test.span); + self.call_deref( + block, + eq_block, + place, + Mutability::Not, + ty, + ref_str, + test.span, + ); self.non_scalar_compare( eq_block, success_block, @@ -249,37 +257,46 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } - TestKind::Deref { temp } => { + TestKind::Deref { temp, mutability } => { let ty = place_ty.ty; let target = target_block(TestBranch::Success); - self.call_deref(block, target, place, ty, temp, test.span); + self.call_deref(block, target, place, mutability, ty, temp, test.span); } } } /// Perform `let temp = ::deref(&place)`. + /// or `let temp = ::deref_mut(&mut place)`. pub(super) fn call_deref( &mut self, block: BasicBlock, target_block: BasicBlock, place: Place<'tcx>, + mutability: Mutability, ty: Ty<'tcx>, temp: Place<'tcx>, span: Span, ) { + let (trait_item, method) = match mutability { + Mutability::Not => (LangItem::Deref, sym::deref), + Mutability::Mut => (LangItem::DerefMut, sym::deref_mut), + }; + let borrow_kind = super::util::ref_pat_borrow_kind(mutability); let source_info = self.source_info(span); let re_erased = self.tcx.lifetimes.re_erased; - let deref = self.tcx.require_lang_item(LangItem::Deref, None); - let method = trait_method(self.tcx, deref, sym::deref, [ty]); - let ref_src = self.temp(Ty::new_imm_ref(self.tcx, re_erased, ty), span); + let trait_item = self.tcx.require_lang_item(trait_item, None); + let method = trait_method(self.tcx, trait_item, method, [ty]); + let ref_src = self.temp(Ty::new_ref(self.tcx, re_erased, ty, mutability), span); // `let ref_src = &src_place;` + // or `let ref_src = &mut src_place;` self.cfg.push_assign( block, source_info, ref_src, - Rvalue::Ref(re_erased, BorrowKind::Shared, place), + Rvalue::Ref(re_erased, borrow_kind, place), ); // `let temp = ::deref(ref_src);` + // or `let temp = ::deref_mut(ref_src);` self.cfg.terminate( block, source_info, @@ -686,7 +703,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - (TestKind::Deref { temp: test_temp }, TestCase::Deref { temp }) + (TestKind::Deref { temp: test_temp, .. }, TestCase::Deref { temp, .. }) if test_temp == temp => { fully_matched = true; diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 3292ab1f1b7e6..8f9fb8beba176 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -249,15 +249,15 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { default_irrefutable() } - PatKind::DerefPattern { ref subpattern, .. } => { + PatKind::DerefPattern { ref subpattern, mutability } => { // Create a new temporary for each deref pattern. // FIXME(deref_patterns): dedup temporaries to avoid multiple `deref()` calls? let temp = cx.temp( - Ty::new_imm_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty), + Ty::new_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty, mutability), pattern.span, ); subpairs.push(MatchPair::new(PlaceBuilder::from(temp).deref(), subpattern, cx)); - TestCase::Deref { temp } + TestCase::Deref { temp, mutability } } }; diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index c0c8a70dbf0dd..4f72058af8fe5 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -24,6 +24,19 @@ fn nested_vec(vecvec: Vec>) -> u32 { } } +fn ref_mut(val: u32) -> u32 { + let mut b = Box::new(0u32); + match &mut b { + deref!(_x) if false => unreachable!(), + deref!(x) => { + *x = val; + } + _ => unreachable!(), + } + let deref!(x) = &b else { unreachable!() }; + *x +} + fn main() { assert_eq!(simple_vec(vec![1]), 1); assert_eq!(simple_vec(vec![1, 2]), 202); @@ -34,4 +47,6 @@ fn main() { assert_eq!(nested_vec(vec![vec![1, 42]]), 42); assert_eq!(nested_vec(vec![vec![1, 2, 3]]), 6); assert_eq!(nested_vec(vec![vec![], vec![1, 2, 3]]), 1); + + assert_eq!(ref_mut(42), 42) } From b55afe475aee1e51196e35845ba1553c96f041cf Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Fri, 5 Apr 2024 23:42:29 +0200 Subject: [PATCH 11/22] Address closure-related review --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 9 ++++++++ .../src/mem_categorization.rs | 4 ++++ .../rustc_middle/src/ty/typeck_results.rs | 2 +- .../pattern/deref-patterns/closure_capture.rs | 21 +++++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/ui/pattern/deref-patterns/closure_capture.rs diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index a0a5a75d3820a..cc42e69f53827 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -750,6 +750,15 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } } + } else if let PatKind::Deref(subpattern) = pat.kind { + // A deref pattern is a bit special: the binding mode of its inner bindings + // determines whether to borrow *at the level of the deref pattern* rather than + // borrowing the bound place (since that inner place is inside the temporary that + // stores the result of calling `deref()`/`deref_mut()` so can't be captured). + let mutable = mc.typeck_results.pat_has_ref_mut_binding(subpattern); + let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + let bk = ty::BorrowKind::from_mutbl(mutability); + delegate.borrow(place, discr_place.hir_id, bk); } })); } diff --git a/compiler/rustc_hir_typeck/src/mem_categorization.rs b/compiler/rustc_hir_typeck/src/mem_categorization.rs index 15f802560fd35..b44c2345933fb 100644 --- a/compiler/rustc_hir_typeck/src/mem_categorization.rs +++ b/compiler/rustc_hir_typeck/src/mem_categorization.rs @@ -719,7 +719,11 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> { self.cat_pattern_(subplace, subpat, op)?; } PatKind::Deref(subpat) => { + let mutable = self.typeck_results.pat_has_ref_mut_binding(subpat); + let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + let re_erased = self.tcx().lifetimes.re_erased; let ty = self.pat_ty_adjusted(subpat)?; + let ty = Ty::new_ref(self.tcx(), re_erased, ty, mutability); // A deref pattern generates a temporary. let place = self.cat_rvalue(pat.hir_id, ty); self.cat_pattern_(place, subpat, op)?; diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs index a28afcc4fb8c2..25bca70f1029e 100644 --- a/compiler/rustc_middle/src/ty/typeck_results.rs +++ b/compiler/rustc_middle/src/ty/typeck_results.rs @@ -451,7 +451,7 @@ impl<'tcx> TypeckResults<'tcx> { /// This is computed from the typeck results since we want to make /// sure to apply any match-ergonomics adjustments, which we cannot /// determine from the HIR alone. - pub fn pat_has_ref_mut_binding(&self, pat: &'tcx hir::Pat<'tcx>) -> bool { + pub fn pat_has_ref_mut_binding(&self, pat: &hir::Pat<'_>) -> bool { let mut has_ref_mut = false; pat.walk(|pat| { if let hir::PatKind::Binding(_, id, _, _) = pat.kind diff --git a/tests/ui/pattern/deref-patterns/closure_capture.rs b/tests/ui/pattern/deref-patterns/closure_capture.rs new file mode 100644 index 0000000000000..fc0ddedac2b78 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/closure_capture.rs @@ -0,0 +1,21 @@ +//@ run-pass +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn main() { + let b = Box::new("aaa".to_string()); + let f = || { + let deref!(ref s) = b else { unreachable!() }; + assert_eq!(s.len(), 3); + }; + assert_eq!(b.len(), 3); + f(); + + let mut b = Box::new("aaa".to_string()); + let mut f = || { + let deref!(ref mut s) = b else { unreachable!() }; + s.push_str("aa"); + }; + f(); + assert_eq!(b.len(), 5); +} From 511bd78863052fae0026698ed139774f3334e5b1 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 7 Apr 2024 01:23:39 +0200 Subject: [PATCH 12/22] Rework fake borrow calculation --- compiler/rustc_middle/src/mir/statement.rs | 5 + .../rustc_mir_build/src/build/matches/mod.rs | 117 ++------------- .../rustc_mir_build/src/build/matches/util.rs | 139 ++++++++++++++++-- ...se_edges.full_tested_match.built.after.mir | 12 +- ...e_edges.full_tested_match2.built.after.mir | 12 +- .../match_false_edges.main.built.after.mir | 20 +-- ....constant_eq.SimplifyCfg-initial.after.mir | 24 +-- ...joint_ranges.SimplifyCfg-initial.after.mir | 24 +-- ...guard.CleanupPostBorrowck.panic-abort.diff | 16 +- ...uard.CleanupPostBorrowck.panic-unwind.diff | 16 +- 10 files changed, 206 insertions(+), 179 deletions(-) diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs index 069c8019cb2c8..235298ffcb8bc 100644 --- a/compiler/rustc_middle/src/mir/statement.rs +++ b/compiler/rustc_middle/src/mir/statement.rs @@ -236,6 +236,11 @@ impl<'tcx> PlaceRef<'tcx> { } } + #[inline] + pub fn to_place(&self, tcx: TyCtxt<'tcx>) -> Place<'tcx> { + Place { local: self.local, projection: tcx.mk_place_elems(self.projection) } + } + #[inline] pub fn last_projection(&self) -> Option<(PlaceRef<'tcx>, PlaceElem<'tcx>)> { if let &[ref proj_base @ .., elem] = self.projection { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 265d80ed8bc59..648d0b52b3214 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -5,15 +5,12 @@ //! This also includes code for pattern bindings in `let` statements and //! function parameters. -use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; +use crate::build::expr::as_place::PlaceBuilder; use crate::build::scope::DropKind; use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::build::{GuardFrame, GuardFrameLocal, LocalsForNode}; -use rustc_data_structures::{ - fx::{FxHashSet, FxIndexMap, FxIndexSet}, - stack::ensure_sufficient_stack, -}; +use rustc_data_structures::{fx::FxIndexMap, stack::ensure_sufficient_stack}; use rustc_hir::{BindingMode, ByRef}; use rustc_middle::middle::region; use rustc_middle::mir::{self, *}; @@ -209,7 +206,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// 2. Create the decision tree ([Builder::lower_match_tree]). /// 3. Determine the fake borrows that are needed from the places that were /// matched against and create the required temporaries for them - /// ([Builder::calculate_fake_borrows]). + /// ([util::collect_fake_borrows]). /// 4. Create everything else: the guards and the arms ([Builder::lower_match_arms]). /// /// ## False edges @@ -379,11 +376,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match_has_guard: bool, candidates: &mut [&mut Candidate<'pat, 'tcx>], ) -> Vec<(Place<'tcx>, Local)> { - // The set of places that we are creating fake borrows of. If there are - // no match guards then we don't need any fake borrows, so don't track - // them. - let fake_borrows = match_has_guard - .then(|| util::FakeBorrowCollector::collect_fake_borrows(self, candidates)); + // The set of places that we are creating fake borrows of. If there are no match guards then + // we don't need any fake borrows, so don't track them. + let fake_borrows: Vec<(Place<'tcx>, Local)> = if match_has_guard { + util::collect_fake_borrows( + self, + candidates, + scrutinee_span, + scrutinee_place_builder.base(), + ) + } else { + Vec::new() + }; // See the doc comment on `match_candidates` for why we have an // otherwise block. Match checking will ensure this is actually @@ -437,11 +441,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }); } - if let Some(ref borrows) = fake_borrows { - self.calculate_fake_borrows(borrows, scrutinee_place_builder.base(), scrutinee_span) - } else { - Vec::new() - } + fake_borrows } /// Lower the bindings, guards and arm bodies of a `match` expression. @@ -1911,91 +1911,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { target_blocks, ); } - - /// Determine the fake borrows that are needed from a set of places that - /// have to be stable across match guards. - /// - /// Returns a list of places that need a fake borrow and the temporary - /// that's used to store the fake borrow. - /// - /// Match exhaustiveness checking is not able to handle the case where the - /// place being matched on is mutated in the guards. We add "fake borrows" - /// to the guards that prevent any mutation of the place being matched. - /// There are a some subtleties: - /// - /// 1. Borrowing `*x` doesn't prevent assigning to `x`. If `x` is a shared - /// reference, the borrow isn't even tracked. As such we have to add fake - /// borrows of any prefixes of a place - /// 2. We don't want `match x { _ => (), }` to conflict with mutable - /// borrows of `x`, so we only add fake borrows for places which are - /// bound or tested by the match. - /// 3. We don't want the fake borrows to conflict with `ref mut` bindings, - /// so we use a special BorrowKind for them. - /// 4. The fake borrows may be of places in inactive variants, so it would - /// be UB to generate code for them. They therefore have to be removed - /// by a MIR pass run after borrow checking. - fn calculate_fake_borrows<'b>( - &mut self, - fake_borrows: &'b FxIndexSet>, - scrutinee_base: PlaceBase, - temp_span: Span, - ) -> Vec<(Place<'tcx>, Local)> { - let tcx = self.tcx; - - debug!("add_fake_borrows fake_borrows = {:?}", fake_borrows); - - let mut all_fake_borrows = Vec::with_capacity(fake_borrows.len()); - - // Insert a Shallow borrow of the prefixes of any fake borrows. - for place in fake_borrows { - if let PlaceBase::Local(l) = scrutinee_base - && l != place.local - { - // The base of this place is a temporary created for deref patterns. We don't emit - // fake borrows for these as they are not initialized in all branches. - // FIXME(deref_patterns): is this sound? - continue; - } - - let mut cursor = place.projection.as_ref(); - while let [proj_base @ .., elem] = cursor { - cursor = proj_base; - - if let ProjectionElem::Deref = elem { - // Insert a shallow borrow after a deref. For other - // projections the borrow of prefix_cursor will - // conflict with any mutation of base. - all_fake_borrows.push(PlaceRef { local: place.local, projection: proj_base }); - } - } - - all_fake_borrows.push(place.as_ref()); - } - - // Deduplicate - let mut dedup = FxHashSet::default(); - all_fake_borrows.retain(|b| dedup.insert(*b)); - - debug!("add_fake_borrows all_fake_borrows = {:?}", all_fake_borrows); - - all_fake_borrows - .into_iter() - .map(|matched_place_ref| { - let matched_place = Place { - local: matched_place_ref.local, - projection: tcx.mk_place_elems(matched_place_ref.projection), - }; - let fake_borrow_deref_ty = matched_place.ty(&self.local_decls, tcx).ty; - let fake_borrow_ty = - Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, fake_borrow_deref_ty); - let mut fake_borrow_temp = LocalDecl::new(fake_borrow_ty, temp_span); - fake_borrow_temp.local_info = ClearCrossCrate::Set(Box::new(LocalInfo::FakeBorrow)); - let fake_borrow_temp = self.local_decls.push(fake_borrow_temp); - - (matched_place, fake_borrow_temp) - }) - .collect() - } } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 8f9fb8beba176..967b0c44588fa 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -7,6 +7,7 @@ use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; use rustc_middle::ty::TypeVisitableExt; use rustc_middle::ty::{self, Ty}; +use rustc_span::Span; impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn field_match_pairs<'pat>( @@ -267,19 +268,99 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { pub(super) struct FakeBorrowCollector<'a, 'b, 'tcx> { cx: &'a mut Builder<'b, 'tcx>, + /// Base of the scrutinee place. Used to distinguish bindings inside the scrutinee place from + /// bindings inside deref patterns. + scrutinee_base: PlaceBase, fake_borrows: FxIndexSet>, } +/// Determine the set of places that have to be stable across match guards. +/// +/// Returns a list of places that need a fake borrow along with a local to store it. +/// +/// Match exhaustiveness checking is not able to handle the case where the place being matched on is +/// mutated in the guards. We add "fake borrows" to the guards that prevent any mutation of the +/// place being matched. There are a some subtleties: +/// +/// 1. Borrowing `*x` doesn't prevent assigning to `x`. If `x` is a shared reference, the borrow +/// isn't even tracked. As such we have to add fake borrows of any prefixes of a place. +/// 2. We don't want `match x { (Some(_), _) => (), .. }` to conflict with mutable borrows of `x.1`, so we +/// only add fake borrows for places which are bound or tested by the match. +/// 3. We don't want `match x { Some(_) => (), .. }` to conflict with mutable borrows of `(x as +/// Some).0`, so the borrows are a special shallow borrow that only affects the place and not its +/// projections. +/// ```rust +/// let mut x = (Some(0), true); +/// match x { +/// (Some(_), false) => {} +/// _ if { if let Some(ref mut y) = x.0 { *y += 1 }; true } => {} +/// _ => {} +/// } +/// ``` +/// 4. The fake borrows may be of places in inactive variants, e.g. here we need to fake borrow `x` +/// and `(x as Some).0`, but when we reach the guard `x` may not be `Some`. +/// ```rust +/// let mut x = (Some(Some(0)), true); +/// match x { +/// (Some(Some(_)), false) => {} +/// _ if { if let Some(Some(ref mut y)) = x.0 { *y += 1 }; true } => {} +/// _ => {} +/// } +/// ``` +/// So it would be UB to generate code for the fake borrows. They therefore have to be removed by +/// a MIR pass run after borrow checking. +pub(super) fn collect_fake_borrows<'tcx>( + cx: &mut Builder<'_, 'tcx>, + candidates: &[&mut Candidate<'_, 'tcx>], + temp_span: Span, + scrutinee_base: PlaceBase, +) -> Vec<(Place<'tcx>, Local)> { + let mut collector = + FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexSet::default() }; + for candidate in candidates.iter() { + collector.visit_candidate(candidate); + } + let fake_borrows = collector.fake_borrows; + debug!("add_fake_borrows fake_borrows = {:?}", fake_borrows); + let tcx = cx.tcx; + fake_borrows + .iter() + .copied() + .map(|matched_place| { + let fake_borrow_deref_ty = matched_place.ty(&cx.local_decls, tcx).ty; + let fake_borrow_ty = + Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, fake_borrow_deref_ty); + let mut fake_borrow_temp = LocalDecl::new(fake_borrow_ty, temp_span); + fake_borrow_temp.local_info = ClearCrossCrate::Set(Box::new(LocalInfo::FakeBorrow)); + let fake_borrow_temp = cx.local_decls.push(fake_borrow_temp); + (matched_place, fake_borrow_temp) + }) + .collect() +} + impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { - pub(super) fn collect_fake_borrows( - cx: &'a mut Builder<'b, 'tcx>, - candidates: &[&mut Candidate<'_, 'tcx>], - ) -> FxIndexSet> { - let mut collector = Self { cx, fake_borrows: FxIndexSet::default() }; - for candidate in candidates.iter() { - collector.visit_candidate(candidate); + // Fake borrow this place and its dereference prefixes. + fn fake_borrow(&mut self, place: Place<'tcx>) { + let new = self.fake_borrows.insert(place); + if !new { + return; + } + // Also fake borrow the prefixes of any fake borrow. + self.fake_borrow_deref_prefixes(place); + } + + // Fake borrow the prefixes of this place that are dereferences. + fn fake_borrow_deref_prefixes(&mut self, place: Place<'tcx>) { + for (place_ref, elem) in place.as_ref().iter_projections().rev() { + if let ProjectionElem::Deref = elem { + // Insert a shallow borrow after a deref. For other projections the borrow of + // `place_ref` will conflict with any mutation of `place.base`. + let new = self.fake_borrows.insert(place_ref.to_place(self.cx.tcx)); + if !new { + return; + } + } } - collector.fake_borrows } fn visit_candidate(&mut self, candidate: &Candidate<'_, 'tcx>) { @@ -305,10 +386,28 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { for flat_pat in pats.iter() { self.visit_flat_pat(flat_pat) } + } else if matches!(match_pair.test_case, TestCase::Deref { .. }) { + // The subpairs of a deref pattern are all places relative to the deref temporary, so we + // don't fake borrow them. Problem is, if we only shallowly fake-borrowed + // `match_pair.place`, this would allow: + // ``` + // let mut b = Box::new(false); + // match b { + // deref!(true) => {} // not reached because `*b == false` + // _ if { *b = true; false } => {} // not reached because the guard is `false` + // deref!(false) => {} // not reached because the guard changed it + // // UB because we reached the unreachable. + // } + // ``` + // FIXME(deref_patterns): Hence we fake borrow using a non-shallow borrow. + if let Some(place) = match_pair.place { + // FIXME(deref_patterns): use a non-shallow borrow. + self.fake_borrow(place); + } } else { // Insert a Shallow borrow of any place that is switched on. if let Some(place) = match_pair.place { - self.fake_borrows.insert(place); + self.fake_borrow(place); } for subpair in &match_pair.subpairs { @@ -318,6 +417,14 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { } fn visit_binding(&mut self, Binding { source, .. }: &Binding<'tcx>) { + if let PlaceBase::Local(l) = self.scrutinee_base + && l != source.local + { + // The base of this place is a temporary created for deref patterns. We don't emit fake + // borrows for these as they are not initialized in all branches. + return; + } + // Insert a borrows of prefixes of places that are bound and are // behind a dereference projection. // @@ -334,13 +441,13 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { // y if { y == 1 && (x = &2) == () } => y, // _ => 3, // } - if let Some(i) = source.projection.iter().rposition(|elem| elem == ProjectionElem::Deref) { - let proj_base = &source.projection[..i]; - self.fake_borrows.insert(Place { - local: source.local, - projection: self.cx.tcx.mk_place_elems(proj_base), - }); - } + // + // We don't just fake borrow the whole place because this is allowed: + // match u { + // _ if { u = true; false } => (), + // x => (), + // } + self.fake_borrow_deref_prefixes(*source); } } diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir index 194afdf7dd8a8..0d381ce70f175 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir @@ -4,8 +4,8 @@ fn full_tested_match() -> () { let mut _0: (); let mut _1: (i32, i32); let mut _2: std::option::Option; - let mut _3: isize; - let mut _4: &std::option::Option; + let mut _3: &std::option::Option; + let mut _4: isize; let _5: i32; let _6: &i32; let mut _7: bool; @@ -27,8 +27,8 @@ fn full_tested_match() -> () { StorageLive(_2); _2 = Option::::Some(const 42_i32); PlaceMention(_2); - _3 = discriminant(_2); - switchInt(move _3) -> [0: bb5, 1: bb2, otherwise: bb1]; + _4 = discriminant(_2); + switchInt(move _4) -> [0: bb5, 1: bb2, otherwise: bb1]; } bb1: { @@ -60,7 +60,7 @@ fn full_tested_match() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _4 = &fake _2; + _3 = &fake _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } @@ -71,7 +71,7 @@ fn full_tested_match() -> () { bb9: { StorageDead(_7); - FakeRead(ForMatchGuard, _4); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _6); StorageLive(_5); _5 = ((_2 as Some).0: i32); diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir index ae83075434f7b..d397de66fc470 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir @@ -4,8 +4,8 @@ fn full_tested_match2() -> () { let mut _0: (); let mut _1: (i32, i32); let mut _2: std::option::Option; - let mut _3: isize; - let mut _4: &std::option::Option; + let mut _3: &std::option::Option; + let mut _4: isize; let _5: i32; let _6: &i32; let mut _7: bool; @@ -27,8 +27,8 @@ fn full_tested_match2() -> () { StorageLive(_2); _2 = Option::::Some(const 42_i32); PlaceMention(_2); - _3 = discriminant(_2); - switchInt(move _3) -> [0: bb5, 1: bb2, otherwise: bb1]; + _4 = discriminant(_2); + switchInt(move _4) -> [0: bb5, 1: bb2, otherwise: bb1]; } bb1: { @@ -66,7 +66,7 @@ fn full_tested_match2() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _4 = &fake _2; + _3 = &fake _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } @@ -77,7 +77,7 @@ fn full_tested_match2() -> () { bb9: { StorageDead(_7); - FakeRead(ForMatchGuard, _4); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _6); StorageLive(_5); _5 = ((_2 as Some).0: i32); diff --git a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir index dfa31cfff6b2c..63920cec885ad 100644 --- a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir @@ -4,9 +4,9 @@ fn main() -> () { let mut _0: (); let mut _1: i32; let mut _2: std::option::Option; - let mut _3: isize; + let mut _3: &std::option::Option; let mut _4: isize; - let mut _5: &std::option::Option; + let mut _5: isize; let _6: i32; let _7: &i32; let mut _8: bool; @@ -38,8 +38,8 @@ fn main() -> () { StorageLive(_2); _2 = Option::::Some(const 1_i32); PlaceMention(_2); - _4 = discriminant(_2); - switchInt(move _4) -> [1: bb8, otherwise: bb2]; + _5 = discriminant(_2); + switchInt(move _5) -> [1: bb8, otherwise: bb2]; } bb1: { @@ -52,8 +52,8 @@ fn main() -> () { } bb3: { - _3 = discriminant(_2); - switchInt(move _3) -> [1: bb6, otherwise: bb4]; + _4 = discriminant(_2); + switchInt(move _4) -> [1: bb6, otherwise: bb4]; } bb4: { @@ -87,7 +87,7 @@ fn main() -> () { bb10: { StorageLive(_7); _7 = &((_2 as Some).0: i32); - _5 = &fake _2; + _3 = &fake _2; StorageLive(_8); _8 = guard() -> [return: bb11, unwind: bb24]; } @@ -98,7 +98,7 @@ fn main() -> () { bb12: { StorageDead(_8); - FakeRead(ForMatchGuard, _5); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _7); StorageLive(_6); _6 = ((_2 as Some).0: i32); @@ -129,7 +129,7 @@ fn main() -> () { bb16: { StorageLive(_11); _11 = &((_2 as Some).0: i32); - _5 = &fake _2; + _3 = &fake _2; StorageLive(_12); StorageLive(_13); _13 = (*_11); @@ -143,7 +143,7 @@ fn main() -> () { bb18: { StorageDead(_13); StorageDead(_12); - FakeRead(ForMatchGuard, _5); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _11); StorageLive(_10); _10 = ((_2 as Some).0: i32); diff --git a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir index c3497c6989d77..21ddd39137fb2 100644 --- a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir @@ -7,10 +7,10 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { let mut _3: (&str, bool); let mut _4: &str; let mut _5: bool; - let mut _6: bool; - let mut _7: bool; - let mut _8: &&str; - let mut _9: &bool; + let mut _6: &&str; + let mut _7: &bool; + let mut _8: bool; + let mut _9: bool; let mut _10: bool; bb0: { @@ -23,7 +23,7 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { StorageDead(_5); StorageDead(_4); PlaceMention(_3); - _7 = ::eq((_3.0: &str), const "a") -> [return: bb11, unwind: bb19]; + _9 = ::eq((_3.0: &str), const "a") -> [return: bb11, unwind: bb19]; } bb1: { @@ -52,7 +52,7 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { } bb7: { - _6 = ::eq((_3.0: &str), const "b") -> [return: bb10, unwind: bb19]; + _8 = ::eq((_3.0: &str), const "b") -> [return: bb10, unwind: bb19]; } bb8: { @@ -64,16 +64,16 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { } bb10: { - switchInt(move _6) -> [0: bb1, otherwise: bb8]; + switchInt(move _8) -> [0: bb1, otherwise: bb8]; } bb11: { - switchInt(move _7) -> [0: bb7, otherwise: bb4]; + switchInt(move _9) -> [0: bb7, otherwise: bb4]; } bb12: { - _8 = &fake (_3.0: &str); - _9 = &fake (_3.1: bool); + _6 = &fake (_3.0: &str); + _7 = &fake (_3.1: bool); StorageLive(_10); _10 = const true; switchInt(move _10) -> [0: bb14, otherwise: bb13]; @@ -81,8 +81,8 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { bb13: { StorageDead(_10); - FakeRead(ForMatchGuard, _8); - FakeRead(ForMatchGuard, _9); + FakeRead(ForMatchGuard, _6); + FakeRead(ForMatchGuard, _7); _0 = const 1_u32; goto -> bb18; } diff --git a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir index 4a1e4fb9ec56f..6068cef8fbcb0 100644 --- a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir @@ -4,17 +4,17 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { debug x => _1; debug b => _2; let mut _0: u32; - let mut _3: bool; + let mut _3: &i32; let mut _4: bool; let mut _5: bool; let mut _6: bool; - let mut _7: &i32; + let mut _7: bool; let mut _8: bool; bb0: { PlaceMention(_1); - _5 = Le(const 0_i32, _1); - switchInt(move _5) -> [0: bb3, otherwise: bb8]; + _6 = Le(const 0_i32, _1); + switchInt(move _6) -> [0: bb3, otherwise: bb8]; } bb1: { @@ -27,8 +27,8 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { } bb3: { - _3 = Le(const 10_i32, _1); - switchInt(move _3) -> [0: bb5, otherwise: bb7]; + _4 = Le(const 10_i32, _1); + switchInt(move _4) -> [0: bb5, otherwise: bb7]; } bb4: { @@ -44,17 +44,17 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { } bb7: { - _4 = Le(_1, const 20_i32); - switchInt(move _4) -> [0: bb5, otherwise: bb4]; + _5 = Le(_1, const 20_i32); + switchInt(move _5) -> [0: bb5, otherwise: bb4]; } bb8: { - _6 = Lt(_1, const 10_i32); - switchInt(move _6) -> [0: bb3, otherwise: bb2]; + _7 = Lt(_1, const 10_i32); + switchInt(move _7) -> [0: bb3, otherwise: bb2]; } bb9: { - _7 = &fake _1; + _3 = &fake _1; StorageLive(_8); _8 = _2; switchInt(move _8) -> [0: bb11, otherwise: bb10]; @@ -62,7 +62,7 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { bb10: { StorageDead(_8); - FakeRead(ForMatchGuard, _7); + FakeRead(ForMatchGuard, _3); _0 = const 0_u32; goto -> bb14; } diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff index 54da6ee659f95..8feb59ec8747b 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff @@ -5,17 +5,17 @@ debug x => _1; debug c => _2; let mut _0: i32; - let mut _3: isize; - let mut _4: &std::option::Option<&&i32>; + let mut _3: &std::option::Option<&&i32>; + let mut _4: &i32; let mut _5: &&i32; let mut _6: &&&i32; - let mut _7: &i32; + let mut _7: isize; let mut _8: bool; bb0: { PlaceMention(_1); - _3 = discriminant(_1); - switchInt(move _3) -> [1: bb2, otherwise: bb1]; + _7 = discriminant(_1); + switchInt(move _7) -> [1: bb2, otherwise: bb1]; } bb1: { @@ -33,10 +33,10 @@ } bb4: { -- _4 = &fake _1; +- _3 = &fake _1; +- _4 = &fake (*(*((_1 as Some).0: &&i32))); - _5 = &fake (*((_1 as Some).0: &&i32)); - _6 = &fake ((_1 as Some).0: &&i32); -- _7 = &fake (*(*((_1 as Some).0: &&i32))); + nop; + nop; + nop; @@ -48,10 +48,10 @@ bb5: { StorageDead(_8); +- FakeRead(ForMatchGuard, _3); - FakeRead(ForMatchGuard, _4); - FakeRead(ForMatchGuard, _5); - FakeRead(ForMatchGuard, _6); -- FakeRead(ForMatchGuard, _7); + nop; + nop; + nop; diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff index 54da6ee659f95..8feb59ec8747b 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff @@ -5,17 +5,17 @@ debug x => _1; debug c => _2; let mut _0: i32; - let mut _3: isize; - let mut _4: &std::option::Option<&&i32>; + let mut _3: &std::option::Option<&&i32>; + let mut _4: &i32; let mut _5: &&i32; let mut _6: &&&i32; - let mut _7: &i32; + let mut _7: isize; let mut _8: bool; bb0: { PlaceMention(_1); - _3 = discriminant(_1); - switchInt(move _3) -> [1: bb2, otherwise: bb1]; + _7 = discriminant(_1); + switchInt(move _7) -> [1: bb2, otherwise: bb1]; } bb1: { @@ -33,10 +33,10 @@ } bb4: { -- _4 = &fake _1; +- _3 = &fake _1; +- _4 = &fake (*(*((_1 as Some).0: &&i32))); - _5 = &fake (*((_1 as Some).0: &&i32)); - _6 = &fake ((_1 as Some).0: &&i32); -- _7 = &fake (*(*((_1 as Some).0: &&i32))); + nop; + nop; + nop; @@ -48,10 +48,10 @@ bb5: { StorageDead(_8); +- FakeRead(ForMatchGuard, _3); - FakeRead(ForMatchGuard, _4); - FakeRead(ForMatchGuard, _5); - FakeRead(ForMatchGuard, _6); -- FakeRead(ForMatchGuard, _7); + nop; + nop; + nop; From 50531806ee4c77be601ecf2fbdac371288770e17 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 7 Apr 2024 00:30:28 +0200 Subject: [PATCH 13/22] Add a non-shallow fake borrow --- compiler/rustc_borrowck/src/borrow_set.rs | 3 +- .../src/diagnostics/conflict_errors.rs | 29 +++++--- .../rustc_borrowck/src/diagnostics/mod.rs | 2 +- compiler/rustc_borrowck/src/lib.rs | 29 ++++---- .../rustc_borrowck/src/places_conflict.rs | 6 +- .../src/polonius/loan_invalidations.rs | 19 +++-- .../src/transform/check_consts/check.rs | 4 +- .../src/transform/check_consts/resolver.rs | 2 +- .../src/transform/validate.rs | 2 +- compiler/rustc_middle/src/mir/pretty.rs | 3 +- compiler/rustc_middle/src/mir/statement.rs | 4 +- compiler/rustc_middle/src/mir/syntax.rs | 73 ++++++++++++++++--- compiler/rustc_middle/src/mir/tcx.rs | 2 +- compiler/rustc_middle/src/mir/visit.rs | 4 +- .../src/build/expr/as_place.rs | 2 +- .../rustc_mir_build/src/build/matches/mod.rs | 3 +- .../rustc_mir_build/src/check_unsafety.rs | 4 +- .../src/impls/borrowed_locals.rs | 2 +- .../src/cleanup_post_borrowck.rs | 2 +- .../rustc_mir_transform/src/promote_consts.rs | 2 +- .../rustc_smir/src/rustc_smir/convert/mir.rs | 13 +++- compiler/stable_mir/src/mir/body.rs | 21 ++++-- compiler/stable_mir/src/mir/pretty.rs | 5 +- ...se_edges.full_tested_match.built.after.mir | 2 +- ...e_edges.full_tested_match2.built.after.mir | 2 +- .../match_false_edges.main.built.after.mir | 4 +- ....constant_eq.SimplifyCfg-initial.after.mir | 4 +- ...joint_ranges.SimplifyCfg-initial.after.mir | 2 +- ...fg-initial.after-ElaborateDrops.after.diff | 8 +- ...fg-initial.after-ElaborateDrops.after.diff | 8 +- ...guard.CleanupPostBorrowck.panic-abort.diff | 8 +- ...uard.CleanupPostBorrowck.panic-unwind.diff | 8 +- 32 files changed, 189 insertions(+), 93 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index a38dd286be51b..af5f757107432 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -69,7 +69,8 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> { fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { let kind = match self.kind { mir::BorrowKind::Shared => "", - mir::BorrowKind::Fake => "fake ", + mir::BorrowKind::Fake(mir::FakeBorrowKind::Deep) => "fake ", + mir::BorrowKind::Fake(mir::FakeBorrowKind::Shallow) => "fake shallow ", mir::BorrowKind::Mut { kind: mir::MutBorrowKind::ClosureCapture } => "uniq ", // FIXME: differentiate `TwoPhaseBorrow` mir::BorrowKind::Mut { diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index ec0d4af599ea5..f6d6b486c8935 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -17,9 +17,9 @@ use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::{ self, AggregateKind, BindingForm, BorrowKind, CallSource, ClearCrossCrate, ConstraintCategory, - FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, MutBorrowKind, Operand, Place, - PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, - VarBindingForm, + FakeBorrowKind, FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, MutBorrowKind, + Operand, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, + TerminatorKind, VarBindingForm, }; use rustc_middle::ty::{ self, suggest_constraining_type_params, PredicateKind, ToPredicate, Ty, TyCtxt, @@ -1486,7 +1486,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let first_borrow_desc; let mut err = match (gen_borrow_kind, issued_borrow.kind) { ( - BorrowKind::Shared, + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow }, ) => { first_borrow_desc = "mutable "; @@ -1504,7 +1504,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } ( BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow }, - BorrowKind::Shared, + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), ) => { first_borrow_desc = "immutable "; let mut err = self.cannot_reborrow_already_borrowed( @@ -1566,7 +1566,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { self.cannot_uniquely_borrow_by_two_closures(span, &desc_place, issued_span, None) } - (BorrowKind::Mut { .. }, BorrowKind::Fake) => { + (BorrowKind::Mut { .. }, BorrowKind::Fake(FakeBorrowKind::Shallow)) => { if let Some(immutable_section_description) = self.classify_immutable_section(issued_borrow.assigned_place) { @@ -1629,7 +1629,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) } - (BorrowKind::Shared, BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture }) => { + ( + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), + BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture }, + ) => { first_borrow_desc = "first "; self.cannot_reborrow_already_uniquely_borrowed( span, @@ -1659,8 +1662,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) } - (BorrowKind::Shared, BorrowKind::Shared | BorrowKind::Fake) - | (BorrowKind::Fake, BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake) => { + ( + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), + BorrowKind::Shared | BorrowKind::Fake(_), + ) + | ( + BorrowKind::Fake(FakeBorrowKind::Shallow), + BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake(_), + ) => { unreachable!() } }; @@ -3572,7 +3581,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let loan_span = loan_spans.args_or_use(); let descr_place = self.describe_any_place(place.as_ref()); - if loan.kind == BorrowKind::Fake { + if let BorrowKind::Fake(_) = loan.kind { if let Some(section) = self.classify_immutable_section(loan.assigned_place) { let mut err = self.cannot_mutate_in_immutable_section( span, diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index dbea317e7bb26..f88b506eb3e43 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -654,7 +654,7 @@ impl UseSpans<'_> { match kind { Some(kd) => match kd { rustc_middle::mir::BorrowKind::Shared - | rustc_middle::mir::BorrowKind::Fake => { + | rustc_middle::mir::BorrowKind::Fake(_) => { CaptureVarKind::Immut { kind_span: capture_kind_span } } diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 7fbf4c47ec818..e839255c34fa0 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -1056,18 +1056,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Control::Continue } - (Read(_), BorrowKind::Shared | BorrowKind::Fake) - | (Read(ReadKind::Borrow(BorrowKind::Fake)), BorrowKind::Mut { .. }) => { - Control::Continue - } + (Read(_), BorrowKind::Shared | BorrowKind::Fake(_)) + | ( + Read(ReadKind::Borrow(BorrowKind::Fake(FakeBorrowKind::Shallow))), + BorrowKind::Mut { .. }, + ) => Control::Continue, - (Reservation(_), BorrowKind::Fake | BorrowKind::Shared) => { + (Reservation(_), BorrowKind::Fake(_) | BorrowKind::Shared) => { // This used to be a future compatibility warning (to be // disallowed on NLL). See rust-lang/rust#56254 Control::Continue } - (Write(WriteKind::Move), BorrowKind::Fake) => { + (Write(WriteKind::Move), BorrowKind::Fake(FakeBorrowKind::Shallow)) => { // Handled by initialization checks. Control::Continue } @@ -1175,10 +1176,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { match rvalue { &Rvalue::Ref(_ /*rgn*/, bk, place) => { let access_kind = match bk { - BorrowKind::Fake => { + BorrowKind::Fake(FakeBorrowKind::Shallow) => { (Shallow(Some(ArtificialField::FakeBorrow)), Read(ReadKind::Borrow(bk))) } - BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))), + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep) => { + (Deep, Read(ReadKind::Borrow(bk))) + } BorrowKind::Mut { .. } => { let wk = WriteKind::MutableBorrow(bk); if allow_two_phase_borrow(bk) { @@ -1197,7 +1200,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { flow_state, ); - let action = if bk == BorrowKind::Fake { + let action = if bk == BorrowKind::Fake(FakeBorrowKind::Shallow) { InitializationRequiringAction::MatchOn } else { InitializationRequiringAction::Borrow @@ -1556,7 +1559,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // only mutable borrows should be 2-phase assert!(match borrow.kind { - BorrowKind::Shared | BorrowKind::Fake => false, + BorrowKind::Shared | BorrowKind::Fake(_) => false, BorrowKind::Mut { .. } => true, }); @@ -2121,14 +2124,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { | WriteKind::Replace | WriteKind::StorageDeadOrDrop | WriteKind::MutableBorrow(BorrowKind::Shared) - | WriteKind::MutableBorrow(BorrowKind::Fake), + | WriteKind::MutableBorrow(BorrowKind::Fake(_)), ) | Write( WriteKind::Move | WriteKind::Replace | WriteKind::StorageDeadOrDrop | WriteKind::MutableBorrow(BorrowKind::Shared) - | WriteKind::MutableBorrow(BorrowKind::Fake), + | WriteKind::MutableBorrow(BorrowKind::Fake(_)), ) => { if self.is_mutable(place.as_ref(), is_local_mutation_allowed).is_err() && !self.has_buffered_diags() @@ -2152,7 +2155,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { return false; } Read( - ReadKind::Borrow(BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake) + ReadKind::Borrow(BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake(_)) | ReadKind::Copy, ) => { // Access authorized diff --git a/compiler/rustc_borrowck/src/places_conflict.rs b/compiler/rustc_borrowck/src/places_conflict.rs index 7e8dba43b715e..c559f9d574817 100644 --- a/compiler/rustc_borrowck/src/places_conflict.rs +++ b/compiler/rustc_borrowck/src/places_conflict.rs @@ -55,7 +55,7 @@ use crate::Overlap; use crate::{AccessDepth, Deep, Shallow}; use rustc_hir as hir; use rustc_middle::mir::{ - Body, BorrowKind, MutBorrowKind, Place, PlaceElem, PlaceRef, ProjectionElem, + Body, BorrowKind, FakeBorrowKind, MutBorrowKind, Place, PlaceElem, PlaceRef, ProjectionElem, }; use rustc_middle::ty::{self, TyCtxt}; use std::cmp::max; @@ -271,10 +271,10 @@ fn place_components_conflict<'tcx>( // If the second example, where we did, then we still know // that the borrow can access a *part* of our place that // our access cares about, so we still have a conflict. - if borrow_kind == BorrowKind::Fake + if borrow_kind == BorrowKind::Fake(FakeBorrowKind::Shallow) && borrow_place.projection.len() < access_place.projection.len() { - debug!("borrow_conflicts_with_place: fake borrow"); + debug!("borrow_conflicts_with_place: shallow borrow"); false } else { debug!("borrow_conflicts_with_place: full borrow, CONFLICT"); diff --git a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs index 956de1dec9b2b..de469080d14ec 100644 --- a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs @@ -1,6 +1,8 @@ use rustc_data_structures::graph::dominators::Dominators; use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{self, BasicBlock, Body, Location, NonDivergingIntrinsic, Place, Rvalue}; +use rustc_middle::mir::{ + self, BasicBlock, Body, FakeBorrowKind, Location, NonDivergingIntrinsic, Place, Rvalue, +}; use rustc_middle::mir::{BorrowKind, Mutability, Operand}; use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind}; use rustc_middle::mir::{Statement, StatementKind}; @@ -239,10 +241,12 @@ impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> { match rvalue { &Rvalue::Ref(_ /*rgn*/, bk, place) => { let access_kind = match bk { - BorrowKind::Fake => { + BorrowKind::Fake(FakeBorrowKind::Shallow) => { (Shallow(Some(ArtificialField::FakeBorrow)), Read(ReadKind::Borrow(bk))) } - BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))), + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep) => { + (Deep, Read(ReadKind::Borrow(bk))) + } BorrowKind::Mut { .. } => { let wk = WriteKind::MutableBorrow(bk); if allow_two_phase_borrow(bk) { @@ -357,8 +361,11 @@ impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> { // have already taken the reservation } - (Read(_), BorrowKind::Fake | BorrowKind::Shared) - | (Read(ReadKind::Borrow(BorrowKind::Fake)), BorrowKind::Mut { .. }) => { + (Read(_), BorrowKind::Fake(_) | BorrowKind::Shared) + | ( + Read(ReadKind::Borrow(BorrowKind::Fake(FakeBorrowKind::Shallow))), + BorrowKind::Mut { .. }, + ) => { // Reads don't invalidate shared or shallow borrows } @@ -403,7 +410,7 @@ impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> { // only mutable borrows should be 2-phase assert!(match borrow.kind { - BorrowKind::Shared | BorrowKind::Fake => false, + BorrowKind::Shared | BorrowKind::Fake(_) => false, BorrowKind::Mut { .. } => true, }); diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index a506d10c1d089..f9786acfc6c26 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -414,7 +414,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { BorrowKind::Shared => { PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) } - BorrowKind::Fake => { + BorrowKind::Fake(_) => { PlaceContext::NonMutatingUse(NonMutatingUseContext::FakeBorrow) } BorrowKind::Mut { .. } => { @@ -487,7 +487,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { } } - Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Fake, place) + Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Fake(_), place) | Rvalue::AddressOf(Mutability::Not, place) => { let borrowed_place_has_mut_interior = qualifs::in_place::( self.ccx, diff --git a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs index 2c835f6750f75..5ae3ffaaec2f2 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs @@ -105,7 +105,7 @@ where fn ref_allows_mutation(&self, kind: mir::BorrowKind, place: mir::Place<'tcx>) -> bool { match kind { mir::BorrowKind::Mut { .. } => true, - mir::BorrowKind::Shared | mir::BorrowKind::Fake => { + mir::BorrowKind::Shared | mir::BorrowKind::Fake(_) => { self.shared_borrow_allows_mutation(place) } } diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index a499e4b980fc3..69274f962cc3a 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -924,7 +924,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } }, - Rvalue::Ref(_, BorrowKind::Fake, _) => { + Rvalue::Ref(_, BorrowKind::Fake(_), _) => { if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 15bd5c089652c..fead4a16dab28 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -954,7 +954,8 @@ impl<'tcx> Debug for Rvalue<'tcx> { Ref(region, borrow_kind, ref place) => { let kind_str = match borrow_kind { BorrowKind::Shared => "", - BorrowKind::Fake => "fake ", + BorrowKind::Fake(FakeBorrowKind::Deep) => "fake ", + BorrowKind::Fake(FakeBorrowKind::Shallow) => "fake shallow ", BorrowKind::Mut { .. } => "mut ", }; diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs index 235298ffcb8bc..375f1f15a39ed 100644 --- a/compiler/rustc_middle/src/mir/statement.rs +++ b/compiler/rustc_middle/src/mir/statement.rs @@ -451,7 +451,7 @@ impl<'tcx> Rvalue<'tcx> { impl BorrowKind { pub fn mutability(&self) -> Mutability { match *self { - BorrowKind::Shared | BorrowKind::Fake => Mutability::Not, + BorrowKind::Shared | BorrowKind::Fake(_) => Mutability::Not, BorrowKind::Mut { .. } => Mutability::Mut, } } @@ -459,7 +459,7 @@ impl BorrowKind { pub fn allows_two_phase_borrow(&self) -> bool { match *self { BorrowKind::Shared - | BorrowKind::Fake + | BorrowKind::Fake(_) | BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::ClosureCapture } => { false } diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 97c3eb5563861..6292633f65c51 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -165,13 +165,16 @@ pub enum BorrowKind { /// Data must be immutable and is aliasable. Shared, - /// The immediately borrowed place must be immutable, but projections from - /// it don't need to be. For example, a shallow borrow of `a.b` doesn't - /// conflict with a mutable borrow of `a.b.c`. - /// - /// This is used when lowering matches: when matching on a place we want to - /// ensure that place have the same value from the start of the match until - /// an arm is selected. This prevents this code from compiling: + /// An immutable, aliasable borrow that is discarded after borrow-checking. Can behave either + /// like a normal shared borrow or like a special shallow borrow (see [`FakeBorrowKind`]). + /// + /// This is used when lowering index expressions and matches. This is used to prevent code like + /// the following from compiling: + /// ```compile_fail,E0506 + /// let mut x = vec![vec![0, 1]]; + /// let y = vec![]; + /// let _ = x[0][{x = y; 1}]; + /// ``` /// ```compile_fail,E0510 /// let mut x = &Some(0); /// match *x { @@ -180,11 +183,8 @@ pub enum BorrowKind { /// Some(_) => (), /// } /// ``` - /// This can't be a shared borrow because mutably borrowing (*x as Some).0 - /// should not prevent `if let None = x { ... }`, for example, because the - /// mutating `(*x as Some).0` can't affect the discriminant of `x`. /// We can also report errors with this kind of borrow differently. - Fake, + Fake(FakeBorrowKind), /// Data is mutable and not aliasable. Mut { kind: MutBorrowKind }, @@ -240,6 +240,57 @@ pub enum MutBorrowKind { ClosureCapture, } +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, TyEncodable, TyDecodable)] +#[derive(Hash, HashStable)] +pub enum FakeBorrowKind { + /// A shared shallow borrow. The immediately borrowed place must be immutable, but projections + /// from it don't need to be. For example, a shallow borrow of `a.b` doesn't conflict with a + /// mutable borrow of `a.b.c`. + /// + /// This is used when lowering matches: when matching on a place we want to ensure that place + /// have the same value from the start of the match until an arm is selected. This prevents this + /// code from compiling: + /// ```compile_fail,E0510 + /// let mut x = &Some(0); + /// match *x { + /// None => (), + /// Some(_) if { x = &None; false } => (), + /// Some(_) => (), + /// } + /// ``` + /// This can't be a shared borrow because mutably borrowing `(*x as Some).0` should not checking + /// the discriminant or accessing other variants, because the mutating `(*x as Some).0` can't + /// affect the discriminant of `x`. E.g. the following is allowed: + /// ```rust + /// let mut x = Some(0); + /// match x { + /// Some(_) + /// if { + /// if let Some(ref mut y) = x { + /// *y += 1; + /// }; + /// true + /// } => {} + /// _ => {} + /// } + /// ``` + Shallow, + /// A shared (deep) borrow. Data must be immutable and is aliasable. + /// + /// This is used when lowering deref patterns, where shallow borrows wouldn't prevent something + /// like: + // ```compile_fail + // let mut b = Box::new(false); + // match b { + // deref!(true) => {} // not reached because `*b == false` + // _ if { *b = true; false } => {} // not reached because the guard is `false` + // deref!(false) => {} // not reached because the guard changed it + // // UB because we reached the unreachable. + // } + // ``` + Deep, +} + /////////////////////////////////////////////////////////////////////////// // Statements diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index 506003ff7c0cc..4c2bf9abe975d 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -293,7 +293,7 @@ impl BorrowKind { // We have no type corresponding to a shallow borrow, so use // `&` as an approximation. - BorrowKind::Fake => hir::Mutability::Not, + BorrowKind::Fake(_) => hir::Mutability::Not, } } } diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 4f7b2f7cbe48b..0f97bcded3ef1 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -653,7 +653,7 @@ macro_rules! make_mir_visitor { BorrowKind::Shared => PlaceContext::NonMutatingUse( NonMutatingUseContext::SharedBorrow ), - BorrowKind::Fake => PlaceContext::NonMutatingUse( + BorrowKind::Fake(_) => PlaceContext::NonMutatingUse( NonMutatingUseContext::FakeBorrow ), BorrowKind::Mut { .. } => @@ -1279,6 +1279,8 @@ pub enum NonMutatingUseContext { /// Shared borrow. SharedBorrow, /// A fake borrow. + /// FIXME: do we need to distinguish shallow and deep fake borrows? In fact, do we need to + /// distinguish fake and normal deep borrows? FakeBorrow, /// AddressOf for *const pointer. AddressOf, diff --git a/compiler/rustc_mir_build/src/build/expr/as_place.rs b/compiler/rustc_mir_build/src/build/expr/as_place.rs index f12e25db6fcd5..060b328ef48ae 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -685,7 +685,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fake_borrow_temp.into(), Rvalue::Ref( tcx.lifetimes.re_erased, - BorrowKind::Fake, + BorrowKind::Fake(FakeBorrowKind::Shallow), Place { local: base_place.local, projection }, ), ); diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 648d0b52b3214..b41523cb9c25f 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -2106,7 +2106,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let re_erased = tcx.lifetimes.re_erased; let scrutinee_source_info = self.source_info(scrutinee_span); for &(place, temp) in fake_borrows { - let borrow = Rvalue::Ref(re_erased, BorrowKind::Fake, place); + let borrow = + Rvalue::Ref(re_erased, BorrowKind::Fake(FakeBorrowKind::Shallow), place); self.cfg.push_assign(block, scrutinee_source_info, Place::from(temp), borrow); } diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 0c1e1d59c4f3f..227d19c3e43c5 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -513,7 +513,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { visit::walk_expr(&mut visitor, expr); if visitor.found { match borrow_kind { - BorrowKind::Fake | BorrowKind::Shared + BorrowKind::Fake(_) | BorrowKind::Shared if !self.thir[arg].ty.is_freeze(self.tcx, self.param_env) => { self.requires_unsafe(expr.span, BorrowOfLayoutConstrainedField) @@ -521,7 +521,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { BorrowKind::Mut { .. } => { self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField) } - BorrowKind::Fake | BorrowKind::Shared => {} + BorrowKind::Fake(_) | BorrowKind::Shared => {} } } } diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index 693994b5da76f..bdc70de58e84b 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -102,7 +102,7 @@ where } Rvalue::Cast(..) - | Rvalue::Ref(_, BorrowKind::Fake, _) + | Rvalue::Ref(_, BorrowKind::Fake(_), _) | Rvalue::ShallowInitBox(..) | Rvalue::Use(..) | Rvalue::ThreadLocalRef(..) diff --git a/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs b/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs index da82f8de78147..48a6a83e14609 100644 --- a/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs +++ b/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs @@ -29,7 +29,7 @@ impl<'tcx> MirPass<'tcx> for CleanupPostBorrowck { for statement in basic_block.statements.iter_mut() { match statement.kind { StatementKind::AscribeUserType(..) - | StatementKind::Assign(box (_, Rvalue::Ref(_, BorrowKind::Fake, _))) + | StatementKind::Assign(box (_, Rvalue::Ref(_, BorrowKind::Fake(_), _))) | StatementKind::Coverage( // These kinds of coverage statements are markers inserted during // MIR building, and are not needed after InstrumentCoverage. diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index 1f4af0ec63dd9..c14d4ceb09106 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -384,7 +384,7 @@ impl<'tcx> Validator<'_, 'tcx> { match kind { // Reject these borrow types just to be safe. // FIXME(RalfJung): could we allow them? Should we? No point in it until we have a usecase. - BorrowKind::Fake | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => { + BorrowKind::Fake(_) | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => { return Err(Unpromotable); } diff --git a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs index c9f6661259022..a701dfe7046d0 100644 --- a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs +++ b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs @@ -229,7 +229,7 @@ impl<'tcx> Stable<'tcx> for mir::BorrowKind { use rustc_middle::mir::BorrowKind::*; match *self { Shared => stable_mir::mir::BorrowKind::Shared, - Fake => stable_mir::mir::BorrowKind::Fake, + Fake(kind) => stable_mir::mir::BorrowKind::Fake(kind.stable(tables)), Mut { kind } => stable_mir::mir::BorrowKind::Mut { kind: kind.stable(tables) }, } } @@ -247,6 +247,17 @@ impl<'tcx> Stable<'tcx> for mir::MutBorrowKind { } } +impl<'tcx> Stable<'tcx> for mir::FakeBorrowKind { + type T = stable_mir::mir::FakeBorrowKind; + fn stable(&self, _: &mut Tables<'_>) -> Self::T { + use rustc_middle::mir::FakeBorrowKind::*; + match *self { + Deep => stable_mir::mir::FakeBorrowKind::Deep, + Shallow => stable_mir::mir::FakeBorrowKind::Shallow, + } + } +} + impl<'tcx> Stable<'tcx> for mir::NullOp<'tcx> { type T = stable_mir::mir::NullOp; fn stable(&self, tables: &mut Tables<'_>) -> Self::T { diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 1ad05633d62dc..e928a6f3dcdba 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -865,11 +865,9 @@ pub enum BorrowKind { /// Data must be immutable and is aliasable. Shared, - /// The immediately borrowed place must be immutable, but projections from - /// it don't need to be. This is used to prevent match guards from replacing - /// the scrutinee. For example, a fake borrow of `a.b` doesn't - /// conflict with a mutable borrow of `a.b.c`. - Fake, + /// An immutable, aliasable borrow that is discarded after borrow-checking. Can behave either + /// like a normal shared borrow or like a special shallow borrow (see [`FakeBorrowKind`]). + Fake(FakeBorrowKind), /// Data is mutable and not aliasable. Mut { @@ -884,7 +882,7 @@ impl BorrowKind { BorrowKind::Mut { .. } => Mutability::Mut, BorrowKind::Shared => Mutability::Not, // FIXME: There's no type corresponding to a shallow borrow, so use `&` as an approximation. - BorrowKind::Fake => Mutability::Not, + BorrowKind::Fake(_) => Mutability::Not, } } } @@ -896,6 +894,17 @@ pub enum MutBorrowKind { ClosureCapture, } +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum FakeBorrowKind { + /// A shared (deep) borrow. Data must be immutable and is aliasable. + Deep, + /// The immediately borrowed place must be immutable, but projections from + /// it don't need to be. This is used to prevent match guards from replacing + /// the scrutinee. For example, a fake borrow of `a.b` doesn't + /// conflict with a mutable borrow of `a.b.c`. + Shallow, +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum Mutability { Not, diff --git a/compiler/stable_mir/src/mir/pretty.rs b/compiler/stable_mir/src/mir/pretty.rs index 4ac4833add715..bbca3965852a6 100644 --- a/compiler/stable_mir/src/mir/pretty.rs +++ b/compiler/stable_mir/src/mir/pretty.rs @@ -8,7 +8,7 @@ use std::{fmt, io, iter}; use super::{AssertMessage, BinOp, TerminatorKind}; -use super::BorrowKind; +use super::{BorrowKind, FakeBorrowKind}; impl Display for Ty { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -352,7 +352,8 @@ fn pretty_rvalue(writer: &mut W, rval: &Rvalue) -> io::Result<()> { Rvalue::Ref(_, borrowkind, place) => { let kind = match borrowkind { BorrowKind::Shared => "&", - BorrowKind::Fake => "&fake ", + BorrowKind::Fake(FakeBorrowKind::Deep) => "&fake ", + BorrowKind::Fake(FakeBorrowKind::Shallow) => "&fake shallow ", BorrowKind::Mut { .. } => "&mut ", }; write!(writer, "{kind}{:?}", place) diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir index 0d381ce70f175..bade0fa4b45c8 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir @@ -60,7 +60,7 @@ fn full_tested_match() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir index d397de66fc470..0d78bb8b23587 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir @@ -66,7 +66,7 @@ fn full_tested_match2() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } diff --git a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir index 63920cec885ad..ebb75ae141a33 100644 --- a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir @@ -87,7 +87,7 @@ fn main() -> () { bb10: { StorageLive(_7); _7 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_8); _8 = guard() -> [return: bb11, unwind: bb24]; } @@ -129,7 +129,7 @@ fn main() -> () { bb16: { StorageLive(_11); _11 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_12); StorageLive(_13); _13 = (*_11); diff --git a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir index 21ddd39137fb2..060cd6132e3a0 100644 --- a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir @@ -72,8 +72,8 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { } bb12: { - _6 = &fake (_3.0: &str); - _7 = &fake (_3.1: bool); + _6 = &fake shallow (_3.0: &str); + _7 = &fake shallow (_3.1: bool); StorageLive(_10); _10 = const true; switchInt(move _10) -> [0: bb14, otherwise: bb13]; diff --git a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir index 6068cef8fbcb0..07daa3eddfa2c 100644 --- a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir @@ -54,7 +54,7 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { } bb9: { - _3 = &fake _1; + _3 = &fake shallow _1; StorageLive(_8); _8 = _2; switchInt(move _8) -> [0: bb11, otherwise: bb10]; diff --git a/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff b/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff index ba333ba1a5866..209f0d09c2943 100644 --- a/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff +++ b/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff @@ -80,8 +80,8 @@ _6 = &(_2.1: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_9); StorageLive(_10); _10 = _1; @@ -137,8 +137,8 @@ _6 = &(_2.0: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_12); StorageLive(_13); _13 = _1; diff --git a/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff b/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff index ba333ba1a5866..209f0d09c2943 100644 --- a/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff +++ b/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff @@ -80,8 +80,8 @@ _6 = &(_2.1: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_9); StorageLive(_10); _10 = _1; @@ -137,8 +137,8 @@ _6 = &(_2.0: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_12); StorageLive(_13); _13 = _1; diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff index 8feb59ec8747b..d76d65a18a7b4 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff @@ -33,10 +33,10 @@ } bb4: { -- _3 = &fake _1; -- _4 = &fake (*(*((_1 as Some).0: &&i32))); -- _5 = &fake (*((_1 as Some).0: &&i32)); -- _6 = &fake ((_1 as Some).0: &&i32); +- _3 = &fake shallow _1; +- _4 = &fake shallow (*(*((_1 as Some).0: &&i32))); +- _5 = &fake shallow (*((_1 as Some).0: &&i32)); +- _6 = &fake shallow ((_1 as Some).0: &&i32); + nop; + nop; + nop; diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff index 8feb59ec8747b..d76d65a18a7b4 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff @@ -33,10 +33,10 @@ } bb4: { -- _3 = &fake _1; -- _4 = &fake (*(*((_1 as Some).0: &&i32))); -- _5 = &fake (*((_1 as Some).0: &&i32)); -- _6 = &fake ((_1 as Some).0: &&i32); +- _3 = &fake shallow _1; +- _4 = &fake shallow (*(*((_1 as Some).0: &&i32))); +- _5 = &fake shallow (*((_1 as Some).0: &&i32)); +- _6 = &fake shallow ((_1 as Some).0: &&i32); + nop; + nop; + nop; From 436c61266c7e9c4d6b7a57a8a6170700eda68fd0 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 6 Apr 2024 00:29:45 +0200 Subject: [PATCH 14/22] Use deep fake borrows for deref patterns --- .../rustc_mir_build/src/build/matches/mod.rs | 17 ++++---- .../rustc_mir_build/src/build/matches/util.rs | 41 ++++++++++--------- .../ui/pattern/deref-patterns/fake_borrows.rs | 14 +++++++ .../deref-patterns/fake_borrows.stderr | 12 ++++++ 4 files changed, 56 insertions(+), 28 deletions(-) create mode 100644 tests/ui/pattern/deref-patterns/fake_borrows.rs create mode 100644 tests/ui/pattern/deref-patterns/fake_borrows.stderr diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index b41523cb9c25f..2ab297a47ae02 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -375,10 +375,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match_start_span: Span, match_has_guard: bool, candidates: &mut [&mut Candidate<'pat, 'tcx>], - ) -> Vec<(Place<'tcx>, Local)> { + ) -> Vec<(Place<'tcx>, Local, FakeBorrowKind)> { // The set of places that we are creating fake borrows of. If there are no match guards then // we don't need any fake borrows, so don't track them. - let fake_borrows: Vec<(Place<'tcx>, Local)> = if match_has_guard { + let fake_borrows: Vec<(Place<'tcx>, Local, FakeBorrowKind)> = if match_has_guard { util::collect_fake_borrows( self, candidates, @@ -457,7 +457,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scrutinee_span: Span, arm_candidates: Vec<(&'_ Arm<'tcx>, Candidate<'_, 'tcx>)>, outer_source_info: SourceInfo, - fake_borrow_temps: Vec<(Place<'tcx>, Local)>, + fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>, ) -> BlockAnd<()> { let arm_end_blocks: Vec<_> = arm_candidates .into_iter() @@ -541,7 +541,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &mut self, outer_source_info: SourceInfo, candidate: Candidate<'_, 'tcx>, - fake_borrow_temps: &[(Place<'tcx>, Local)], + fake_borrow_temps: &[(Place<'tcx>, Local, FakeBorrowKind)], scrutinee_span: Span, arm_match_scope: Option<(&Arm<'tcx>, region::Scope)>, storages_alive: bool, @@ -1975,7 +1975,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &mut self, candidate: Candidate<'pat, 'tcx>, parent_data: &[PatternExtraData<'tcx>], - fake_borrows: &[(Place<'tcx>, Local)], + fake_borrows: &[(Place<'tcx>, Local, FakeBorrowKind)], scrutinee_span: Span, arm_match_scope: Option<(&Arm<'tcx>, region::Scope)>, schedule_drops: bool, @@ -2105,9 +2105,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let re_erased = tcx.lifetimes.re_erased; let scrutinee_source_info = self.source_info(scrutinee_span); - for &(place, temp) in fake_borrows { - let borrow = - Rvalue::Ref(re_erased, BorrowKind::Fake(FakeBorrowKind::Shallow), place); + for &(place, temp, kind) in fake_borrows { + let borrow = Rvalue::Ref(re_erased, BorrowKind::Fake(kind), place); self.cfg.push_assign(block, scrutinee_source_info, Place::from(temp), borrow); } @@ -2130,7 +2129,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let guard_frame = self.guard_context.pop().unwrap(); debug!("Exiting guard building context with locals: {:?}", guard_frame); - for &(_, temp) in fake_borrows { + for &(_, temp, _) in fake_borrows { let cause = FakeReadCause::ForMatchGuard; self.cfg.push_fake_read(post_guard_block, guard_end, cause, Place::from(temp)); } diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 967b0c44588fa..2f9390c22a89d 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -1,7 +1,7 @@ use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; use crate::build::matches::{Binding, Candidate, FlatPat, MatchPair, TestCase}; use crate::build::Builder; -use rustc_data_structures::fx::FxIndexSet; +use rustc_data_structures::fx::FxIndexMap; use rustc_infer::infer::type_variable::TypeVariableOrigin; use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; @@ -271,7 +271,11 @@ pub(super) struct FakeBorrowCollector<'a, 'b, 'tcx> { /// Base of the scrutinee place. Used to distinguish bindings inside the scrutinee place from /// bindings inside deref patterns. scrutinee_base: PlaceBase, - fake_borrows: FxIndexSet>, + /// Store for each place the kind of borrow to take. In case of conflicts, we take the strongest + /// borrow (i.e. Deep > Shallow). + /// Invariant: for any place in `fake_borrows`, all the prefixes of this place that are + /// dereferences are also borrowed with the same of stronger borrow kind. + fake_borrows: FxIndexMap, FakeBorrowKind>, } /// Determine the set of places that have to be stable across match guards. @@ -314,9 +318,9 @@ pub(super) fn collect_fake_borrows<'tcx>( candidates: &[&mut Candidate<'_, 'tcx>], temp_span: Span, scrutinee_base: PlaceBase, -) -> Vec<(Place<'tcx>, Local)> { +) -> Vec<(Place<'tcx>, Local, FakeBorrowKind)> { let mut collector = - FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexSet::default() }; + FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexMap::default() }; for candidate in candidates.iter() { collector.visit_candidate(candidate); } @@ -325,40 +329,40 @@ pub(super) fn collect_fake_borrows<'tcx>( let tcx = cx.tcx; fake_borrows .iter() - .copied() - .map(|matched_place| { + .map(|(matched_place, borrow_kind)| { let fake_borrow_deref_ty = matched_place.ty(&cx.local_decls, tcx).ty; let fake_borrow_ty = Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, fake_borrow_deref_ty); let mut fake_borrow_temp = LocalDecl::new(fake_borrow_ty, temp_span); fake_borrow_temp.local_info = ClearCrossCrate::Set(Box::new(LocalInfo::FakeBorrow)); let fake_borrow_temp = cx.local_decls.push(fake_borrow_temp); - (matched_place, fake_borrow_temp) + (*matched_place, fake_borrow_temp, *borrow_kind) }) .collect() } impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { // Fake borrow this place and its dereference prefixes. - fn fake_borrow(&mut self, place: Place<'tcx>) { - let new = self.fake_borrows.insert(place); - if !new { + fn fake_borrow(&mut self, place: Place<'tcx>, kind: FakeBorrowKind) { + if self.fake_borrows.get(&place).is_some_and(|k| *k >= kind) { return; } + self.fake_borrows.insert(place, kind); // Also fake borrow the prefixes of any fake borrow. - self.fake_borrow_deref_prefixes(place); + self.fake_borrow_deref_prefixes(place, kind); } // Fake borrow the prefixes of this place that are dereferences. - fn fake_borrow_deref_prefixes(&mut self, place: Place<'tcx>) { + fn fake_borrow_deref_prefixes(&mut self, place: Place<'tcx>, kind: FakeBorrowKind) { for (place_ref, elem) in place.as_ref().iter_projections().rev() { if let ProjectionElem::Deref = elem { // Insert a shallow borrow after a deref. For other projections the borrow of // `place_ref` will conflict with any mutation of `place.base`. - let new = self.fake_borrows.insert(place_ref.to_place(self.cx.tcx)); - if !new { + let place = place_ref.to_place(self.cx.tcx); + if self.fake_borrows.get(&place).is_some_and(|k| *k >= kind) { return; } + self.fake_borrows.insert(place, kind); } } } @@ -399,15 +403,14 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { // // UB because we reached the unreachable. // } // ``` - // FIXME(deref_patterns): Hence we fake borrow using a non-shallow borrow. + // Hence we fake borrow using a deep borrow. if let Some(place) = match_pair.place { - // FIXME(deref_patterns): use a non-shallow borrow. - self.fake_borrow(place); + self.fake_borrow(place, FakeBorrowKind::Deep); } } else { // Insert a Shallow borrow of any place that is switched on. if let Some(place) = match_pair.place { - self.fake_borrow(place); + self.fake_borrow(place, FakeBorrowKind::Shallow); } for subpair in &match_pair.subpairs { @@ -447,7 +450,7 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { // _ if { u = true; false } => (), // x => (), // } - self.fake_borrow_deref_prefixes(*source); + self.fake_borrow_deref_prefixes(*source, FakeBorrowKind::Shallow); } } diff --git a/tests/ui/pattern/deref-patterns/fake_borrows.rs b/tests/ui/pattern/deref-patterns/fake_borrows.rs new file mode 100644 index 0000000000000..35fa9cbf7d859 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/fake_borrows.rs @@ -0,0 +1,14 @@ +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +#[rustfmt::skip] +fn main() { + let mut b = Box::new(false); + match b { + deref!(true) => {} + _ if { *b = true; false } => {} + //~^ ERROR cannot assign `*b` in match guard + deref!(false) => {} + _ => {}, + } +} diff --git a/tests/ui/pattern/deref-patterns/fake_borrows.stderr b/tests/ui/pattern/deref-patterns/fake_borrows.stderr new file mode 100644 index 0000000000000..6a591e6416c5d --- /dev/null +++ b/tests/ui/pattern/deref-patterns/fake_borrows.stderr @@ -0,0 +1,12 @@ +error[E0510]: cannot assign `*b` in match guard + --> $DIR/fake_borrows.rs:9:16 + | +LL | match b { + | - value is immutable in match guard +LL | deref!(true) => {} +LL | _ if { *b = true; false } => {} + | ^^^^^^^^^ cannot assign + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0510`. From 217a4dff7da0e4be2c22fd8f822cb543745c6727 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 8 Apr 2024 20:05:23 +0200 Subject: [PATCH 15/22] Test or-patterns inside deref patterns --- tests/ui/pattern/deref-patterns/bindings.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index 4f72058af8fe5..5881e4166a46f 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -37,6 +37,17 @@ fn ref_mut(val: u32) -> u32 { *x } +#[rustfmt::skip] +fn or_and_guard(tuple: (u32, u32)) -> u32 { + let mut sum = 0; + let b = Box::new(tuple); + match b { + deref!((x, _) | (_, x)) if { sum += x; false } => {}, + _ => {}, + } + sum +} + fn main() { assert_eq!(simple_vec(vec![1]), 1); assert_eq!(simple_vec(vec![1, 2]), 202); @@ -48,5 +59,6 @@ fn main() { assert_eq!(nested_vec(vec![vec![1, 2, 3]]), 6); assert_eq!(nested_vec(vec![vec![], vec![1, 2, 3]]), 1); - assert_eq!(ref_mut(42), 42) + assert_eq!(ref_mut(42), 42); + assert_eq!(or_and_guard((10, 32)), 42); } From 726fb55ae29a778a698e1c880aa0ae46920ece88 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 20 Apr 2024 16:07:27 +0200 Subject: [PATCH 16/22] Fix documentation of `BorrowKind::Fake` --- compiler/rustc_middle/src/mir/syntax.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 6292633f65c51..e07b6c6bd81f9 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -170,9 +170,9 @@ pub enum BorrowKind { /// /// This is used when lowering index expressions and matches. This is used to prevent code like /// the following from compiling: - /// ```compile_fail,E0506 - /// let mut x = vec![vec![0, 1]]; - /// let y = vec![]; + /// ```compile_fail,E0510 + /// let mut x: &[_] = &[[0, 1]]; + /// let y: &[_] = &[]; /// let _ = x[0][{x = y; 1}]; /// ``` /// ```compile_fail,E0510 From 5646b65cf58123679fdf4ef489f690d1b21e1436 Mon Sep 17 00:00:00 2001 From: Xiretza Date: Thu, 18 Apr 2024 19:18:35 +0000 Subject: [PATCH 17/22] Pass translation closure to add_to_diag_with() as reference --- compiler/rustc_ast_lowering/src/errors.rs | 2 +- compiler/rustc_ast_passes/src/errors.rs | 4 ++-- compiler/rustc_builtin_macros/src/errors.rs | 2 +- compiler/rustc_errors/src/diagnostic.rs | 6 +++--- compiler/rustc_errors/src/diagnostic_impls.rs | 2 +- compiler/rustc_hir_typeck/src/errors.rs | 6 +++--- compiler/rustc_infer/src/errors/mod.rs | 18 +++++++++--------- .../rustc_infer/src/errors/note_and_explain.rs | 2 +- compiler/rustc_lint/src/errors.rs | 2 +- compiler/rustc_lint/src/lints.rs | 18 +++++++++--------- .../src/diagnostics/subdiagnostic.rs | 2 +- compiler/rustc_mir_build/src/errors.rs | 4 ++-- compiler/rustc_parse/src/errors.rs | 2 +- compiler/rustc_passes/src/errors.rs | 2 +- compiler/rustc_pattern_analysis/src/errors.rs | 4 ++-- compiler/rustc_trait_selection/src/errors.rs | 2 +- .../ui-fulldeps/internal-lints/diagnostics.rs | 4 ++-- 17 files changed, 41 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/errors.rs b/compiler/rustc_ast_lowering/src/errors.rs index ca0821e2c9edb..6799513a323c4 100644 --- a/compiler/rustc_ast_lowering/src/errors.rs +++ b/compiler/rustc_ast_lowering/src/errors.rs @@ -44,7 +44,7 @@ impl Subdiagnostic for InvalidAbiReason { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _: F, + _: &F, ) { #[allow(rustc::untranslatable_diagnostic)] diag.note(self.0); diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index f397c949e0486..25a125f83935b 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -382,7 +382,7 @@ impl Subdiagnostic for EmptyLabelManySpans { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _: F, + _: &F, ) { diag.span_labels(self.0, ""); } @@ -751,7 +751,7 @@ impl Subdiagnostic for StableFeature { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _: F, + _: &F, ) { diag.arg("name", self.name); diag.arg("since", self.since); diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 6b6647ef085c2..9078dc07a3152 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -601,7 +601,7 @@ impl Subdiagnostic for FormatUnusedArg { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { diag.arg("named", self.named); let msg = f(diag, crate::fluent_generated::builtin_macros_format_unused_arg.into()); diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index a17e0da47a5cf..6c84eeaf888d7 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -177,7 +177,7 @@ where { /// Add a subdiagnostic to an existing diagnostic. fn add_to_diag(self, diag: &mut Diag<'_, G>) { - self.add_to_diag_with(diag, |_, m| m); + self.add_to_diag_with(diag, &|_, m| m); } /// Add a subdiagnostic to an existing diagnostic where `f` is invoked on every message used @@ -185,7 +185,7 @@ where fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ); } @@ -1197,7 +1197,7 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { dcx: &crate::DiagCtxt, subdiagnostic: impl Subdiagnostic, ) -> &mut Self { - subdiagnostic.add_to_diag_with(self, |diag, msg| { + subdiagnostic.add_to_diag_with(self, &|diag, msg| { let args = diag.args.iter(); let msg = diag.subdiagnostic_message_to_diagnostic_message(msg); dcx.eagerly_translate(msg, args) diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 6c0551848d688..1ef945985be0a 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -302,7 +302,7 @@ impl Subdiagnostic for SingleLabelManySpans { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _: F, + _: &F, ) { diag.span_labels(self.spans, self.label); } diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index 3dc9c7b86f713..1c4d5657b171e 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -191,7 +191,7 @@ impl Subdiagnostic for TypeMismatchFruTypo { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { diag.arg("expr", self.expr.as_deref().unwrap_or("NONE")); @@ -370,7 +370,7 @@ impl Subdiagnostic for RemoveSemiForCoerce { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { let mut multispan: MultiSpan = self.semi.into(); multispan.push_span_label(self.expr, fluent::hir_typeck_remove_semi_for_coerce_expr); @@ -546,7 +546,7 @@ impl rustc_errors::Subdiagnostic for CastUnknownPointerSub { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { match self { CastUnknownPointerSub::To(span) => { diff --git a/compiler/rustc_infer/src/errors/mod.rs b/compiler/rustc_infer/src/errors/mod.rs index 4593108edac03..2acaeac24398d 100644 --- a/compiler/rustc_infer/src/errors/mod.rs +++ b/compiler/rustc_infer/src/errors/mod.rs @@ -239,7 +239,7 @@ impl Subdiagnostic for RegionOriginNote<'_> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { let mut label_or_note = |span, msg: DiagMessage| { let sub_count = diag.children.iter().filter(|d| d.span.is_dummy()).count(); @@ -304,7 +304,7 @@ impl Subdiagnostic for LifetimeMismatchLabels { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { match self { LifetimeMismatchLabels::InRet { param_span, ret_span, span, label_var1 } => { @@ -352,7 +352,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { let mut mk_suggestion = || { let ( @@ -454,7 +454,7 @@ impl Subdiagnostic for IntroducesStaticBecauseUnmetLifetimeReq { fn add_to_diag_with>( mut self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { self.unmet_requirements .push_span_label(self.binding_span, fluent::infer_msl_introduces_static); @@ -773,7 +773,7 @@ impl Subdiagnostic for ConsiderBorrowingParamHelp { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { let mut type_param_span: MultiSpan = self.spans.clone().into(); for &span in &self.spans { @@ -818,7 +818,7 @@ impl Subdiagnostic for DynTraitConstraintSuggestion { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { let mut multi_span: MultiSpan = vec![self.span].into(); multi_span.push_span_label(self.span, fluent::infer_dtcs_has_lifetime_req_label); @@ -865,7 +865,7 @@ impl Subdiagnostic for ReqIntroducedLocations { fn add_to_diag_with>( mut self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { for sp in self.spans { self.span.push_span_label(sp, fluent::infer_ril_introduced_here); @@ -888,7 +888,7 @@ impl Subdiagnostic for MoreTargeted { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { diag.code(E0772); diag.primary_message(fluent::infer_more_targeted); @@ -1293,7 +1293,7 @@ impl Subdiagnostic for SuggestTuplePatternMany { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { diag.arg("path", self.path); let message = f(diag, crate::fluent_generated::infer_stp_wrap_many.into()); diff --git a/compiler/rustc_infer/src/errors/note_and_explain.rs b/compiler/rustc_infer/src/errors/note_and_explain.rs index 7b962b0140808..70d94873530e7 100644 --- a/compiler/rustc_infer/src/errors/note_and_explain.rs +++ b/compiler/rustc_infer/src/errors/note_and_explain.rs @@ -163,7 +163,7 @@ impl Subdiagnostic for RegionExplanation<'_> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { diag.arg("pref_kind", self.prefix); diag.arg("suff_kind", self.suffix); diff --git a/compiler/rustc_lint/src/errors.rs b/compiler/rustc_lint/src/errors.rs index ee99e824a548b..c23d1221bc866 100644 --- a/compiler/rustc_lint/src/errors.rs +++ b/compiler/rustc_lint/src/errors.rs @@ -27,7 +27,7 @@ impl Subdiagnostic for OverruledAttributeSub { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { match self { OverruledAttributeSub::DefaultSource { id } => { diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index a034bebc85ec0..74a0a224dba4c 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -274,7 +274,7 @@ impl<'a, 'b> Subdiagnostic for SuggestChangingAssocTypes<'a, 'b> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { // Access to associates types should use `::Assoc`, which does not need a // bound. Let's see if this type does that. @@ -330,7 +330,7 @@ impl Subdiagnostic for BuiltinTypeAliasGenericBoundsSuggestion { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { diag.multipart_suggestion( fluent::lint_suggestion, @@ -451,7 +451,7 @@ impl Subdiagnostic for BuiltinUnpermittedTypeInitSub { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { let mut err = self.err; loop { @@ -506,7 +506,7 @@ impl Subdiagnostic for BuiltinClashingExternSub<'_> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { let mut expected_str = DiagStyledString::new(); expected_str.push(self.expected.fn_sig(self.tcx).to_string(), false); @@ -788,7 +788,7 @@ impl Subdiagnostic for HiddenUnicodeCodepointsDiagLabels { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { for (c, span) in self.spans { diag.span_label(span, format!("{c:?}")); @@ -806,7 +806,7 @@ impl Subdiagnostic for HiddenUnicodeCodepointsDiagSub { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { match self { HiddenUnicodeCodepointsDiagSub::Escape { spans } => { @@ -954,7 +954,7 @@ impl Subdiagnostic for NonBindingLetSub { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { let can_suggest_binding = self.drop_fn_start_end.is_some() || !self.is_assign_desugar; @@ -1240,7 +1240,7 @@ impl Subdiagnostic for NonSnakeCaseDiagSub { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { match self { NonSnakeCaseDiagSub::Label { span } => { @@ -1482,7 +1482,7 @@ impl Subdiagnostic for OverflowingBinHexSign { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { match self { OverflowingBinHexSign::Positive => { diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index ced782cdbc016..9491426688444 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -90,7 +90,7 @@ impl SubdiagnosticDerive { fn add_to_diag_with<__G, __F>( self, #diag: &mut rustc_errors::Diag<'_, __G>, - #f: __F + #f: &__F ) where __G: rustc_errors::EmissionGuarantee, __F: rustc_errors::SubdiagMessageOp<__G>, diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 9ddfb12bf7643..f67113afd6d9d 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -423,7 +423,7 @@ impl Subdiagnostic for UnsafeNotInheritedLintNote { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { diag.span_note(self.signature_span, fluent::mir_build_unsafe_fn_safe_body); let body_start = self.body_span.shrink_to_lo(); @@ -871,7 +871,7 @@ impl<'tcx> Subdiagnostic for AdtDefinedHere<'tcx> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { diag.arg("ty", self.ty); let mut spans = MultiSpan::from(self.adt_def_span); diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index eae2d904c35e1..a5d18a919c4a5 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -1463,7 +1463,7 @@ impl Subdiagnostic for FnTraitMissingParen { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _: F, + _: &F, ) { diag.span_label(self.span, crate::fluent_generated::parse_fn_trait_missing_paren); let applicability = if self.machine_applicable { diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 3f26ea4507d41..27e0ddc64d9d5 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1755,7 +1755,7 @@ impl Subdiagnostic for UnusedVariableStringInterp { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { diag.span_label(self.lit, crate::fluent_generated::passes_maybe_string_interpolation); diag.multipart_suggestion( diff --git a/compiler/rustc_pattern_analysis/src/errors.rs b/compiler/rustc_pattern_analysis/src/errors.rs index 75b7b7c8f6784..27f227e6d9c11 100644 --- a/compiler/rustc_pattern_analysis/src/errors.rs +++ b/compiler/rustc_pattern_analysis/src/errors.rs @@ -65,7 +65,7 @@ impl<'tcx> Subdiagnostic for Overlap<'tcx> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _: F, + _: &F, ) { let Overlap { span, range } = self; @@ -113,7 +113,7 @@ impl<'tcx> Subdiagnostic for GappedRange<'tcx> { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _: F, + _: &F, ) { let GappedRange { span, gap, first_range } = self; diff --git a/compiler/rustc_trait_selection/src/errors.rs b/compiler/rustc_trait_selection/src/errors.rs index b126062102eda..9b08a86ef1604 100644 --- a/compiler/rustc_trait_selection/src/errors.rs +++ b/compiler/rustc_trait_selection/src/errors.rs @@ -104,7 +104,7 @@ impl Subdiagnostic for AdjustSignatureBorrow { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - _f: F, + _f: &F, ) { match self { AdjustSignatureBorrow::Borrow { to_borrow } => { diff --git a/tests/ui-fulldeps/internal-lints/diagnostics.rs b/tests/ui-fulldeps/internal-lints/diagnostics.rs index 1d7417be58cc3..380b2b6c431d5 100644 --- a/tests/ui-fulldeps/internal-lints/diagnostics.rs +++ b/tests/ui-fulldeps/internal-lints/diagnostics.rs @@ -59,7 +59,7 @@ impl Subdiagnostic for UntranslatableInAddtoDiag { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { diag.note("untranslatable diagnostic"); //~^ ERROR diagnostics should be created using translatable messages @@ -72,7 +72,7 @@ impl Subdiagnostic for TranslatableInAddtoDiag { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, - f: F, + f: &F, ) { diag.note(crate::fluent_generated::no_crate_note); } From c88bb6c011a034a8b29d2a61734de0d36a02a1e9 Mon Sep 17 00:00:00 2001 From: Xiretza Date: Sat, 20 Apr 2024 12:52:05 +0000 Subject: [PATCH 18/22] Allow nesting subdiagnostics --- .../src/diagnostics/subdiagnostic.rs | 37 +++++++++++++------ compiler/rustc_macros/src/lib.rs | 1 + .../subdiagnostic-derive.rs | 10 +++++ 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index 9491426688444..45236771bce63 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -71,6 +71,7 @@ impl SubdiagnosticDerive { span_field: None, applicability: None, has_suggestion_parts: false, + has_subdiagnostic: false, is_enum, }; builder.into_tokens().unwrap_or_else(|v| v.to_compile_error()) @@ -133,6 +134,10 @@ struct SubdiagnosticDeriveVariantBuilder<'parent, 'a> { /// during finalization if still `false`. has_suggestion_parts: bool, + /// Set to true when a `#[subdiagnostic]` field is encountered, used to suppress the error + /// emitted when no subdiagnostic kinds are specified on the variant itself. + has_subdiagnostic: bool, + /// Set to true when this variant is an enum variant rather than just the body of a struct. is_enum: bool, } @@ -373,6 +378,13 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { Ok(quote! {}) } + "subdiagnostic" => { + let f = &self.parent.f; + let diag = &self.parent.diag; + let binding = &info.binding; + self.has_subdiagnostic = true; + Ok(quote! { #binding.add_to_diag_with(#diag, #f); }) + } _ => { let mut span_attrs = vec![]; if kind_stats.has_multipart_suggestion { @@ -480,18 +492,6 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { pub(crate) fn into_tokens(&mut self) -> Result { let kind_slugs = self.identify_kind()?; - if kind_slugs.is_empty() { - if self.is_enum { - // It's okay for a variant to not be a subdiagnostic at all.. - return Ok(quote! {}); - } else { - // ..but structs should always be _something_. - throw_span_err!( - self.variant.ast().ident.span().unwrap(), - "subdiagnostic kind not specified" - ); - } - }; let kind_stats: KindsStatistics = kind_slugs.iter().map(|(kind, _slug, _no_span)| kind).collect(); @@ -510,6 +510,19 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { .map(|binding| self.generate_field_attr_code(binding, kind_stats)) .collect(); + if kind_slugs.is_empty() { + if self.is_enum { + // It's okay for a variant to not be a subdiagnostic at all.. + return Ok(quote! {}); + } else if !self.has_subdiagnostic { + // ..but structs should always be _something_. + throw_span_err!( + self.variant.ast().ident.span().unwrap(), + "subdiagnostic kind not specified" + ); + } + }; + let span_field = self.span_field.value_ref(); let diag = &self.parent.diag; diff --git a/compiler/rustc_macros/src/lib.rs b/compiler/rustc_macros/src/lib.rs index eedc508fb1461..c7b7eadbd9d6b 100644 --- a/compiler/rustc_macros/src/lib.rs +++ b/compiler/rustc_macros/src/lib.rs @@ -144,6 +144,7 @@ decl_derive!( help, note, warning, + subdiagnostic, suggestion, suggestion_short, suggestion_hidden, diff --git a/tests/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs b/tests/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs index 163af7ff0e214..659ae54f7a3be 100644 --- a/tests/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs +++ b/tests/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs @@ -827,3 +827,13 @@ struct PrimarySpanOnVec { //~| NOTE there must be exactly one primary span sub: Vec, } + +#[derive(Subdiagnostic)] +struct NestedParent { + #[subdiagnostic] + single_sub: A, + #[subdiagnostic] + option_sub: Option, + #[subdiagnostic] + vec_sub: Vec, +} From b220b741c6d4e16e34d183b38d6384d369c9a620 Mon Sep 17 00:00:00 2001 From: Xiretza Date: Sat, 20 Apr 2024 12:57:29 +0000 Subject: [PATCH 19/22] Fix source ordering of IntoDiagArg impls --- compiler/rustc_errors/src/diagnostic_impls.rs | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 1ef945985be0a..93b1c6775d673 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -227,6 +227,36 @@ impl IntoDiagArg for rustc_lint_defs::Level { } } +impl IntoDiagArg for hir::def::Res { + fn into_diag_arg(self) -> DiagArgValue { + DiagArgValue::Str(Cow::Borrowed(self.descr())) + } +} + +impl IntoDiagArg for DiagLocation { + fn into_diag_arg(self) -> DiagArgValue { + DiagArgValue::Str(Cow::from(self.to_string())) + } +} + +impl IntoDiagArg for Backtrace { + fn into_diag_arg(self) -> DiagArgValue { + DiagArgValue::Str(Cow::from(self.to_string())) + } +} + +impl IntoDiagArg for Level { + fn into_diag_arg(self) -> DiagArgValue { + DiagArgValue::Str(Cow::from(self.to_string())) + } +} + +impl IntoDiagArg for type_ir::ClosureKind { + fn into_diag_arg(self) -> DiagArgValue { + DiagArgValue::Str(self.as_str().into()) + } +} + #[derive(Clone)] pub struct DiagSymbolList(Vec); @@ -244,12 +274,6 @@ impl IntoDiagArg for DiagSymbolList { } } -impl IntoDiagArg for hir::def::Res { - fn into_diag_arg(self) -> DiagArgValue { - DiagArgValue::Str(Cow::Borrowed(self.descr())) - } -} - impl Diagnostic<'_, G> for TargetDataLayoutErrors<'_> { fn into_diag(self, dcx: &DiagCtxt, level: Level) -> Diag<'_, G> { match self { @@ -316,24 +340,6 @@ pub struct ExpectedLifetimeParameter { pub count: usize, } -impl IntoDiagArg for DiagLocation { - fn into_diag_arg(self) -> DiagArgValue { - DiagArgValue::Str(Cow::from(self.to_string())) - } -} - -impl IntoDiagArg for Backtrace { - fn into_diag_arg(self) -> DiagArgValue { - DiagArgValue::Str(Cow::from(self.to_string())) - } -} - -impl IntoDiagArg for Level { - fn into_diag_arg(self) -> DiagArgValue { - DiagArgValue::Str(Cow::from(self.to_string())) - } -} - #[derive(Subdiagnostic)] #[suggestion(errors_indicate_anonymous_lifetime, code = "{suggestion}", style = "verbose")] pub struct IndicateAnonymousLifetime { @@ -342,9 +348,3 @@ pub struct IndicateAnonymousLifetime { pub count: usize, pub suggestion: String, } - -impl IntoDiagArg for type_ir::ClosureKind { - fn into_diag_arg(self) -> DiagArgValue { - DiagArgValue::Str(self.as_str().into()) - } -} From 6974e9cf7012b0f3e2a58a8accde23007274f9a9 Mon Sep 17 00:00:00 2001 From: Xiretza Date: Sat, 20 Apr 2024 13:21:15 +0000 Subject: [PATCH 20/22] Move "elided lifetime in path" to subdiagnostic struct This requires nested subdiagnostics. --- compiler/rustc_errors/src/diagnostic_impls.rs | 8 +++++ compiler/rustc_errors/src/lib.rs | 33 +++++++++---------- .../rustc_lint/src/context/diagnostics.rs | 18 +++++----- compiler/rustc_resolve/src/errors.rs | 4 ++- compiler/rustc_resolve/src/late.rs | 12 +++---- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 93b1c6775d673..2b10fcd8d6ab0 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -348,3 +348,11 @@ pub struct IndicateAnonymousLifetime { pub count: usize, pub suggestion: String, } + +#[derive(Subdiagnostic)] +pub struct ElidedLifetimeInPathSubdiag { + #[subdiagnostic] + pub expected: ExpectedLifetimeParameter, + #[subdiagnostic] + pub indicate: Option, +} diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 8d6b22a9fa988..02b7ec188784c 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -41,8 +41,8 @@ pub use diagnostic::{ SubdiagMessageOp, Subdiagnostic, }; pub use diagnostic_impls::{ - DiagArgFromDisplay, DiagSymbolList, ExpectedLifetimeParameter, IndicateAnonymousLifetime, - SingleLabelManySpans, + DiagArgFromDisplay, DiagSymbolList, ElidedLifetimeInPathSubdiag, ExpectedLifetimeParameter, + IndicateAnonymousLifetime, SingleLabelManySpans, }; pub use emitter::ColorConfig; pub use rustc_error_messages::{ @@ -1912,27 +1912,24 @@ impl Level { } // FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite. -pub fn add_elided_lifetime_in_path_suggestion( +pub fn elided_lifetime_in_path_suggestion( source_map: &SourceMap, - diag: &mut Diag<'_, G>, n: usize, path_span: Span, incl_angl_brckt: bool, insertion_span: Span, -) { - diag.subdiagnostic(diag.dcx, ExpectedLifetimeParameter { span: path_span, count: n }); - if !source_map.is_span_accessible(insertion_span) { - // Do not try to suggest anything if generated by a proc-macro. - return; - } - let anon_lts = vec!["'_"; n].join(", "); - let suggestion = - if incl_angl_brckt { format!("<{anon_lts}>") } else { format!("{anon_lts}, ") }; - - diag.subdiagnostic( - diag.dcx, - IndicateAnonymousLifetime { span: insertion_span.shrink_to_hi(), count: n, suggestion }, - ); +) -> ElidedLifetimeInPathSubdiag { + let expected = ExpectedLifetimeParameter { span: path_span, count: n }; + // Do not try to suggest anything if generated by a proc-macro. + let indicate = source_map.is_span_accessible(insertion_span).then(|| { + let anon_lts = vec!["'_"; n].join(", "); + let suggestion = + if incl_angl_brckt { format!("<{anon_lts}>") } else { format!("{anon_lts}, ") }; + + IndicateAnonymousLifetime { span: insertion_span.shrink_to_hi(), count: n, suggestion } + }); + + ElidedLifetimeInPathSubdiag { expected, indicate } } pub fn report_ambiguity_error<'a, G: EmissionGuarantee>( diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index b2403836397a1..8458b53933537 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -2,7 +2,7 @@ #![allow(rustc::untranslatable_diagnostic)] use rustc_ast::util::unicode::TEXT_FLOW_CONTROL_CHARS; -use rustc_errors::{add_elided_lifetime_in_path_suggestion, Diag}; +use rustc_errors::{elided_lifetime_in_path_suggestion, Diag}; use rustc_errors::{Applicability, SuggestionStyle}; use rustc_middle::middle::stability; use rustc_session::lint::BuiltinLintDiag; @@ -74,13 +74,15 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di diag.span_note(span_def, "the macro is defined here"); } BuiltinLintDiag::ElidedLifetimesInPaths(n, path_span, incl_angl_brckt, insertion_span) => { - add_elided_lifetime_in_path_suggestion( - sess.source_map(), - diag, - n, - path_span, - incl_angl_brckt, - insertion_span, + diag.subdiagnostic( + sess.dcx(), + elided_lifetime_in_path_suggestion( + sess.source_map(), + n, + path_span, + incl_angl_brckt, + insertion_span, + ), ); } BuiltinLintDiag::UnknownCrateTypes(span, note, sugg) => { diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs index b0329702d1143..edfeacec7e3b7 100644 --- a/compiler/rustc_resolve/src/errors.rs +++ b/compiler/rustc_resolve/src/errors.rs @@ -1,4 +1,4 @@ -use rustc_errors::{codes::*, Applicability, MultiSpan}; +use rustc_errors::{codes::*, Applicability, ElidedLifetimeInPathSubdiag, MultiSpan}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_span::{ symbol::{Ident, Symbol}, @@ -907,6 +907,8 @@ pub(crate) struct ExplicitAnonymousLivetimeReportError { pub(crate) struct ImplicitElidedLifetimeNotAllowedHere { #[primary_span] pub(crate) span: Span, + #[subdiagnostic] + pub(crate) subdiag: ElidedLifetimeInPathSubdiag, } #[derive(Diagnostic)] diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 753ba09d88686..c877ae5e21f48 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1883,20 +1883,18 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // async fn foo(_: std::cell::Ref) { ... } LifetimeRibKind::AnonymousCreateParameter { report_in_path: true, .. } | LifetimeRibKind::AnonymousWarn(_) => { - let mut err = - self.r.dcx().create_err(errors::ImplicitElidedLifetimeNotAllowedHere { - span: path_span, - }); let sess = self.r.tcx.sess; - rustc_errors::add_elided_lifetime_in_path_suggestion( + let subdiag = rustc_errors::elided_lifetime_in_path_suggestion( sess.source_map(), - &mut err, expected_lifetimes, path_span, !segment.has_generic_args, elided_lifetime_span, ); - err.emit(); + self.r.dcx().emit_err(errors::ImplicitElidedLifetimeNotAllowedHere { + span: path_span, + subdiag, + }); should_lint = false; for id in node_ids { From 31e581ec1201aac9a3475b16d2100a3ddd35e2e3 Mon Sep 17 00:00:00 2001 From: long-long-float Date: Thu, 25 Jan 2024 23:15:54 +0900 Subject: [PATCH 21/22] Wrap dyn type with parentheses in suggestion --- compiler/rustc_hir/src/hir.rs | 46 +++++++++-- .../rustc_hir_typeck/src/method/suggest.rs | 65 ++++++++++------ .../src/infer/error_reporting/mod.rs | 16 ++-- compiler/rustc_middle/src/ty/diagnostics.rs | 39 ++++++---- .../error_reporting/type_err_ctxt_ext.rs | 27 +++++-- ...ait-with-missing-trait-bounds-in-arg.fixed | 13 +++- ...-trait-with-missing-trait-bounds-in-arg.rs | 13 +++- ...it-with-missing-trait-bounds-in-arg.stderr | 16 +++- .../wrap-dyn-in-suggestion-issue-120223.rs | 35 +++++++++ ...wrap-dyn-in-suggestion-issue-120223.stderr | 78 +++++++++++++++++++ 10 files changed, 286 insertions(+), 62 deletions(-) create mode 100644 tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs create mode 100644 tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.stderr diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index e4f8d77dbc2e5..2268905430a7a 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -644,13 +644,49 @@ impl<'hir> Generics<'hir> { }) } - pub fn bounds_span_for_suggestions(&self, param_def_id: LocalDefId) -> Option { + /// Returns a suggestable empty span right after the "final" bound of the generic parameter. + /// + /// If that bound needs to be wrapped in parentheses to avoid ambiguity with + /// subsequent bounds, it also returns an empty span for an open parenthesis + /// as the second component. + /// + /// E.g., adding `+ 'static` after `Fn() -> dyn Future` or + /// `Fn() -> &'static dyn Debug` requires parentheses: + /// `Fn() -> (dyn Future) + 'static` and + /// `Fn() -> &'static (dyn Debug) + 'static`, respectively. + pub fn bounds_span_for_suggestions( + &self, + param_def_id: LocalDefId, + ) -> Option<(Span, Option)> { self.bounds_for_param(param_def_id).flat_map(|bp| bp.bounds.iter().rev()).find_map( |bound| { - // We include bounds that come from a `#[derive(_)]` but point at the user's code, - // as we use this method to get a span appropriate for suggestions. - let bs = bound.span(); - bs.can_be_used_for_suggestions().then(|| bs.shrink_to_hi()) + let span_for_parentheses = if let Some(trait_ref) = bound.trait_ref() + && let [.., segment] = trait_ref.path.segments + && segment.args().parenthesized == GenericArgsParentheses::ParenSugar + && let [binding] = segment.args().bindings + && let TypeBindingKind::Equality { term: Term::Ty(ret_ty) } = binding.kind + && let ret_ty = ret_ty.peel_refs() + && let TyKind::TraitObject( + _, + _, + TraitObjectSyntax::Dyn | TraitObjectSyntax::DynStar, + ) = ret_ty.kind + && ret_ty.span.can_be_used_for_suggestions() + { + Some(ret_ty.span) + } else { + None + }; + + span_for_parentheses.map_or_else( + || { + // We include bounds that come from a `#[derive(_)]` but point at the user's code, + // as we use this method to get a span appropriate for suggestions. + let bs = bound.span(); + bs.can_be_used_for_suggestions().then(|| (bs.shrink_to_hi(), None)) + }, + |span| Some((span.shrink_to_hi(), Some(span.shrink_to_lo()))), + ) }, ) } diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 46227e406a328..be9ac962434f5 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -3291,14 +3291,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { param.name.ident(), )); let bounds_span = hir_generics.bounds_span_for_suggestions(def_id); - if rcvr_ty.is_ref() && param.is_impl_trait() && bounds_span.is_some() { + if rcvr_ty.is_ref() + && param.is_impl_trait() + && let Some((bounds_span, _)) = bounds_span + { err.multipart_suggestions( msg, candidates.iter().map(|t| { vec![ (param.span.shrink_to_lo(), "(".to_string()), ( - bounds_span.unwrap(), + bounds_span, format!(" + {})", self.tcx.def_path_str(t.def_id)), ), ] @@ -3308,32 +3311,46 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return; } - let (sp, introducer) = if let Some(span) = bounds_span { - (span, Introducer::Plus) - } else if let Some(colon_span) = param.colon_span { - (colon_span.shrink_to_hi(), Introducer::Nothing) - } else if param.is_impl_trait() { - (param.span.shrink_to_hi(), Introducer::Plus) - } else { - (param.span.shrink_to_hi(), Introducer::Colon) - }; + let (sp, introducer, open_paren_sp) = + if let Some((span, open_paren_sp)) = bounds_span { + (span, Introducer::Plus, open_paren_sp) + } else if let Some(colon_span) = param.colon_span { + (colon_span.shrink_to_hi(), Introducer::Nothing, None) + } else if param.is_impl_trait() { + (param.span.shrink_to_hi(), Introducer::Plus, None) + } else { + (param.span.shrink_to_hi(), Introducer::Colon, None) + }; - err.span_suggestions( - sp, + let all_suggs = candidates.iter().map(|cand| { + let suggestion = format!( + "{} {}", + match introducer { + Introducer::Plus => " +", + Introducer::Colon => ":", + Introducer::Nothing => "", + }, + self.tcx.def_path_str(cand.def_id) + ); + + let mut suggs = vec![]; + + if let Some(open_paren_sp) = open_paren_sp { + suggs.push((open_paren_sp, "(".to_string())); + suggs.push((sp, format!("){suggestion}"))); + } else { + suggs.push((sp, suggestion)); + } + + suggs + }); + + err.multipart_suggestions( msg, - candidates.iter().map(|t| { - format!( - "{} {}", - match introducer { - Introducer::Plus => " +", - Introducer::Colon => ":", - Introducer::Nothing => "", - }, - self.tcx.def_path_str(t.def_id) - ) - }), + all_suggs, Applicability::MaybeIncorrect, ); + return; } Node::Item(hir::Item { diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index fdae9d84b5ff6..c8e6580f4b771 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -2369,7 +2369,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { generic_param_scope = self.tcx.local_parent(generic_param_scope); } - // type_param_sugg_span is (span, has_bounds) + // type_param_sugg_span is (span, has_bounds, needs_parentheses) let (type_scope, type_param_sugg_span) = match bound_kind { GenericKind::Param(ref param) => { let generics = self.tcx.generics_of(generic_param_scope); @@ -2380,10 +2380,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // instead we suggest `T: 'a + 'b` in that case. let hir_generics = self.tcx.hir().get_generics(scope).unwrap(); let sugg_span = match hir_generics.bounds_span_for_suggestions(def_id) { - Some(span) => Some((span, true)), + Some((span, open_paren_sp)) => Some((span, true, open_paren_sp)), // If `param` corresponds to `Self`, no usable suggestion span. None if generics.has_self && param.index == 0 => None, - None => Some((self.tcx.def_span(def_id).shrink_to_hi(), false)), + None => Some((self.tcx.def_span(def_id).shrink_to_hi(), false, None)), }; (scope, sugg_span) } @@ -2406,12 +2406,18 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let mut suggs = vec![]; let lt_name = self.suggest_name_region(sub, &mut suggs); - if let Some((sp, has_lifetimes)) = type_param_sugg_span + if let Some((sp, has_lifetimes, open_paren_sp)) = type_param_sugg_span && suggestion_scope == type_scope { let suggestion = if has_lifetimes { format!(" + {lt_name}") } else { format!(": {lt_name}") }; - suggs.push((sp, suggestion)) + + if let Some(open_paren_sp) = open_paren_sp { + suggs.push((open_paren_sp, "(".to_string())); + suggs.push((sp, format!("){suggestion}"))); + } else { + suggs.push((sp, suggestion)) + } } else if let GenericKind::Alias(ref p) = bound_kind && let ty::Projection = p.kind(self.tcx) && let DefKind::AssocTy = self.tcx.def_kind(p.def_id) diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index cc1d6e50f6d9a..8c3ee6955f5d2 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -279,24 +279,29 @@ pub fn suggest_constraining_type_params<'a>( constraint.sort(); constraint.dedup(); let constraint = constraint.join(" + "); - let mut suggest_restrict = |span, bound_list_non_empty| { - suggestions.push(( - span, - if span_to_replace.is_some() { - constraint.clone() - } else if constraint.starts_with('<') { - constraint.to_string() - } else if bound_list_non_empty { - format!(" + {constraint}") - } else { - format!(" {constraint}") - }, - SuggestChangingConstraintsMessage::RestrictBoundFurther, - )) + let mut suggest_restrict = |span, bound_list_non_empty, open_paren_sp| { + let suggestion = if span_to_replace.is_some() { + constraint.clone() + } else if constraint.starts_with('<') { + constraint.to_string() + } else if bound_list_non_empty { + format!(" + {constraint}") + } else { + format!(" {constraint}") + }; + + use SuggestChangingConstraintsMessage::RestrictBoundFurther; + + if let Some(open_paren_sp) = open_paren_sp { + suggestions.push((open_paren_sp, "(".to_string(), RestrictBoundFurther)); + suggestions.push((span, format!("){suggestion}"), RestrictBoundFurther)); + } else { + suggestions.push((span, suggestion, RestrictBoundFurther)); + } }; if let Some(span) = span_to_replace { - suggest_restrict(span, true); + suggest_restrict(span, true, None); continue; } @@ -327,8 +332,8 @@ pub fn suggest_constraining_type_params<'a>( // -- // | // replace with: `T: Bar +` - if let Some(span) = generics.bounds_span_for_suggestions(param.def_id) { - suggest_restrict(span, true); + if let Some((span, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) { + suggest_restrict(span, true, open_paren_sp); continue; } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 96596de32aa37..0e309689680ad 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -2938,17 +2938,28 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } _ => {} }; + // Didn't add an indirection suggestion, so add a general suggestion to relax `Sized`. - let (span, separator) = if let Some(s) = generics.bounds_span_for_suggestions(param.def_id) - { - (s, " +") + let (span, separator, open_paren_sp) = + if let Some((s, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) { + (s, " +", open_paren_sp) + } else { + (param.name.ident().span.shrink_to_hi(), ":", None) + }; + + let mut suggs = vec![]; + let suggestion = format!("{separator} ?Sized"); + + if let Some(open_paren_sp) = open_paren_sp { + suggs.push((open_paren_sp, "(".to_string())); + suggs.push((span, format!("){suggestion}"))); } else { - (param.name.ident().span.shrink_to_hi(), ":") - }; - err.span_suggestion_verbose( - span, + suggs.push((span, suggestion)); + } + + err.multipart_suggestion_verbose( "consider relaxing the implicit `Sized` restriction", - format!("{separator} ?Sized"), + suggs, Applicability::MachineApplicable, ); } diff --git a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed index 232d1dd8138be..69780648ab666 100644 --- a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed +++ b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed @@ -12,7 +12,18 @@ impl Foo for S {} impl Bar for S {} fn test(foo: impl Foo + Bar) { - foo.hello(); //~ ERROR E0599 + foo.hello(); //~ ERROR no method named `hello` found +} + +trait Trait { + fn method(&self) {} +} + +impl Trait for fn() {} + +#[allow(dead_code)] +fn test2(f: impl Fn() -> (dyn std::fmt::Debug) + Trait) { + f.method(); //~ ERROR no method named `method` found } fn main() { diff --git a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs index ab25b362fedbb..f49512bdd62af 100644 --- a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs +++ b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs @@ -12,7 +12,18 @@ impl Foo for S {} impl Bar for S {} fn test(foo: impl Foo) { - foo.hello(); //~ ERROR E0599 + foo.hello(); //~ ERROR no method named `hello` found +} + +trait Trait { + fn method(&self) {} +} + +impl Trait for fn() {} + +#[allow(dead_code)] +fn test2(f: impl Fn() -> dyn std::fmt::Debug) { + f.method(); //~ ERROR no method named `method` found } fn main() { diff --git a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr index 3a6052448cb0f..0aced78ddacba 100644 --- a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr +++ b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr @@ -12,6 +12,20 @@ help: the following trait defines an item `hello`, perhaps you need to restrict LL | fn test(foo: impl Foo + Bar) { | +++++ -error: aborting due to 1 previous error +error[E0599]: no method named `method` found for type parameter `impl Fn() -> dyn std::fmt::Debug` in the current scope + --> $DIR/impl-trait-with-missing-trait-bounds-in-arg.rs:26:7 + | +LL | fn test2(f: impl Fn() -> dyn std::fmt::Debug) { + | -------------------------------- method `method` not found for this type parameter +LL | f.method(); + | ^^^^^^ method not found in `impl Fn() -> dyn std::fmt::Debug` + | + = help: items from traits can only be used if the type parameter is bounded by the trait +help: the following trait defines an item `method`, perhaps you need to restrict type parameter `impl Fn() -> dyn std::fmt::Debug` with it: + | +LL | fn test2(f: impl Fn() -> (dyn std::fmt::Debug) + Trait) { + | + +++++++++ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0599`. diff --git a/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs new file mode 100644 index 0000000000000..6a273997ee630 --- /dev/null +++ b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs @@ -0,0 +1,35 @@ +#![feature(dyn_star)] //~ WARNING the feature `dyn_star` is incomplete + +use std::future::Future; + +pub fn dyn_func( + executor: impl FnOnce(T) -> dyn Future, +) -> Box dyn Future> { + Box::new(executor) //~ ERROR may not live long enough +} + +pub fn dyn_star_func( + executor: impl FnOnce(T) -> dyn* Future, +) -> Box dyn* Future> { + Box::new(executor) //~ ERROR may not live long enough +} + +trait Trait { + fn method(&self) {} +} + +impl Trait for fn() {} + +pub fn in_ty_param dyn std::fmt::Debug> (t: T) { + t.method(); + //~^ ERROR no method named `method` found for type parameter `T` +} + +fn with_sized &'static (dyn std::fmt::Debug) + ?Sized>() { + without_sized::(); + //~^ ERROR the size for values of type `T` cannot be known at compilation time +} + +fn without_sized &'static dyn std::fmt::Debug>() {} + +fn main() {} diff --git a/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.stderr b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.stderr new file mode 100644 index 0000000000000..f7fc17ea24f18 --- /dev/null +++ b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.stderr @@ -0,0 +1,78 @@ +warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:1:12 + | +LL | #![feature(dyn_star)] + | ^^^^^^^^ + | + = note: see issue #102425 for more information + = note: `#[warn(incomplete_features)]` on by default + +error[E0599]: no method named `method` found for type parameter `T` in the current scope + --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:24:7 + | +LL | pub fn in_ty_param dyn std::fmt::Debug> (t: T) { + | - method `method` not found for this type parameter +LL | t.method(); + | ^^^^^^ method not found in `T` + | + = help: items from traits can only be used if the type parameter is bounded by the trait +help: the following trait defines an item `method`, perhaps you need to restrict type parameter `T` with it: + | +LL | pub fn in_ty_param (dyn std::fmt::Debug) + Trait> (t: T) { + | + +++++++++ + +error[E0277]: the size for values of type `T` cannot be known at compilation time + --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:29:21 + | +LL | fn with_sized &'static (dyn std::fmt::Debug) + ?Sized>() { + | - this type parameter needs to be `Sized` +LL | without_sized::(); + | ^ doesn't have a size known at compile-time + | +note: required by an implicit `Sized` bound in `without_sized` + --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:33:18 + | +LL | fn without_sized &'static dyn std::fmt::Debug>() {} + | ^ required by the implicit `Sized` requirement on this type parameter in `without_sized` +help: consider removing the `?Sized` bound to make the type parameter `Sized` + | +LL - fn with_sized &'static (dyn std::fmt::Debug) + ?Sized>() { +LL + fn with_sized &'static (dyn std::fmt::Debug)>() { + | +help: consider relaxing the implicit `Sized` restriction + | +LL | fn without_sized &'static (dyn std::fmt::Debug) + ?Sized>() {} + | + ++++++++++ + +error[E0310]: the parameter type `impl FnOnce(T) -> dyn Future` may not live long enough + --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:8:5 + | +LL | Box::new(executor) + | ^^^^^^^^^^^^^^^^^^ + | | + | the parameter type `impl FnOnce(T) -> dyn Future` must be valid for the static lifetime... + | ...so that the type `impl FnOnce(T) -> dyn Future` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | executor: impl FnOnce(T) -> (dyn Future) + 'static, + | + +++++++++++ + +error[E0310]: the parameter type `impl FnOnce(T) -> Future` may not live long enough + --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:14:5 + | +LL | Box::new(executor) + | ^^^^^^^^^^^^^^^^^^ + | | + | the parameter type `impl FnOnce(T) -> Future` must be valid for the static lifetime... + | ...so that the type `impl FnOnce(T) -> Future` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | executor: impl FnOnce(T) -> (dyn* Future) + 'static, + | + +++++++++++ + +error: aborting due to 4 previous errors; 1 warning emitted + +Some errors have detailed explanations: E0277, E0310, E0599. +For more information about an error, try `rustc --explain E0277`. From 481515500a7588146dfc6a2e21be3ba3a2df4668 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 23 Apr 2024 10:44:14 +0200 Subject: [PATCH 22/22] Mark @RUSTC_BUILTIN search path usage as unstable --- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_session/src/config.rs | 8 +++++++- compiler/rustc_session/src/search_paths.rs | 9 +++++++++ src/librustdoc/config.rs | 11 ++++++++++- 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 8d741ef4c1b69..f6014bcd89303 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -321,6 +321,7 @@ fn test_search_paths_tracking_hash_different_order() { &opts.target_triple, &early_dcx, search_path, + false, )); }; diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index d5b22f841d273..07c116631737f 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2548,7 +2548,13 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M let mut search_paths = vec![]; for s in &matches.opt_strs("L") { - search_paths.push(SearchPath::from_cli_opt(&sysroot, &target_triple, early_dcx, s)); + search_paths.push(SearchPath::from_cli_opt( + &sysroot, + &target_triple, + early_dcx, + s, + unstable_opts.unstable_options, + )); } let working_dir = std::env::current_dir().unwrap_or_else(|e| { diff --git a/compiler/rustc_session/src/search_paths.rs b/compiler/rustc_session/src/search_paths.rs index 16dd40acef047..bc2f2a8af1fa9 100644 --- a/compiler/rustc_session/src/search_paths.rs +++ b/compiler/rustc_session/src/search_paths.rs @@ -52,6 +52,7 @@ impl SearchPath { triple: &TargetTriple, early_dcx: &EarlyDiagCtxt, path: &str, + is_unstable_enabled: bool, ) -> Self { let (kind, path) = if let Some(stripped) = path.strip_prefix("native=") { (PathKind::Native, stripped) @@ -68,6 +69,14 @@ impl SearchPath { }; let dir = match path.strip_prefix("@RUSTC_BUILTIN") { Some(stripped) => { + if !is_unstable_enabled { + #[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable + early_dcx.early_fatal( + "the `-Z unstable-options` flag must also be passed to \ + enable the use of `@RUSTC_BUILTIN`", + ); + } + make_target_lib_path(sysroot, triple.triple()).join("builtin").join(stripped) } None => PathBuf::from(path), diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index f078ad2fb5376..465f969435d1b 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -635,7 +635,16 @@ impl Options { let libs = matches .opt_strs("L") .iter() - .map(|s| SearchPath::from_cli_opt(&sysroot, &target, early_dcx, s)) + .map(|s| { + SearchPath::from_cli_opt( + &sysroot, + &target, + early_dcx, + s, + #[allow(rustc::bad_opt_access)] // we have no `Session` here + unstable_opts.unstable_options, + ) + }) .collect(); let show_coverage = matches.opt_present("show-coverage");