From 168de14ac9f3ec0b0f6fa7f5328336e6ce97d60b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 16:02:11 +0000 Subject: [PATCH 01/11] Make simd_shuffle_indices use valtrees --- compiler/rustc_codegen_ssa/src/mir/block.rs | 11 +++- .../rustc_codegen_ssa/src/mir/constant.rs | 23 ++++--- .../rustc_const_eval/src/const_eval/mod.rs | 1 - .../rustc_middle/src/mir/interpret/queries.rs | 26 +++----- .../clippy/clippy_lints/src/non_copy_const.rs | 64 +++++++++++++++---- .../clippy/tests/ui/crashes/ice-9445.stderr | 12 ++++ 6 files changed, 92 insertions(+), 45 deletions(-) create mode 100644 src/tools/clippy/tests/ui/crashes/ice-9445.stderr diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 0cec560ba4518..d53bffb3e03cc 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -863,7 +863,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // promotes any complex rvalues to constants. if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { if let mir::Operand::Constant(constant) = arg { - let c = self.eval_mir_constant(constant); + let ct = self.monomorphize(constant.literal); + let uv = match ct { + mir::ConstantKind::Unevaluated(uv, _) => uv.shrink(), + other => span_bug!(constant.span, "{other:#?}"), + }; + let c = self.cx.tcx().const_eval_resolve_for_typeck( + ty::ParamEnv::reveal_all(), + uv, + Some(constant.span), + ); let (llval, ty) = self.simd_shuffle_indices( &bx, constant.span, diff --git a/compiler/rustc_codegen_ssa/src/mir/constant.rs b/compiler/rustc_codegen_ssa/src/mir/constant.rs index 14fe84a146da0..a2ee9c54b16f9 100644 --- a/compiler/rustc_codegen_ssa/src/mir/constant.rs +++ b/compiler/rustc_codegen_ssa/src/mir/constant.rs @@ -65,16 +65,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx: &Bx, span: Span, ty: Ty<'tcx>, - constant: Result, ErrorHandled>, + constant: Result>, ErrorHandled>, ) -> (Bx::Value, Ty<'tcx>) { - constant + let val = constant + .ok() + .flatten() .map(|val| { let field_ty = ty.builtin_index().unwrap(); - let c = mir::ConstantKind::from_value(val, ty); - let values: Vec<_> = bx - .tcx() - .destructure_mir_constant(ty::ParamEnv::reveal_all(), c) - .fields + let values: Vec<_> = val + .unwrap_branch() .iter() .map(|field| { if let Some(prim) = field.try_to_scalar() { @@ -88,15 +87,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } }) .collect(); - let llval = bx.const_struct(&values, false); - (llval, c.ty()) + bx.const_struct(&values, false) }) - .unwrap_or_else(|_| { + .unwrap_or_else(|| { bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span }); // We've errored, so we don't have to produce working code. let ty = self.monomorphize(ty); let llty = bx.backend_type(bx.layout_of(ty)); - (bx.const_undef(llty), ty) - }) + bx.const_undef(llty) + }); + (val, ty) } } diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs index b9ab0a4b7c8f4..5cc1fa2a4974b 100644 --- a/compiler/rustc_const_eval/src/const_eval/mod.rs +++ b/compiler/rustc_const_eval/src/const_eval/mod.rs @@ -92,7 +92,6 @@ pub(crate) fn try_destructure_mir_constant<'tcx>( param_env: ty::ParamEnv<'tcx>, val: mir::ConstantKind<'tcx>, ) -> InterpResult<'tcx, mir::DestructuredConstant<'tcx>> { - trace!("destructure_mir_constant: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No); let op = ecx.eval_mir_constant(&val, None, None)?; diff --git a/compiler/rustc_middle/src/mir/interpret/queries.rs b/compiler/rustc_middle/src/mir/interpret/queries.rs index ae32a54be3ded..9c97431f3614a 100644 --- a/compiler/rustc_middle/src/mir/interpret/queries.rs +++ b/compiler/rustc_middle/src/mir/interpret/queries.rs @@ -95,11 +95,15 @@ impl<'tcx> TyCtxt<'tcx> { // used generic parameters is a bug of evaluation, so checking for it // here does feel somewhat sensible. if !self.features().generic_const_exprs && ct.substs.has_non_region_param() { - assert!(matches!( - self.def_kind(ct.def), - DefKind::InlineConst | DefKind::AnonConst - )); - let mir_body = self.mir_for_ctfe(ct.def); + let def_kind = self.def_kind(instance.def_id()); + assert!( + matches!( + def_kind, + DefKind::InlineConst | DefKind::AnonConst | DefKind::AssocConst + ), + "{cid:?} is {def_kind:?}", + ); + let mir_body = self.mir_for_ctfe(instance.def_id()); if mir_body.is_polymorphic { let Some(local_def_id) = ct.def.as_local() else { return }; self.struct_span_lint_hir( @@ -239,15 +243,3 @@ impl<'tcx> TyCtxtEnsure<'tcx> { self.eval_to_allocation_raw(param_env.and(gid)) } } - -impl<'tcx> TyCtxt<'tcx> { - /// Destructure a mir constant ADT or array into its variant index and its field values. - /// Panics if the destructuring fails, use `try_destructure_mir_constant` for fallible version. - pub fn destructure_mir_constant( - self, - param_env: ty::ParamEnv<'tcx>, - constant: mir::ConstantKind<'tcx>, - ) -> mir::DestructuredConstant<'tcx> { - self.try_destructure_mir_constant(param_env.and(constant)).unwrap() - } -} diff --git a/src/tools/clippy/clippy_lints/src/non_copy_const.rs b/src/tools/clippy/clippy_lints/src/non_copy_const.rs index 58590df1fedf8..ac49ccdb5011b 100644 --- a/src/tools/clippy/clippy_lints/src/non_copy_const.rs +++ b/src/tools/clippy/clippy_lints/src/non_copy_const.rs @@ -15,12 +15,14 @@ use rustc_hir::{ }; use rustc_hir_analysis::hir_ty_to_ty; use rustc_lint::{LateContext, LateLintPass, Lint}; -use rustc_middle::mir; -use rustc_middle::mir::interpret::{ConstValue, ErrorHandled}; +use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::ty::adjustment::Adjust; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, InnerSpan, Span}; +use rustc_target::abi::VariantIdx; +use rustc_middle::mir::interpret::EvalToValTreeResult; +use rustc_middle::mir::interpret::GlobalId; // FIXME: this is a correctness problem but there's no suitable // warn-by-default category. @@ -141,21 +143,35 @@ fn is_unfrozen<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { fn is_value_unfrozen_raw<'tcx>( cx: &LateContext<'tcx>, - result: Result, ErrorHandled>, + result: Result>, ErrorHandled>, ty: Ty<'tcx>, ) -> bool { - fn inner<'tcx>(cx: &LateContext<'tcx>, val: mir::ConstantKind<'tcx>) -> bool { - match val.ty().kind() { + fn inner<'tcx>(cx: &LateContext<'tcx>, val: ty::ValTree<'tcx>, ty: Ty<'tcx>) -> bool { + match *ty.kind() { // the fact that we have to dig into every structs to search enums // leads us to the point checking `UnsafeCell` directly is the only option. ty::Adt(ty_def, ..) if ty_def.is_unsafe_cell() => true, // As of 2022-09-08 miri doesn't track which union field is active so there's no safe way to check the // contained value. ty::Adt(def, ..) if def.is_union() => false, - ty::Array(..) | ty::Adt(..) | ty::Tuple(..) => { - let val = cx.tcx.destructure_mir_constant(cx.param_env, val); - val.fields.iter().any(|field| inner(cx, *field)) + ty::Array(ty, _) => { + val.unwrap_branch().iter().any(|field| inner(cx, *field, ty)) }, + ty::Adt(def, _) if def.is_union() => false, + ty::Adt(def, substs) if def.is_enum() => { + let (&variant_index, fields) = val.unwrap_branch().split_first().unwrap(); + let variant_index = + VariantIdx::from_u32(variant_index.unwrap_leaf().try_to_u32().ok().unwrap()); + fields.iter().copied().zip( + def.variants()[variant_index] + .fields + .iter() + .map(|field| field.ty(cx.tcx, substs))).any(|(field, ty)| inner(cx, field, ty)) + } + ty::Adt(def, substs) => { + val.unwrap_branch().iter().zip(def.non_enum_variant().fields.iter().map(|field| field.ty(cx.tcx, substs))).any(|(field, ty)| inner(cx, *field, ty)) + } + ty::Tuple(tys) => val.unwrap_branch().iter().zip(tys).any(|(field, ty)| inner(cx, *field, ty)), _ => false, } } @@ -184,24 +200,44 @@ fn is_value_unfrozen_raw<'tcx>( // I chose this way because unfrozen enums as assoc consts are rare (or, hopefully, none). err == ErrorHandled::TooGeneric }, - |val| inner(cx, mir::ConstantKind::from_value(val, ty)), + |val| val.map_or(true, |val| inner(cx, val, ty)), ) } fn is_value_unfrozen_poly<'tcx>(cx: &LateContext<'tcx>, body_id: BodyId, ty: Ty<'tcx>) -> bool { - let result = cx.tcx.const_eval_poly(body_id.hir_id.owner.to_def_id()); + let def_id = body_id.hir_id.owner.to_def_id(); + let substs = ty::InternalSubsts::identity_for_item(cx.tcx, def_id); + let instance = ty::Instance::new(def_id, substs); + let cid = rustc_middle::mir::interpret::GlobalId { instance, promoted: None }; + let param_env = cx.tcx.param_env(def_id).with_reveal_all_normalized(cx.tcx); + let result = cx.tcx.const_eval_global_id_for_typeck(param_env, cid, None); is_value_unfrozen_raw(cx, result, ty) } fn is_value_unfrozen_expr<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId, def_id: DefId, ty: Ty<'tcx>) -> bool { let substs = cx.typeck_results().node_substs(hir_id); - let result = cx - .tcx - .const_eval_resolve(cx.param_env, mir::UnevaluatedConst::new(def_id, substs), None); + let result = const_eval_resolve(cx.tcx, cx.param_env, ty::UnevaluatedConst::new(def_id, substs), None); is_value_unfrozen_raw(cx, result, ty) } + +pub fn const_eval_resolve<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + ct: ty::UnevaluatedConst<'tcx>, + span: Option, +) -> EvalToValTreeResult<'tcx> { + match ty::Instance::resolve(tcx, param_env, ct.def, ct.substs) { + Ok(Some(instance)) => { + let cid = GlobalId { instance, promoted: None }; + tcx.const_eval_global_id_for_typeck(param_env, cid, span) + } + Ok(None) => Err(ErrorHandled::TooGeneric), + Err(err) => Err(ErrorHandled::Reported(err.into())), + } +} + #[derive(Copy, Clone)] enum Source { Item { item: Span }, diff --git a/src/tools/clippy/tests/ui/crashes/ice-9445.stderr b/src/tools/clippy/tests/ui/crashes/ice-9445.stderr new file mode 100644 index 0000000000000..a59d098e5ac0a --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/ice-9445.stderr @@ -0,0 +1,12 @@ +error: a `const` item should never be interior mutable + --> $DIR/ice-9445.rs:1:1 + | +LL | const UNINIT: core::mem::MaybeUninit> = core::mem::MaybeUninit::uninit(); + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | make this a static item (maybe with lazy_static) + | + = note: `-D clippy::declare-interior-mutable-const` implied by `-D warnings` + +error: aborting due to previous error + From acdfec60615ffe82658f3c782ae36e6f481d4e75 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 20 Jun 2023 08:14:51 +0000 Subject: [PATCH 02/11] Move mir const to valtree conversion to its own method. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 17 +--------- .../rustc_codegen_ssa/src/mir/constant.rs | 31 ++++++++++++++----- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index d53bffb3e03cc..068b7c1320d32 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -863,22 +863,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // promotes any complex rvalues to constants. if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { if let mir::Operand::Constant(constant) = arg { - let ct = self.monomorphize(constant.literal); - let uv = match ct { - mir::ConstantKind::Unevaluated(uv, _) => uv.shrink(), - other => span_bug!(constant.span, "{other:#?}"), - }; - let c = self.cx.tcx().const_eval_resolve_for_typeck( - ty::ParamEnv::reveal_all(), - uv, - Some(constant.span), - ); - let (llval, ty) = self.simd_shuffle_indices( - &bx, - constant.span, - self.monomorphize(constant.ty()), - c, - ); + let (llval, ty) = self.simd_shuffle_indices(&bx, constant); return OperandRef { val: Immediate(llval), layout: bx.layout_of(ty), diff --git a/compiler/rustc_codegen_ssa/src/mir/constant.rs b/compiler/rustc_codegen_ssa/src/mir/constant.rs index a2ee9c54b16f9..1c5031dfc4b4b 100644 --- a/compiler/rustc_codegen_ssa/src/mir/constant.rs +++ b/compiler/rustc_codegen_ssa/src/mir/constant.rs @@ -5,7 +5,6 @@ use rustc_middle::mir; use rustc_middle::mir::interpret::{ConstValue, ErrorHandled}; use rustc_middle::ty::layout::HasTyCtxt; use rustc_middle::ty::{self, Ty}; -use rustc_span::source_map::Span; use rustc_target::abi::Abi; use super::FunctionCx; @@ -59,15 +58,34 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) } + /// This is a convenience helper for `simd_shuffle_indices`. It has the precondition + /// that the given `constant` is an `ConstantKind::Unevaluated` and must be convertible to + /// a `ValTree`. If you want a more general version of this, talk to `wg-const-eval` on zulip. + pub fn eval_unevaluated_mir_constant_to_valtree( + &self, + constant: &mir::Constant<'tcx>, + ) -> Result>, ErrorHandled> { + let uv = match constant.literal { + mir::ConstantKind::Unevaluated(uv, _) => uv.shrink(), + other => span_bug!(constant.span, "{other:#?}"), + }; + let uv = self.monomorphize(uv); + self.cx.tcx().const_eval_resolve_for_typeck( + ty::ParamEnv::reveal_all(), + uv, + Some(constant.span), + ) + } + /// process constant containing SIMD shuffle indices pub fn simd_shuffle_indices( &mut self, bx: &Bx, - span: Span, - ty: Ty<'tcx>, - constant: Result>, ErrorHandled>, + constant: &mir::Constant<'tcx>, ) -> (Bx::Value, Ty<'tcx>) { - let val = constant + let ty = self.monomorphize(constant.ty()); + let val = self + .eval_unevaluated_mir_constant_to_valtree(constant) .ok() .flatten() .map(|val| { @@ -90,9 +108,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx.const_struct(&values, false) }) .unwrap_or_else(|| { - bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span }); + bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span: constant.span }); // We've errored, so we don't have to produce working code. - let ty = self.monomorphize(ty); let llty = bx.backend_type(bx.layout_of(ty)); bx.const_undef(llty) }); From 1c992c0b1c9b106c755d6ed4dce89ec285216486 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 26 Jun 2023 09:32:15 +0000 Subject: [PATCH 03/11] Assert that we don't convert unevaluated MIR promoteds to unevaluated type constants --- compiler/rustc_middle/src/mir/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 669c609d99579..ad1c93c31e982 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2581,10 +2581,9 @@ pub struct UnevaluatedConst<'tcx> { } impl<'tcx> UnevaluatedConst<'tcx> { - // FIXME: probably should get rid of this method. It's also wrong to - // shrink and then later expand a promoted. #[inline] pub fn shrink(self) -> ty::UnevaluatedConst<'tcx> { + assert_eq!(self.promoted, None); ty::UnevaluatedConst { def: self.def, substs: self.substs } } } From e21d56a91b8527ee4c82689b3a6750b2d74ca652 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 29 Jun 2023 16:18:36 -0700 Subject: [PATCH 04/11] Update std to backtrace 0.3.68 --- Cargo.lock | 50 +++++++++++++++++++++--------------------- library/backtrace | 2 +- library/std/Cargo.toml | 6 ++--- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3aa3e4c443857..931e41007b249 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,9 +4,9 @@ version = 3 [[package]] name = "addr2line" -version = "0.19.0" +version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a76fd60b23679b7d19bd066031410fb7e458ccc5e958eb5c325888ce4baedc97" +checksum = "f4fa78e18c64fce05e902adecd7a5eed15a5e0a3439f7b0e169f0252214865e3" dependencies = [ "compiler_builtins", "gimli 0.27.2", @@ -164,7 +164,7 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74cfb39880a59e122232cb5fb06b20b4382d58c12fa9747d16f846d38a7b094c" dependencies = [ - "object 0.31.1", + "object", ] [[package]] @@ -230,16 +230,16 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" [[package]] name = "backtrace" -version = "0.3.67" +version = "0.3.68" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "233d376d6d185f2a3093e58f283f60f880315b6c60075b01f36b3b85154564ca" +checksum = "4319208da049c43661739c5fade2ba182f09d1dc2299b32298d3a31692b17e12" dependencies = [ "addr2line", "cc", "cfg-if", "libc", - "miniz_oxide", - "object 0.30.1", + "miniz_oxide 0.7.1", + "object", "rustc-demangle", ] @@ -1179,7 +1179,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8a2db397cb1c8772f31494cb8917e48cd1e64f0fa7efac59fbd741a0a8ce841" dependencies = [ "crc32fast", - "miniz_oxide", + "miniz_oxide 0.6.2", ] [[package]] @@ -2149,6 +2149,15 @@ name = "miniz_oxide" version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b275950c28b37e794e8c55d88aeb5e139d0ce23fdbbeda68f8d7174abdf9e8fa" +dependencies = [ + "adler", +] + +[[package]] +name = "miniz_oxide" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7810e0be55b428ada41041c41f32c9f1a42817901b4ccf45fa3d4b6561e74c7" dependencies = [ "adler", "compiler_builtins", @@ -2260,29 +2269,20 @@ dependencies = [ "libc", ] -[[package]] -name = "object" -version = "0.30.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d864c91689fdc196779b98dba0aceac6118594c2df6ee5d943eb6a8df4d107a" -dependencies = [ - "compiler_builtins", - "memchr", - "rustc-std-workspace-alloc", - "rustc-std-workspace-core", -] - [[package]] name = "object" version = "0.31.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8bda667d9f2b5051b8833f59f3bf748b28ef54f850f4fcb389a252aa383866d1" dependencies = [ + "compiler_builtins", "crc32fast", "flate2", "hashbrown 0.13.1", "indexmap", "memchr", + "rustc-std-workspace-alloc", + "rustc-std-workspace-core", "ruzstd", ] @@ -3104,7 +3104,7 @@ dependencies = [ "bitflags", "libc", "measureme", - "object 0.31.1", + "object", "rustc-demangle", "rustc_ast", "rustc_attr", @@ -3140,7 +3140,7 @@ dependencies = [ "itertools", "jobserver", "libc", - "object 0.31.1", + "object", "pathdiff", "regex", "rustc_arena", @@ -4559,8 +4559,8 @@ dependencies = [ "hashbrown 0.14.0", "hermit-abi 0.3.0", "libc", - "miniz_oxide", - "object 0.30.1", + "miniz_oxide 0.7.1", + "object", "panic_abort", "panic_unwind", "profiler_builtins", @@ -4831,7 +4831,7 @@ checksum = "98c040e1340b889d4180c64e1d787efa9c32cb1617757e101480b61238b0d927" dependencies = [ "gimli 0.26.2", "hashbrown 0.12.3", - "object 0.31.1", + "object", "tracing", ] diff --git a/library/backtrace b/library/backtrace index 4245978ca8169..e1c49fbd6124a 160000 --- a/library/backtrace +++ b/library/backtrace @@ -1 +1 @@ -Subproject commit 4245978ca8169c40c088ff733825e4527f7b914c +Subproject commit e1c49fbd6124a1b626cdf19871aff68c362bdf07 diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 151809b2df5e1..716602a809982 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -25,11 +25,11 @@ hashbrown = { version = "0.14", default-features = false, features = ['rustc-dep std_detect = { path = "../stdarch/crates/std_detect", default-features = false, features = ['rustc-dep-of-std'] } # Dependencies of the `backtrace` crate -addr2line = { version = "0.19.0", optional = true, default-features = false } +addr2line = { version = "0.20.0", optional = true, default-features = false } rustc-demangle = { version = "0.1.21", features = ['rustc-dep-of-std'] } -miniz_oxide = { version = "0.6.0", optional = true, default-features = false, public = false } +miniz_oxide = { version = "0.7.0", optional = true, default-features = false, public = false } [dependencies.object] -version = "0.30.0" +version = "0.31.1" optional = true default-features = false features = ['read_core', 'elf', 'macho', 'pe', 'unaligned', 'archive'] From 2150451ab6b79db1d8a2e098b84d24a79b634178 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 29 Jun 2023 16:27:09 -0700 Subject: [PATCH 05/11] Update flate2 to dedup miniz_oxide --- Cargo.lock | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 931e41007b249..6dea4d7704ac1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -238,7 +238,7 @@ dependencies = [ "cc", "cfg-if", "libc", - "miniz_oxide 0.7.1", + "miniz_oxide", "object", "rustc-demangle", ] @@ -1174,12 +1174,12 @@ checksum = "37ab347416e802de484e4d03c7316c48f1ecb56574dfd4a46a80f173ce1de04d" [[package]] name = "flate2" -version = "1.0.25" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8a2db397cb1c8772f31494cb8917e48cd1e64f0fa7efac59fbd741a0a8ce841" +checksum = "3b9429470923de8e8cbd4d2dc513535400b4b3fef0319fb5c4e1f520a7bef743" dependencies = [ "crc32fast", - "miniz_oxide 0.6.2", + "miniz_oxide", ] [[package]] @@ -2144,15 +2144,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" -[[package]] -name = "miniz_oxide" -version = "0.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b275950c28b37e794e8c55d88aeb5e139d0ce23fdbbeda68f8d7174abdf9e8fa" -dependencies = [ - "adler", -] - [[package]] name = "miniz_oxide" version = "0.7.1" @@ -4559,7 +4550,7 @@ dependencies = [ "hashbrown 0.14.0", "hermit-abi 0.3.0", "libc", - "miniz_oxide 0.7.1", + "miniz_oxide", "object", "panic_abort", "panic_unwind", From d567e4f8b6abb0381e7884f36330a9f39344fb98 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 30 Jun 2023 00:16:05 +0000 Subject: [PATCH 06/11] Error for RPITIT hidden tys that capture more than their trait defn --- .../src/check/compare_impl_item.rs | 131 +++++++++++++++--- .../signature-mismatch.current.stderr | 18 +-- .../in-trait/signature-mismatch.next.stderr | 18 +-- .../impl-trait/in-trait/signature-mismatch.rs | 2 +- 4 files changed, 130 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 3048c175e1e4a..6873fa0bf27f3 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -19,7 +19,7 @@ use rustc_middle::ty::{ self, InternalSubsts, Ty, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt, }; use rustc_middle::ty::{GenericParamDefKind, ToPredicate, TyCtxt}; -use rustc_span::Span; +use rustc_span::{Span, DUMMY_SP}; use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _; use rustc_trait_selection::traits::{ @@ -767,8 +767,10 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( // contains `def_id`'s early-bound regions. let id_substs = InternalSubsts::identity_for_item(tcx, def_id); debug!(?id_substs, ?substs); - let map: FxHashMap, ty::GenericArg<'tcx>> = - std::iter::zip(substs, id_substs).collect(); + let map: FxHashMap<_, _> = std::iter::zip(substs, id_substs) + .skip(tcx.generics_of(trait_m.def_id).count()) + .filter_map(|(a, b)| Some((a.as_region()?, b.as_region()?))) + .collect(); debug!(?map); // NOTE(compiler-errors): RPITITs, like all other RPITs, have early-bound @@ -793,25 +795,19 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( // same generics. let num_trait_substs = trait_to_impl_substs.len(); let num_impl_substs = tcx.generics_of(impl_m.container_id(tcx)).params.len(); - let ty = tcx.fold_regions(ty, |region, _| { - match region.kind() { - // Remap all free regions, which correspond to late-bound regions in the function. - ty::ReFree(_) => {} - // Remap early-bound regions as long as they don't come from the `impl` itself. - ty::ReEarlyBound(ebr) if tcx.parent(ebr.def_id) != impl_m.container_id(tcx) => {} - _ => return region, - } - let Some(ty::ReEarlyBound(e)) = map.get(®ion.into()).map(|r| r.expect_region().kind()) - else { - return ty::Region::new_error_with_message(tcx, return_span, "expected ReFree to map to ReEarlyBound") - }; - ty::Region::new_early_bound(tcx, ty::EarlyBoundRegion { - def_id: e.def_id, - name: e.name, - index: (e.index as usize - num_trait_substs + num_impl_substs) as u32, - }) - }); - debug!(%ty); + let ty = match ty.try_fold_with(&mut RemapHiddenTyRegions { + tcx, + map, + num_trait_substs, + num_impl_substs, + def_id, + impl_def_id: impl_m.container_id(tcx), + ty, + return_span, + }) { + Ok(ty) => ty, + Err(guar) => tcx.ty_error(guar), + }; collected_tys.insert(def_id, ty::EarlyBinder::bind(ty)); } Err(err) => { @@ -895,6 +891,97 @@ impl<'tcx> TypeFolder> for ImplTraitInTraitCollector<'_, 'tcx> { } } +struct RemapHiddenTyRegions<'tcx> { + tcx: TyCtxt<'tcx>, + map: FxHashMap, ty::Region<'tcx>>, + num_trait_substs: usize, + num_impl_substs: usize, + def_id: DefId, + impl_def_id: DefId, + ty: Ty<'tcx>, + return_span: Span, +} + +impl<'tcx> ty::FallibleTypeFolder> for RemapHiddenTyRegions<'tcx> { + type Error = ErrorGuaranteed; + + fn interner(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn try_fold_ty(&mut self, t: Ty<'tcx>) -> Result, Self::Error> { + if let ty::Alias(ty::Opaque, ty::AliasTy { substs, def_id, .. }) = *t.kind() { + let mut mapped_substs = Vec::with_capacity(substs.len()); + for (arg, v) in std::iter::zip(substs, self.tcx.variances_of(def_id)) { + mapped_substs.push(match (arg.unpack(), v) { + // Skip uncaptured opaque substs + (ty::GenericArgKind::Lifetime(_), ty::Bivariant) => arg, + _ => arg.try_fold_with(self)?, + }); + } + Ok(self.tcx.mk_opaque(def_id, self.tcx.mk_substs(&mapped_substs))) + } else { + t.try_super_fold_with(self) + } + } + + fn try_fold_region( + &mut self, + region: ty::Region<'tcx>, + ) -> Result, Self::Error> { + match region.kind() { + // Remap all free regions, which correspond to late-bound regions in the function. + ty::ReFree(_) => {} + // Remap early-bound regions as long as they don't come from the `impl` itself, + // in which case we don't really need to renumber them. + ty::ReEarlyBound(ebr) if self.tcx.parent(ebr.def_id) != self.impl_def_id => {} + _ => return Ok(region), + } + + let e = if let Some(region) = self.map.get(®ion) { + if let ty::ReEarlyBound(e) = region.kind() { e } else { bug!() } + } else { + let guar = match region.kind() { + ty::ReEarlyBound(ty::EarlyBoundRegion { def_id, .. }) + | ty::ReFree(ty::FreeRegion { + bound_region: ty::BoundRegionKind::BrNamed(def_id, _), + .. + }) => { + let return_span = if let ty::Alias(ty::Opaque, opaque_ty) = self.ty.kind() { + self.tcx.def_span(opaque_ty.def_id) + } else { + self.return_span + }; + self.tcx + .sess + .struct_span_err( + return_span, + "return type captures more lifetimes than trait definition", + ) + .span_label(self.tcx.def_span(def_id), "this lifetime was captured") + .span_note( + self.tcx.def_span(self.def_id), + "hidden type must only reference lifetimes captured by this impl trait", + ) + .note(format!("hidden type inferred to be `{}`", self.ty)) + .emit() + } + _ => self.tcx.sess.delay_span_bug(DUMMY_SP, "should've been able to remap region"), + }; + return Err(guar); + }; + + Ok(ty::Region::new_early_bound( + self.tcx, + ty::EarlyBoundRegion { + def_id: e.def_id, + name: e.name, + index: (e.index as usize - self.num_trait_substs + self.num_impl_substs) as u32, + }, + )) + } +} + fn report_trait_method_mismatch<'tcx>( infcx: &InferCtxt<'tcx>, mut cause: ObligationCause<'tcx>, diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr b/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr index eba270af7f0ee..2db7dd3421c89 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr @@ -1,16 +1,18 @@ -error: `impl` item signature doesn't match `trait` item signature - --> $DIR/signature-mismatch.rs:17:5 +error: return type captures more lifetimes than trait definition + --> $DIR/signature-mismatch.rs:17:47 | LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; - | ----------------------------------------------------------------- expected `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '3` + | - this lifetime was captured ... LL | fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '2` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: expected signature `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '3` - found signature `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '2` - = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` - = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output +note: hidden type must only reference lifetimes captured by this impl trait + --> $DIR/signature-mismatch.rs:11:40 + | +LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: hidden type inferred to be `impl Future> + '_` error: aborting due to previous error diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr b/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr index eba270af7f0ee..2db7dd3421c89 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr @@ -1,16 +1,18 @@ -error: `impl` item signature doesn't match `trait` item signature - --> $DIR/signature-mismatch.rs:17:5 +error: return type captures more lifetimes than trait definition + --> $DIR/signature-mismatch.rs:17:47 | LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; - | ----------------------------------------------------------------- expected `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '3` + | - this lifetime was captured ... LL | fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '2` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: expected signature `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '3` - found signature `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '2` - = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` - = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output +note: hidden type must only reference lifetimes captured by this impl trait + --> $DIR/signature-mismatch.rs:11:40 + | +LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: hidden type inferred to be `impl Future> + '_` error: aborting due to previous error diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.rs b/tests/ui/impl-trait/in-trait/signature-mismatch.rs index 38c902a97a980..c562441774ec3 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.rs +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.rs @@ -15,7 +15,7 @@ pub struct Struct; impl AsyncTrait for Struct { fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { - //~^ ERROR `impl` item signature doesn't match `trait` item signature + //~^ ERROR return type captures more lifetimes than trait definition async move { buff.to_vec() } } } From 473c88dfb69f95b2e8c5f71ba7f6b7b448d22dc2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 30 Jun 2023 00:27:03 +0000 Subject: [PATCH 07/11] Flip the order of binder instantiation for better diagnostics --- .../src/check/compare_impl_item.rs | 17 +++++++---------- .../in-trait/method-signature-matches.lt.stderr | 6 +++--- .../in-trait/signature-mismatch.current.stderr | 7 ++----- .../in-trait/signature-mismatch.next.stderr | 7 ++----- 4 files changed, 14 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 6873fa0bf27f3..76edeccaf29f5 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -651,11 +651,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( let impl_sig = ocx.normalize( &norm_cause, param_env, - infcx.instantiate_binder_with_fresh_vars( - return_span, - infer::HigherRankedType, - tcx.fn_sig(impl_m.def_id).subst_identity(), - ), + tcx.liberate_late_bound_regions(impl_m.def_id, tcx.fn_sig(impl_m.def_id).subst_identity()), ); impl_sig.error_reported()?; let impl_return_ty = impl_sig.output(); @@ -665,9 +661,10 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( // them with inference variables. // We will use these inference variables to collect the hidden types of RPITITs. let mut collector = ImplTraitInTraitCollector::new(&ocx, return_span, param_env, impl_m_def_id); - let unnormalized_trait_sig = tcx - .liberate_late_bound_regions( - impl_m.def_id, + let unnormalized_trait_sig = infcx + .instantiate_binder_with_fresh_vars( + return_span, + infer::HigherRankedType, tcx.fn_sig(trait_m.def_id).subst(tcx, trait_to_placeholder_substs), ) .fold_with(&mut collector); @@ -760,8 +757,8 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( let mut collected_tys = FxHashMap::default(); for (def_id, (ty, substs)) in collected_types { - match infcx.fully_resolve(ty) { - Ok(ty) => { + match infcx.fully_resolve((ty, substs)) { + Ok((ty, substs)) => { // `ty` contains free regions that we created earlier while liberating the // trait fn signature. However, projection normalization expects `ty` to // contains `def_id`'s early-bound regions. diff --git a/tests/ui/impl-trait/in-trait/method-signature-matches.lt.stderr b/tests/ui/impl-trait/in-trait/method-signature-matches.lt.stderr index f604ada6ac760..239c4b35c7207 100644 --- a/tests/ui/impl-trait/in-trait/method-signature-matches.lt.stderr +++ b/tests/ui/impl-trait/in-trait/method-signature-matches.lt.stderr @@ -5,7 +5,7 @@ LL | fn early<'late, T>(_: &'late ()) {} | - ^^^^^^^^^ | | | | | expected type parameter `T`, found `()` - | | help: change the parameter type to match the trait: `&'early T` + | | help: change the parameter type to match the trait: `&T` | this type parameter | note: type in trait @@ -13,8 +13,8 @@ note: type in trait | LL | fn early<'early, T>(x: &'early T) -> impl Sized; | ^^^^^^^^^ - = note: expected signature `fn(&'early T)` - found signature `fn(&())` + = note: expected signature `fn(&T)` + found signature `fn(&'late ())` error: aborting due to previous error diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr b/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr index 2db7dd3421c89..0d805383da144 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr @@ -1,18 +1,15 @@ error: return type captures more lifetimes than trait definition --> $DIR/signature-mismatch.rs:17:47 | -LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; - | - this lifetime was captured -... LL | fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | -- this lifetime was captured ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: hidden type must only reference lifetimes captured by this impl trait --> $DIR/signature-mismatch.rs:11:40 | LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: hidden type inferred to be `impl Future> + '_` + = note: hidden type inferred to be `impl Future> + 'a` error: aborting due to previous error diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr b/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr index 2db7dd3421c89..0d805383da144 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr @@ -1,18 +1,15 @@ error: return type captures more lifetimes than trait definition --> $DIR/signature-mismatch.rs:17:47 | -LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; - | - this lifetime was captured -... LL | fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | -- this lifetime was captured ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: hidden type must only reference lifetimes captured by this impl trait --> $DIR/signature-mismatch.rs:11:40 | LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: hidden type inferred to be `impl Future> + '_` + = note: hidden type inferred to be `impl Future> + 'a` error: aborting due to previous error From 8ad5ea7b0190256cd7d7c9d8f3f614ecb14796ad Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 30 Jun 2023 00:38:34 +0000 Subject: [PATCH 08/11] Adapt tests from #105258 --- .../signature-mismatch.current.stderr | 52 ++++++++++++++++-- .../in-trait/signature-mismatch.next.stderr | 52 ++++++++++++++++-- .../impl-trait/in-trait/signature-mismatch.rs | 53 +++++++++++++++++++ 3 files changed, 151 insertions(+), 6 deletions(-) diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr b/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr index 0d805383da144..8c9dd403174b7 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.current.stderr @@ -1,15 +1,61 @@ error: return type captures more lifetimes than trait definition - --> $DIR/signature-mismatch.rs:17:47 + --> $DIR/signature-mismatch.rs:36:47 | LL | fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { | -- this lifetime was captured ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: hidden type must only reference lifetimes captured by this impl trait - --> $DIR/signature-mismatch.rs:11:40 + --> $DIR/signature-mismatch.rs:17:40 | LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: hidden type inferred to be `impl Future> + 'a` -error: aborting due to previous error +error: return type captures more lifetimes than trait definition + --> $DIR/signature-mismatch.rs:41:57 + | +LL | fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { + | -- this lifetime was captured ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: hidden type must only reference lifetimes captured by this impl trait + --> $DIR/signature-mismatch.rs:18:57 + | +LL | fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: hidden type inferred to be `impl Future> + 'a` + +error: return type captures more lifetimes than trait definition + --> $DIR/signature-mismatch.rs:49:10 + | +LL | fn async_fn_multiple<'a, 'b>( + | -- this lifetime was captured +... +LL | ) -> impl Future> + Captures2<'a, 'b> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: hidden type must only reference lifetimes captured by this impl trait + --> $DIR/signature-mismatch.rs:20:12 + | +LL | -> impl Future> + Captures<'a>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: hidden type inferred to be `impl Future> + Captures2<'a, 'b>` + +error[E0309]: the parameter type `T` may not live long enough + --> $DIR/signature-mismatch.rs:58:10 + | +LL | ) -> impl Future> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `impl Future>` will meet its required lifetime bounds... + | +note: ...that is required by this bound + --> $DIR/signature-mismatch.rs:25:42 + | +LL | ) -> impl Future> + 'a; + | ^^ +help: consider adding an explicit lifetime bound... + | +LL | fn async_fn_reduce_outlive<'a, 'b, T: 'a>( + | ++++ + +error: aborting due to 4 previous errors +For more information about this error, try `rustc --explain E0309`. diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr b/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr index 0d805383da144..8c9dd403174b7 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.next.stderr @@ -1,15 +1,61 @@ error: return type captures more lifetimes than trait definition - --> $DIR/signature-mismatch.rs:17:47 + --> $DIR/signature-mismatch.rs:36:47 | LL | fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { | -- this lifetime was captured ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: hidden type must only reference lifetimes captured by this impl trait - --> $DIR/signature-mismatch.rs:11:40 + --> $DIR/signature-mismatch.rs:17:40 | LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: hidden type inferred to be `impl Future> + 'a` -error: aborting due to previous error +error: return type captures more lifetimes than trait definition + --> $DIR/signature-mismatch.rs:41:57 + | +LL | fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { + | -- this lifetime was captured ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: hidden type must only reference lifetimes captured by this impl trait + --> $DIR/signature-mismatch.rs:18:57 + | +LL | fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: hidden type inferred to be `impl Future> + 'a` + +error: return type captures more lifetimes than trait definition + --> $DIR/signature-mismatch.rs:49:10 + | +LL | fn async_fn_multiple<'a, 'b>( + | -- this lifetime was captured +... +LL | ) -> impl Future> + Captures2<'a, 'b> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: hidden type must only reference lifetimes captured by this impl trait + --> $DIR/signature-mismatch.rs:20:12 + | +LL | -> impl Future> + Captures<'a>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: hidden type inferred to be `impl Future> + Captures2<'a, 'b>` + +error[E0309]: the parameter type `T` may not live long enough + --> $DIR/signature-mismatch.rs:58:10 + | +LL | ) -> impl Future> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `impl Future>` will meet its required lifetime bounds... + | +note: ...that is required by this bound + --> $DIR/signature-mismatch.rs:25:42 + | +LL | ) -> impl Future> + 'a; + | ^^ +help: consider adding an explicit lifetime bound... + | +LL | fn async_fn_reduce_outlive<'a, 'b, T: 'a>( + | ++++ + +error: aborting due to 4 previous errors +For more information about this error, try `rustc --explain E0309`. diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.rs b/tests/ui/impl-trait/in-trait/signature-mismatch.rs index c562441774ec3..23dd71acecb96 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.rs +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.rs @@ -7,8 +7,27 @@ use std::future::Future; +trait Captures<'a> {} +impl Captures<'_> for T {} + +trait Captures2<'a, 'b> {} +impl Captures2<'_, '_> for T {} + pub trait AsyncTrait { fn async_fn(&self, buff: &[u8]) -> impl Future>; + fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future>; + fn async_fn_multiple<'a>(&'a self, buff: &[u8]) + -> impl Future> + Captures<'a>; + fn async_fn_reduce_outlive<'a, T>( + &'a self, + buff: &[u8], + t: T, + ) -> impl Future> + 'a; + fn async_fn_reduce<'a, T>( + &'a self, + buff: &[u8], + t: T, + ) -> impl Future> + Captures<'a>; } pub struct Struct; @@ -18,6 +37,40 @@ impl AsyncTrait for Struct { //~^ ERROR return type captures more lifetimes than trait definition async move { buff.to_vec() } } + + fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { + //~^ ERROR return type captures more lifetimes than trait definition + async move { buff.to_vec() } + } + + fn async_fn_multiple<'a, 'b>( + &'a self, + buff: &'b [u8], + ) -> impl Future> + Captures2<'a, 'b> { + //~^ ERROR return type captures more lifetimes than trait definition + async move { buff.to_vec() } + } + + fn async_fn_reduce_outlive<'a, 'b, T>( + &'a self, + buff: &'b [u8], + t: T, + ) -> impl Future> { + //~^ ERROR the parameter type `T` may not live long enough + async move { + let _t = t; + vec![] + } + } + + // OK: We remove the `Captures<'a>`, providing a guarantee that we don't capture `'a`, + // but we still fulfill the `Captures<'a>` trait bound. + fn async_fn_reduce<'a, 'b, T>(&'a self, buff: &'b [u8], t: T) -> impl Future> { + async move { + let _t = t; + vec![] + } + } } fn main() {} From 44a8a8d0ca407a08612b87eaff1211dda1528633 Mon Sep 17 00:00:00 2001 From: yukang Date: Fri, 30 Jun 2023 00:38:56 +0800 Subject: [PATCH 09/11] Better messages for next in a iterator inside for loops --- .../src/diagnostics/conflict_errors.rs | 79 ++++++++++++++++++- .../rustc_borrowck/src/diagnostics/mod.rs | 6 ++ tests/ui/suggestions/issue-102972.rs | 16 ++++ tests/ui/suggestions/issue-102972.stderr | 33 ++++++++ 4 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 tests/ui/suggestions/issue-102972.rs create mode 100644 tests/ui/suggestions/issue-102972.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index a068f2d69268b..8a6bdb72968e8 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1,6 +1,5 @@ -use std::iter; - use either::Either; +use hir::PatField; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxIndexSet; use rustc_errors::{ @@ -28,6 +27,8 @@ use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{BytePos, Span, Symbol}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::ObligationCtxt; +use std::iter; +use std::marker::PhantomData; use crate::borrow_set::TwoPhaseActivation; use crate::borrowck_errors; @@ -992,6 +993,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { issued_borrow.borrowed_place, &issued_spans, ); + self.explain_iterator_advancement_in_for_loop_if_applicable( + &mut err, + span, + &issued_spans, + ); err } @@ -1278,6 +1284,75 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } + /// Suggest using `while let` for call `next` on an iterator in a for loop. + /// + /// For example: + /// ```ignore (illustrative) + /// + /// for x in iter { + /// ... + /// iter.next() + /// } + /// ``` + pub(crate) fn explain_iterator_advancement_in_for_loop_if_applicable( + &self, + err: &mut Diagnostic, + span: Span, + issued_spans: &UseSpans<'tcx>, + ) { + let issue_span = issued_spans.args_or_use(); + let tcx = self.infcx.tcx; + let hir = tcx.hir(); + + let Some(body_id) = hir.get(self.mir_hir_id()).body_id() else { return }; + + struct ExprFinder<'hir> { + phantom: PhantomData<&'hir hir::Expr<'hir>>, + issue_span: Span, + expr_span: Span, + found_body_expr: bool, + loop_bind: Option, + } + impl<'hir> Visitor<'hir> for ExprFinder<'hir> { + fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) { + if let hir::ExprKind::Loop(hir::Block{ stmts: [stmt, ..], ..}, _, hir::LoopSource::ForLoop, _) = ex.kind && + let hir::StmtKind::Expr(hir::Expr{ kind: hir::ExprKind::Match(call, [_, bind, ..], _), ..}) = stmt.kind && + let hir::ExprKind::Call(path, _args) = call.kind && + let hir::ExprKind::Path(hir::QPath::LangItem(LangItem::IteratorNext, _, _, )) = path.kind && + let hir::PatKind::Struct(path, [field, ..], _) = bind.pat.kind && + let hir::QPath::LangItem(LangItem::OptionSome, _, _) = path && + let PatField { pat: hir::Pat{ kind: hir::PatKind::Binding(_, _, ident, ..), .. }, ..} = field && + self.issue_span.source_equal(call.span) { + self.loop_bind = Some(ident.name); + } + + if let hir::ExprKind::MethodCall(body_call, ..) = ex.kind && + body_call.ident.name == sym::next && + ex.span.source_equal(self.expr_span) { + self.found_body_expr = true; + } + + hir::intravisit::walk_expr(self, ex); + } + } + let mut finder = ExprFinder { + phantom: PhantomData, + expr_span: span, + issue_span, + loop_bind: None, + found_body_expr: false, + }; + finder.visit_expr(hir.body(body_id).value); + if let Some(loop_bind) = finder.loop_bind && + finder.found_body_expr { + err.note(format!( + "a for loop advances the iterator for you, the result is stored in `{}`.", + loop_bind + )); + err.help("if you want to call `next` on a iterator within the loop, consider using `while let`."); + } + } + /// Suggest using closure argument instead of capture. /// /// For example: diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 2f8c970f806d3..42e50fd0fad07 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -1146,6 +1146,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // Avoid pointing to the same function in multiple different // error messages. if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) { + self.explain_iterator_advancement_in_for_loop_if_applicable( + err, + span, + &move_spans, + ); + let func = tcx.def_path_str(method_did); err.subdiagnostic(CaptureReasonNote::FuncTakeSelf { func, diff --git a/tests/ui/suggestions/issue-102972.rs b/tests/ui/suggestions/issue-102972.rs new file mode 100644 index 0000000000000..106288b054d5d --- /dev/null +++ b/tests/ui/suggestions/issue-102972.rs @@ -0,0 +1,16 @@ +fn test1() { + let mut chars = "Hello".chars(); + for _c in chars.by_ref() { + chars.next(); //~ ERROR cannot borrow `chars` as mutable more than once at a time + } +} + +fn test2() { + let v = vec![1, 2, 3]; + let mut iter = v.iter(); + for _i in iter { + iter.next(); //~ ERROR borrow of moved value: `iter` + } +} + +fn main() { } diff --git a/tests/ui/suggestions/issue-102972.stderr b/tests/ui/suggestions/issue-102972.stderr new file mode 100644 index 0000000000000..03f9dbb6c895a --- /dev/null +++ b/tests/ui/suggestions/issue-102972.stderr @@ -0,0 +1,33 @@ +error[E0499]: cannot borrow `chars` as mutable more than once at a time + --> $DIR/issue-102972.rs:4:9 + | +LL | for _c in chars.by_ref() { + | -------------- + | | + | first mutable borrow occurs here + | first borrow later used here +LL | chars.next(); + | ^^^^^^^^^^^^ second mutable borrow occurs here + | + = note: a for loop advances the iterator for you, the result is stored in `_c`. + = help: if you want to call `next` on a iterator within the loop, consider using `while let`. + +error[E0382]: borrow of moved value: `iter` + --> $DIR/issue-102972.rs:12:9 + | +LL | let mut iter = v.iter(); + | -------- move occurs because `iter` has type `std::slice::Iter<'_, i32>`, which does not implement the `Copy` trait +LL | for _i in iter { + | ---- `iter` moved due to this implicit call to `.into_iter()` +LL | iter.next(); + | ^^^^^^^^^^^ value borrowed here after move + | + = note: a for loop advances the iterator for you, the result is stored in `_i`. + = help: if you want to call `next` on a iterator within the loop, consider using `while let`. +note: `into_iter` takes ownership of the receiver `self`, which moves `iter` + --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0382, E0499. +For more information about an error, try `rustc --explain E0382`. From 4189faa21edc123e2ccfd3b444b0ad3c030aba0a Mon Sep 17 00:00:00 2001 From: yukang Date: Fri, 30 Jun 2023 14:50:27 +0800 Subject: [PATCH 10/11] add typecheck for iterator --- .../src/diagnostics/conflict_errors.rs | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 8a6bdb72968e8..93eef4d5966b4 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -28,7 +28,6 @@ use rustc_span::{BytePos, Span, Symbol}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::ObligationCtxt; use std::iter; -use std::marker::PhantomData; use crate::borrow_set::TwoPhaseActivation; use crate::borrowck_errors; @@ -1305,12 +1304,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let hir = tcx.hir(); let Some(body_id) = hir.get(self.mir_hir_id()).body_id() else { return }; + let typeck_results = tcx.typeck(self.mir_def_id()); struct ExprFinder<'hir> { - phantom: PhantomData<&'hir hir::Expr<'hir>>, issue_span: Span, expr_span: Span, - found_body_expr: bool, + body_expr: Option<&'hir hir::Expr<'hir>>, loop_bind: Option, } impl<'hir> Visitor<'hir> for ExprFinder<'hir> { @@ -1326,30 +1325,28 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { self.loop_bind = Some(ident.name); } - if let hir::ExprKind::MethodCall(body_call, ..) = ex.kind && - body_call.ident.name == sym::next && - ex.span.source_equal(self.expr_span) { - self.found_body_expr = true; + if let hir::ExprKind::MethodCall(body_call, _recv, ..) = ex.kind && + body_call.ident.name == sym::next && ex.span.source_equal(self.expr_span) { + self.body_expr = Some(ex); } hir::intravisit::walk_expr(self, ex); } } - let mut finder = ExprFinder { - phantom: PhantomData, - expr_span: span, - issue_span, - loop_bind: None, - found_body_expr: false, - }; + let mut finder = + ExprFinder { expr_span: span, issue_span, loop_bind: None, body_expr: None }; finder.visit_expr(hir.body(body_id).value); + if let Some(loop_bind) = finder.loop_bind && - finder.found_body_expr { - err.note(format!( - "a for loop advances the iterator for you, the result is stored in `{}`.", - loop_bind - )); - err.help("if you want to call `next` on a iterator within the loop, consider using `while let`."); + let Some(body_expr) = finder.body_expr && + let Some(def_id) = typeck_results.type_dependent_def_id(body_expr.hir_id) && + let Some(trait_did) = tcx.trait_of_item(def_id) && + tcx.is_diagnostic_item(sym::Iterator, trait_did) { + err.note(format!( + "a for loop advances the iterator for you, the result is stored in `{}`.", + loop_bind + )); + err.help("if you want to call `next` on a iterator within the loop, consider using `while let`."); } } From f9a4db731288a64eb9e3aef037128c825157c0b0 Mon Sep 17 00:00:00 2001 From: Bryanskiy Date: Thu, 29 Jun 2023 22:08:04 +0300 Subject: [PATCH 11/11] Fix associated items effective visibility calculation for type privacy lints --- compiler/rustc_privacy/src/lib.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index c3e8d45d20152..055006373efd7 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -2135,16 +2135,18 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> { // lints shouldn't be emmited even if `from` effective visibility // is larger than `Priv` nominal visibility and if `Priv` can leak // in some scenarios due to type inference. - let impl_ev = Some(EffectiveVisibility::of_impl::( + let impl_ev = EffectiveVisibility::of_impl::( item.owner_id.def_id, tcx, self.effective_visibilities, - )); + ); // check that private components do not appear in the generics or predicates of inherent impls // this check is intentionally NOT performed for impls of traits, per #90586 if impl_.of_trait.is_none() { - self.check(item.owner_id.def_id, impl_vis, impl_ev).generics().predicates(); + self.check(item.owner_id.def_id, impl_vis, Some(impl_ev)) + .generics() + .predicates(); } for impl_item_ref in impl_.items { let impl_item_vis = if impl_.of_trait.is_none() { @@ -2159,8 +2161,9 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> { let impl_item_ev = if impl_.of_trait.is_none() { self.get(impl_item_ref.id.owner_id.def_id) + .map(|ev| ev.min(impl_ev, self.tcx)) } else { - impl_ev + Some(impl_ev) }; self.check_assoc_item(