Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add a note about implicit temporaries on &mut (fn or const) #104857

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it actually doesn't put the function pointer into a temporary, but creates a temporary of the function item type (a zst). Maybe it could literally mention the zero sized nature, as that should make it obvious that you get no data to mutate (in case someone expected a 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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you already mentioned, this message is the misleading part. Maybe pick a different message here for ConstRef and FnDefRef instead of adding a new note?

}
Expand Down Expand Up @@ -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 {} copies the {} to a temporary",
item_name, value_desc
));
}

Some(err)
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: _ } => {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/as_constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/as_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. }
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. } => {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_mir_build/src/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/category.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. } => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. }
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
34 changes: 34 additions & 0 deletions tests/ui/borrowck/return-borrow-mut-const.rs
Original file line number Diff line number Diff line change
@@ -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
}
35 changes: 35 additions & 0 deletions tests/ui/borrowck/return-borrow-mut-const.stderr
Original file line number Diff line number Diff line change
@@ -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`.
31 changes: 31 additions & 0 deletions tests/ui/borrowck/return-borrow-mut-fn.rs
Original file line number Diff line number Diff line change
@@ -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
}
35 changes: 35 additions & 0 deletions tests/ui/borrowck/return-borrow-mut-fn.stderr
Original file line number Diff line number Diff line change
@@ -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`.