From 99f6cb102e8f7cbdb2eb737e030cd9a22ff4e357 Mon Sep 17 00:00:00 2001 From: sharnoff Date: Thu, 24 Nov 2022 16:56:21 -0800 Subject: [PATCH 1/3] Add a note for implicit temporaries on &mut (fn or const) --- .../src/diagnostics/conflict_errors.rs | 26 ++++++++++++++++++- compiler/rustc_middle/src/mir/mod.rs | 2 ++ compiler/rustc_middle/src/thir.rs | 4 +-- compiler/rustc_middle/src/thir/visit.rs | 2 +- .../src/build/custom/parse/instruction.rs | 2 +- .../src/build/expr/as_constant.rs | 2 +- .../src/build/expr/as_place.rs | 2 +- .../src/build/expr/as_rvalue.rs | 2 +- .../rustc_mir_build/src/build/expr/as_temp.rs | 3 +++ .../src/build/expr/category.rs | 2 +- .../rustc_mir_build/src/build/expr/into.rs | 2 +- .../rustc_mir_build/src/check_unsafety.rs | 2 +- compiler/rustc_mir_build/src/thir/cx/expr.rs | 4 +-- compiler/rustc_ty_utils/src/consts.rs | 2 +- 14 files changed, 43 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 6658ee89ad6f1..aeeae7249f125 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1836,6 +1836,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { "value referencing" }; + let mut implicit_temporary = None; + let (place_desc, note) = if let Some(place_desc) = opt_place_desc { let local_kind = if let Some(local) = borrow.borrowed_place.as_local() { match self.body.local_kind(local) { @@ -1861,7 +1863,22 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let root_place = self.prefixes(borrow.borrowed_place.as_ref(), PrefixSet::All).last().unwrap(); let local = root_place.local; - match self.body.local_kind(local) { + let local_kind = self.body.local_kind(local); + if let LocalKind::Temp = local_kind { + // Mutable references to constants and functions will implicitly create + // temporaries, even though immutable references don't. We use these below to add a + // note about this. + match &self.body.local_decls[local].local_info { + Some(box LocalInfo::FnDefRef) => { + implicit_temporary = Some(("function", "function pointer")); + } + Some(box LocalInfo::ConstRef { .. }) => { + implicit_temporary = Some(("constant", "value")); + } + _ => {} + } + } + match local_kind { LocalKind::ReturnPointer | LocalKind::Temp => { ("temporary value".to_string(), "temporary value created here".to_string()) } @@ -1906,6 +1923,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } + if let Some((item_name, value_desc)) = implicit_temporary { + err.note(format!( + "mutably borrowing a {} implicitly copies the {} to a temporary", + item_name, value_desc + )); + } + Some(err) } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 14bdff4568f5e..011f36bc21472 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -900,6 +900,8 @@ pub enum LocalInfo<'tcx> { StaticRef { def_id: DefId, is_thread_local: bool }, /// A temporary created that references the const with the given `DefId` ConstRef { def_id: DefId }, + /// A temporary created that references a function + FnDefRef, /// A temporary created during the creation of an aggregate /// (e.g. a temporary for `foo` in `MyStruct { my_field: foo }`) AggregateTemp, diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 5f320708c8416..11f71d8fbc131 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -432,8 +432,8 @@ pub enum ExprKind<'tcx> { lit: ty::ScalarInt, user_ty: UserTy<'tcx>, }, - /// A literal of a ZST type. - ZstLiteral { + /// Named functions, associated functions, and constructors (including `Self(...)`) + NamedFn { user_ty: UserTy<'tcx>, }, /// Associated constants and named constants diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index 79a0e75aa7c78..aba96f0c9dbc3 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -136,7 +136,7 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp }) => {} Literal { lit: _, neg: _ } => {} NonHirLiteral { lit: _, user_ty: _ } => {} - ZstLiteral { user_ty: _ } => {} + NamedFn { user_ty: _ } => {} NamedConst { def_id: _, substs: _, user_ty: _ } => {} ConstParam { param: _, def_id: _ } => {} StaticRef { alloc_id: _, ty: _, def_id: _ } => {} diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index dca4906c07de5..6b3838ad96b91 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -153,7 +153,7 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> { ExprKind::Literal { .. } | ExprKind::NamedConst { .. } | ExprKind::NonHirLiteral { .. } - | ExprKind::ZstLiteral { .. } + | ExprKind::NamedFn { .. } | ExprKind::ConstParam { .. } | ExprKind::ConstBlock { .. } => { Ok(Operand::Constant(Box::new( diff --git a/compiler/rustc_mir_build/src/build/expr/as_constant.rs b/compiler/rustc_mir_build/src/build/expr/as_constant.rs index 1d96893c7a3ea..403010b435430 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_constant.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_constant.rs @@ -68,7 +68,7 @@ pub fn as_constant_inner<'tcx>( Constant { span, user_ty, literal } } - ExprKind::ZstLiteral { ref user_ty } => { + ExprKind::NamedFn { ref user_ty } => { let user_ty = user_ty.as_ref().map(push_cuta).flatten(); let literal = ConstantKind::Val(ConstValue::ZeroSized, ty); 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 e22fa6365dcb4..536f10671256a 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -554,7 +554,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Literal { .. } | ExprKind::NamedConst { .. } | ExprKind::NonHirLiteral { .. } - | ExprKind::ZstLiteral { .. } + | ExprKind::NamedFn { .. } | ExprKind::ConstParam { .. } | ExprKind::ConstBlock { .. } | ExprKind::StaticRef { .. } diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index c7b3eb44dc5fb..118ce91e18228 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -459,7 +459,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ExprKind::Literal { .. } | ExprKind::NamedConst { .. } | ExprKind::NonHirLiteral { .. } - | ExprKind::ZstLiteral { .. } + | ExprKind::NamedFn { .. } | ExprKind::ConstParam { .. } | ExprKind::ConstBlock { .. } | ExprKind::StaticRef { .. } => { diff --git a/compiler/rustc_mir_build/src/build/expr/as_temp.rs b/compiler/rustc_mir_build/src/build/expr/as_temp.rs index 0ca4e37451995..22f1519a3fafe 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_temp.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_temp.rs @@ -70,6 +70,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ExprKind::NamedConst { def_id, .. } | ExprKind::ConstParam { def_id, .. } => { local_decl.local_info = Some(Box::new(LocalInfo::ConstRef { def_id })); } + ExprKind::NamedFn { .. } => { + local_decl.local_info = Some(Box::new(LocalInfo::FnDefRef)); + } _ => {} } this.local_decls.push(local_decl) diff --git a/compiler/rustc_mir_build/src/build/expr/category.rs b/compiler/rustc_mir_build/src/build/expr/category.rs index d33401f07645e..03bdfc45a988d 100644 --- a/compiler/rustc_mir_build/src/build/expr/category.rs +++ b/compiler/rustc_mir_build/src/build/expr/category.rs @@ -72,7 +72,7 @@ impl Category { ExprKind::ConstBlock { .. } | ExprKind::Literal { .. } | ExprKind::NonHirLiteral { .. } - | ExprKind::ZstLiteral { .. } + | ExprKind::NamedFn { .. } | ExprKind::ConstParam { .. } | ExprKind::StaticRef { .. } | ExprKind::NamedConst { .. } => Some(Category::Constant), diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 38b1fa91d0a67..6854a6bcd1783 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -554,7 +554,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Literal { .. } | ExprKind::NamedConst { .. } | ExprKind::NonHirLiteral { .. } - | ExprKind::ZstLiteral { .. } + | ExprKind::NamedFn { .. } | ExprKind::ConstParam { .. } | ExprKind::ThreadLocalRef(_) | ExprKind::StaticRef { .. } => { diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 03a7f2d70faeb..ebe9dcae5943d 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -295,7 +295,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { | ExprKind::Literal { .. } | ExprKind::NamedConst { .. } | ExprKind::NonHirLiteral { .. } - | ExprKind::ZstLiteral { .. } + | ExprKind::NamedFn { .. } | ExprKind::ConstParam { .. } | ExprKind::ConstBlock { .. } | ExprKind::Deref { .. } diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 261b95ba95b0e..0b826d8bfe4c7 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -822,7 +822,7 @@ impl<'tcx> Cx<'tcx> { ) } }; - Expr { temp_lifetime, ty, span, kind: ExprKind::ZstLiteral { user_ty } } + Expr { temp_lifetime, ty, span, kind: ExprKind::NamedFn { user_ty } } } fn convert_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) -> ArmId { @@ -851,7 +851,7 @@ impl<'tcx> Cx<'tcx> { | Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(_) => { let user_ty = self.user_substs_applied_to_res(expr.hir_id, res); - ExprKind::ZstLiteral { user_ty } + ExprKind::NamedFn { user_ty } } Res::Def(DefKind::ConstParam, def_id) => { diff --git a/compiler/rustc_ty_utils/src/consts.rs b/compiler/rustc_ty_utils/src/consts.rs index a9b4e1420ea0d..e214b435c4550 100644 --- a/compiler/rustc_ty_utils/src/consts.rs +++ b/compiler/rustc_ty_utils/src/consts.rs @@ -127,7 +127,7 @@ fn recurse_build<'tcx>( let val = ty::ValTree::from_scalar_int(lit); tcx.mk_const(val, node.ty) } - &ExprKind::ZstLiteral { user_ty: _ } => { + &ExprKind::NamedFn { user_ty: _ } => { let val = ty::ValTree::zst(); tcx.mk_const(val, node.ty) } From da49674d1aa5beb4acf5eb101e1c55affacc4220 Mon Sep 17 00:00:00 2001 From: sharnoff Date: Fri, 25 Nov 2022 12:48:36 -0800 Subject: [PATCH 2/3] Remove unnecessary "implicitly" in error message --- compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index aeeae7249f125..f8cc13ab894b6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1925,7 +1925,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let Some((item_name, value_desc)) = implicit_temporary { err.note(format!( - "mutably borrowing a {} implicitly copies the {} to a temporary", + "mutably borrowing a {} copies the {} to a temporary", item_name, value_desc )); } From 3818536ebf1670e8b0094cb794cbe9b1815c5513 Mon Sep 17 00:00:00 2001 From: sharnoff Date: Fri, 25 Nov 2022 13:21:35 -0800 Subject: [PATCH 3/3] Add tests for "mutably borrowing copies into a temporary" --- tests/ui/borrowck/return-borrow-mut-const.rs | 34 ++++++++++++++++++ .../borrowck/return-borrow-mut-const.stderr | 35 +++++++++++++++++++ tests/ui/borrowck/return-borrow-mut-fn.rs | 31 ++++++++++++++++ tests/ui/borrowck/return-borrow-mut-fn.stderr | 35 +++++++++++++++++++ 4 files changed, 135 insertions(+) create mode 100644 tests/ui/borrowck/return-borrow-mut-const.rs create mode 100644 tests/ui/borrowck/return-borrow-mut-const.stderr create mode 100644 tests/ui/borrowck/return-borrow-mut-fn.rs create mode 100644 tests/ui/borrowck/return-borrow-mut-fn.stderr diff --git a/tests/ui/borrowck/return-borrow-mut-const.rs b/tests/ui/borrowck/return-borrow-mut-const.rs new file mode 100644 index 0000000000000..497b40131918f --- /dev/null +++ b/tests/ui/borrowck/return-borrow-mut-const.rs @@ -0,0 +1,34 @@ +// See PR #104857 for details + +// don't want to tie this test to the lint, even though it's related +#![allow(const_item_mutation)] + +fn main() {} + +const X: i32 = 42; + +fn borrow_const_immut() -> &'static i32 { + &X +} + +fn borrow_const_immut_explicit_return() -> &'static i32 { + return &X; +} + +fn borrow_const_immut_into_temp() -> &'static i32 { + let x_ref = &X; + x_ref +} + +fn borrow_const_mut() -> &'static mut i32 { + return &mut X; //~ ERROR +} + +fn borrow_const_mut_explicit_return() -> &'static mut i32 { + return &mut X; //~ ERROR +} + +fn borrow_const_mut_into_temp() -> &'static mut i32 { + let x_ref = &mut X; + x_ref //~ ERROR +} diff --git a/tests/ui/borrowck/return-borrow-mut-const.stderr b/tests/ui/borrowck/return-borrow-mut-const.stderr new file mode 100644 index 0000000000000..e9aca721cc831 --- /dev/null +++ b/tests/ui/borrowck/return-borrow-mut-const.stderr @@ -0,0 +1,35 @@ +error[E0515]: cannot return reference to temporary value + --> $DIR/return-borrow-mut-const.rs:24:12 + | +LL | return &mut X; + | ^^^^^- + | | | + | | temporary value created here + | returns a reference to data owned by the current function + | + = note: mutably borrowing a constant copies the value to a temporary + +error[E0515]: cannot return reference to temporary value + --> $DIR/return-borrow-mut-const.rs:28:12 + | +LL | return &mut X; + | ^^^^^- + | | | + | | temporary value created here + | returns a reference to data owned by the current function + | + = note: mutably borrowing a constant copies the value to a temporary + +error[E0515]: cannot return value referencing temporary value + --> $DIR/return-borrow-mut-const.rs:33:5 + | +LL | let x_ref = &mut X; + | - temporary value created here +LL | x_ref + | ^^^^^ returns a value referencing data owned by the current function + | + = note: mutably borrowing a constant copies the value to a temporary + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0515`. diff --git a/tests/ui/borrowck/return-borrow-mut-fn.rs b/tests/ui/borrowck/return-borrow-mut-fn.rs new file mode 100644 index 0000000000000..54a9c8fd5526a --- /dev/null +++ b/tests/ui/borrowck/return-borrow-mut-fn.rs @@ -0,0 +1,31 @@ +// See PR #104857 for details + +fn main() {} + +fn do_nothing() {} + +fn borrow_fn_immut() -> &'static dyn Fn() { + &do_nothing +} + +fn borrow_fn_immut_explicit_return() -> &'static dyn Fn() { + &do_nothing +} + +fn borrow_fn_immut_into_temp() -> &'static dyn Fn() { + let f = &do_nothing; + f +} + +fn borrow_fn_mut() -> &'static mut dyn FnMut() { + &mut do_nothing //~ ERROR +} + +fn borrow_fn_mut_explicit_return() -> &'static mut dyn FnMut() { + &mut do_nothing //~ ERROR +} + +fn borrow_fn_mut_into_temp() -> &'static mut dyn FnMut() { + let f = &mut do_nothing; + f //~ ERROR +} diff --git a/tests/ui/borrowck/return-borrow-mut-fn.stderr b/tests/ui/borrowck/return-borrow-mut-fn.stderr new file mode 100644 index 0000000000000..9b3bb8717ed46 --- /dev/null +++ b/tests/ui/borrowck/return-borrow-mut-fn.stderr @@ -0,0 +1,35 @@ +error[E0515]: cannot return reference to temporary value + --> $DIR/return-borrow-mut-fn.rs:21:5 + | +LL | &mut do_nothing + | ^^^^^---------- + | | | + | | temporary value created here + | returns a reference to data owned by the current function + | + = note: mutably borrowing a function copies the function pointer to a temporary + +error[E0515]: cannot return reference to temporary value + --> $DIR/return-borrow-mut-fn.rs:25:5 + | +LL | &mut do_nothing + | ^^^^^---------- + | | | + | | temporary value created here + | returns a reference to data owned by the current function + | + = note: mutably borrowing a function copies the function pointer to a temporary + +error[E0515]: cannot return value referencing temporary value + --> $DIR/return-borrow-mut-fn.rs:30:5 + | +LL | let f = &mut do_nothing; + | ---------- temporary value created here +LL | f + | ^ returns a value referencing data owned by the current function + | + = note: mutably borrowing a function copies the function pointer to a temporary + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0515`.