mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add moved-out-of-ref diagnostic

Browse Source
This commit is contained in:
hkalbasi 2023-05-18 19:17:06 +03:30
parent 09d1265e1e
commit b55fbd3ad7
11 changed files with 352 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
crates/hir-ty/src/infer/expr.rs
View File

@ -934,7 +934,18 @@ pub(crate) fn write_fn_trait_method_resolution(
match fn_x {
FnTrait::FnOnce => (),
FnTrait::FnMut => {
if !matches!(derefed_callee.kind(Interner), TyKind::Ref(Mutability::Mut, _, _)) {
if let TyKind::Ref(Mutability::Mut, _, inner) = derefed_callee.kind(Interner) {
if adjustments
.last()
.map(|x| matches!(x.kind, Adjust::Borrow(_)))
.unwrap_or(true)
{
// prefer reborrow to move
adjustments
.push(Adjustment { kind: Adjust::Deref(None), target: inner.clone() });
adjustments.push(Adjustment::borrow(Mutability::Mut, inner.clone()))
}
} else {
adjustments.push(Adjustment::borrow(Mutability::Mut, derefed_callee.clone()));
}
}

118
crates/hir-ty/src/mir/borrowck.rs
View File

@ -5,12 +5,14 @@
use std::iter;
use hir_def::DefWithBodyId;
use hir_def::{DefWithBodyId, HasModule};
use la_arena::ArenaMap;
use stdx::never;
use triomphe::Arc;
use crate::{db::HirDatabase, ClosureId};
use crate::{
db::HirDatabase, mir::Operand, utils::ClosureSubst, ClosureId, Interner, Ty, TyExt, TypeFlags,
};
use super::{
BasicBlockId, BorrowKind, LocalId, MirBody, MirLowerError, MirSpan, Place, ProjectionElem,
@ -24,10 +26,17 @@ pub enum MutabilityReason {
Not,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct MovedOutOfRef {
pub ty: Ty,
pub span: MirSpan,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BorrowckResult {
pub mir_body: Arc<MirBody>,
pub mutability_of_locals: ArenaMap<LocalId, MutabilityReason>,
pub moved_out_of_ref: Vec<MovedOutOfRef>,
}
fn all_mir_bodies(
@ -68,12 +77,115 @@ pub fn borrowck_query(
let r = all_mir_bodies(db, def)
.map(|body| {
let body = body?;
Ok(BorrowckResult { mutability_of_locals: mutability_of_locals(&body), mir_body: body })
Ok(BorrowckResult {
mutability_of_locals: mutability_of_locals(&body),
moved_out_of_ref: moved_out_of_ref(db, &body),
mir_body: body,
})
})
.collect::<Result<Vec<_>, MirLowerError>>()?;
Ok(r.into())
}
fn moved_out_of_ref(db: &dyn HirDatabase, body: &MirBody) -> Vec<MovedOutOfRef> {
let mut result = vec![];
let mut for_operand = |op: &Operand, span: MirSpan| match op {
Operand::Copy(p) | Operand::Move(p) => {
let mut ty: Ty = body.locals[p.local].ty.clone();
let mut is_dereference_of_ref = false;
for proj in &p.projection {
if *proj == ProjectionElem::Deref && ty.as_reference().is_some() {
is_dereference_of_ref = true;
}
ty = proj.projected_ty(
ty,
db,
|c, subst, f| {
let (def, _) = db.lookup_intern_closure(c.into());
let infer = db.infer(def);
let (captures, _) = infer.closure_info(&c);
let parent_subst = ClosureSubst(subst).parent_subst();
captures
.get(f)
.expect("broken closure field")
.ty
.clone()
.substitute(Interner, parent_subst)
},
body.owner.module(db.upcast()).krate(),
);
}
if is_dereference_of_ref
&& !ty.clone().is_copy(db, body.owner)
&& !ty.data(Interner).flags.intersects(TypeFlags::HAS_ERROR)
{
result.push(MovedOutOfRef { span, ty });
}
}
Operand::Constant(_) | Operand::Static(_) => (),
};
for (_, block) in body.basic_blocks.iter() {
for statement in &block.statements {
match &statement.kind {
StatementKind::Assign(_, r) => match r {
Rvalue::ShallowInitBoxWithAlloc(_) => (),
Rvalue::ShallowInitBox(o, _)
| Rvalue::UnaryOp(_, o)
| Rvalue::Cast(_, o, _)
| Rvalue::Repeat(o, _)
| Rvalue::Use(o) => for_operand(o, statement.span),
Rvalue::CopyForDeref(_)
| Rvalue::Discriminant(_)
| Rvalue::Len(_)
| Rvalue::Ref(_, _) => (),
Rvalue::CheckedBinaryOp(_, o1, o2) => {
for_operand(o1, statement.span);
for_operand(o2, statement.span);
}
Rvalue::Aggregate(_, ops) => {
for op in ops {
for_operand(op, statement.span);
}
}
},
StatementKind::Deinit(_)
| StatementKind::StorageLive(_)
| StatementKind::StorageDead(_)
| StatementKind::Nop => (),
}
}
match &block.terminator {
Some(terminator) => match &terminator.kind {
TerminatorKind::SwitchInt { discr, .. } => for_operand(discr, terminator.span),
TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::Goto { .. }
| TerminatorKind::Resume
| TerminatorKind::GeneratorDrop
| TerminatorKind::Abort
| TerminatorKind::Return
| TerminatorKind::Unreachable
| TerminatorKind::Drop { .. } => (),
TerminatorKind::DropAndReplace { value, .. } => {
for_operand(value, terminator.span);
}
TerminatorKind::Call { func, args, .. } => {
for_operand(func, terminator.span);
args.iter().for_each(|x| for_operand(x, terminator.span));
}
TerminatorKind::Assert { cond, .. } => {
for_operand(cond, terminator.span);
}
TerminatorKind::Yield { value, .. } => {
for_operand(value, terminator.span);
}
},
None => (),
}
}
result
}
fn is_place_direct(lvalue: &Place) -> bool {
!lvalue.projection.iter().any(|x| *x == ProjectionElem::Deref)
}

7
crates/hir/src/diagnostics.rs
View File

@ -46,6 +46,7 @@ fn from(d: $diag) -> AnyDiagnostic {
MissingFields,
MissingMatchArms,
MissingUnsafe,
MovedOutOfRef,
NeedMut,
NoSuchField,
PrivateAssocItem,
@ -252,3 +253,9 @@ pub struct NeedMut {
pub struct UnusedMut {
pub local: Local,
}
#[derive(Debug)]
pub struct MovedOutOfRef {
pub ty: Type,
pub span: InFile<SyntaxNodePtr>,
}

29
crates/hir/src/lib.rs
View File

@ -90,10 +90,11 @@
AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncoherentImpl,
IncorrectCase, InvalidDeriveTarget, MacroDefError, MacroError, MacroExpansionParseError,
MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms, MissingUnsafe,
NeedMut, NoSuchField, PrivateAssocItem, PrivateField, ReplaceFilterMapNextWithFindMap,
TypeMismatch, UndeclaredLabel, UnimplementedBuiltinMacro, UnreachableLabel,
UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall,
UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, UnusedMut,
MovedOutOfRef, NeedMut, NoSuchField, PrivateAssocItem, PrivateField,
ReplaceFilterMapNextWithFindMap, TypeMismatch, UndeclaredLabel, UnimplementedBuiltinMacro,
UnreachableLabel, UnresolvedExternCrate, UnresolvedField, UnresolvedImport,
UnresolvedMacroCall, UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro,
UnusedMut,
},
has_source::HasSource,
semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits},
@ -1575,6 +1576,26 @@ pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
if let Ok(borrowck_results) = db.borrowck(self.into()) {
for borrowck_result in borrowck_results.iter() {
let mir_body = &borrowck_result.mir_body;
for moof in &borrowck_result.moved_out_of_ref {
let span: InFile<SyntaxNodePtr> = match moof.span {
mir::MirSpan::ExprId(e) => match source_map.expr_syntax(e) {
Ok(s) => s.map(|x| x.into()),
Err(_) => continue,
},
mir::MirSpan::PatId(p) => match source_map.pat_syntax(p) {
Ok(s) => s.map(|x| match x {
Either::Left(e) => e.into(),
Either::Right(e) => e.into(),
}),
Err(_) => continue,
},
mir::MirSpan::Unknown => continue,
};
acc.push(
MovedOutOfRef { ty: Type::new_for_crate(krate, moof.ty.clone()), span }
.into(),
)
}
let mol = &borrowck_result.mutability_of_locals;
for (binding_id, _) in hir_body.bindings.iter() {
let Some(&local) = mir_body.binding_locals.get(binding_id) else {

2
crates/ide-diagnostics/src/handlers/missing_match_arms.rs
View File

@ -1027,6 +1027,7 @@ fn reference_patterns_at_top_level() {
check_diagnostics(
r#"
//- minicore: copy
fn main() {
match &false {
&true => {}
@ -1041,6 +1042,7 @@ fn reference_patterns_in_fields() {
cov_mark::check_count!(validate_match_bailed_out, 1);
check_diagnostics(
r#"
//- minicore: copy
fn main() {
match (&false,) {
//^^^^^^^^^ error: missing match arm: `(&false,)` not covered

5
crates/ide-diagnostics/src/handlers/missing_unsafe.rs
View File

@ -142,6 +142,8 @@ fn main() {
fn missing_unsafe_diagnostic_with_static_mut() {
check_diagnostics(
r#"
//- minicore: copy
struct Ty {
a: u8,
}
@ -256,6 +258,7 @@ fn main() {
fn add_unsafe_block_when_accessing_mutable_static() {
check_fix(
r#"
//- minicore: copy
struct Ty {
a: u8,
}
@ -374,6 +377,7 @@ fn main() {
fn unsafe_expr_as_right_hand_side_of_assignment() {
check_fix(
r#"
//- minicore: copy
static mut STATIC_MUT: u8 = 0;
fn main() {
@ -396,6 +400,7 @@ fn main() {
fn unsafe_expr_in_binary_plus() {
check_fix(
r#"
//- minicore: copy
static mut STATIC_MUT: u8 = 0;
fn main() {

154
crates/ide-diagnostics/src/handlers/moved_out_of_ref.rs Normal file
View File

@ -0,0 +1,154 @@
use crate::{Diagnostic, DiagnosticsContext};
use hir::HirDisplay;
// Diagnostic: moved-out-of-ref
//
// This diagnostic is triggered on moving non copy things out of references.
pub(crate) fn moved_out_of_ref(ctx: &DiagnosticsContext<'_>, d: &hir::MovedOutOfRef) -> Diagnostic {
Diagnostic::new(
"moved-out-of-ref",
format!("cannot move `{}` out of reference", d.ty.display(ctx.sema.db)),
ctx.sema.diagnostics_display_range(d.span.clone()).range,
)
.experimental() // spans are broken, and I'm not sure how precise we can detect copy types
}
#[cfg(test)]
mod tests {
use crate::tests::check_diagnostics;
// FIXME: spans are broken
#[test]
fn move_by_explicit_deref() {
check_diagnostics(
r#"
struct X;
fn main() {
let a = &X;
let b = *a;
//^ error: cannot move `X` out of reference
}
"#,
);
}
#[test]
fn move_out_of_field() {
check_diagnostics(
r#"
//- minicore: copy
struct X;
struct Y(X, i32);
fn main() {
let a = &Y(X, 5);
let b = a.0;
//^ error: cannot move `X` out of reference
let y = a.1;
}
"#,
);
}
#[test]
fn move_out_of_static() {
check_diagnostics(
r#"
//- minicore: copy
struct X;
fn main() {
static S: X = X;
let s = S;
//^ error: cannot move `X` out of reference
}
"#,
);
}
#[test]
fn generic_types() {
check_diagnostics(
r#"
//- minicore: derive, copy
#[derive(Copy)]
struct X<T>(T);
struct Y;
fn consume<T>(_: X<T>) {
}
fn main() {
let a = &X(Y);
consume(*a);
//^^^^^^^^^^^ error: cannot move `X<Y>` out of reference
let a = &X(5);
consume(*a);
}
"#,
);
}
#[test]
fn no_false_positive_simple() {
check_diagnostics(
r#"
//- minicore: copy
fn f(_: i32) {}
fn main() {
let x = &2;
f(*x);
}
"#,
);
}
#[test]
fn no_false_positive_unknown_type() {
check_diagnostics(
r#"
//- minicore: derive, copy
fn f(x: &Unknown) -> Unknown {
*x
}
#[derive(Copy)]
struct X<T>(T);
struct Y<T>(T);
fn g(x: &X<Unknown>) -> X<Unknown> {
*x
}
fn h(x: &Y<Unknown>) -> Y<Unknown> {
// FIXME: we should show error for this, as `Y` is not copy
// regardless of its generic parameter.
*x
}
"#,
);
}
#[test]
fn no_false_positive_dyn_fn() {
check_diagnostics(
r#"
//- minicore: copy, fn
fn f(x: &mut &mut dyn Fn()) {
x();
}
struct X<'a> {
field: &'a mut dyn Fn(),
}
fn f(x: &mut X<'_>) {
(x.field)();
}
"#,
);
}
}

1
crates/ide-diagnostics/src/handlers/mutability_errors.rs
View File

@ -340,6 +340,7 @@ fn main() {
fn regression_14310() {
check_diagnostics(
r#"
//- minicore: copy, builtin_impls
fn clone(mut i: &!) -> ! {
//^^^^^ 💡 weak: variable does not need to be mutable
*i

2
crates/ide-diagnostics/src/lib.rs
View File

@ -38,6 +38,7 @@ mod handlers {
pub(crate) mod missing_fields;
pub(crate) mod missing_match_arms;
pub(crate) mod missing_unsafe;
pub(crate) mod moved_out_of_ref;
pub(crate) mod mutability_errors;
pub(crate) mod no_such_field;
pub(crate) mod private_assoc_item;
@ -283,6 +284,7 @@ pub fn diagnostics(
AnyDiagnostic::MissingFields(d) => handlers::missing_fields::missing_fields(&ctx, &d),
AnyDiagnostic::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d),
AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d),
AnyDiagnostic::MovedOutOfRef(d) => handlers::moved_out_of_ref::moved_out_of_ref(&ctx, &d),
AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx, &d),
AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d),
AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx, &d),

14
crates/ide-diagnostics/src/tests.rs
View File

@ -121,6 +121,15 @@ pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixtur
})
.collect::<Vec<_>>();
actual.sort_by_key(|(range, _)| range.start());
if expected.is_empty() {
// makes minicore smoke test debugable
for (e, _) in &actual {
eprintln!(
"Code in range {e:?} = {}",
&db.file_text(file_id)[usize::from(e.start())..usize::from(e.end())]
)
}
}
assert_eq!(expected, actual);
}
}
@ -156,6 +165,11 @@ fn check(minicore: MiniCore) {
// Checks that there is no diagnostic in minicore for each flag.
for flag in MiniCore::available_flags() {
if flag == "clone" {
// Clone without copy has `moved-out-of-ref`, so ignoring.
// FIXME: Maybe we should merge copy and clone in a single flag?
continue;
}
eprintln!("Checking minicore flag {flag}");
check(MiniCore::from_flags([flag]));
}

17
crates/test-utils/src/minicore.rs
View File

@ -111,6 +111,7 @@ impl Copy for $t {}
impl<T: ?Sized> Copy for *const T {}
impl<T: ?Sized> Copy for *mut T {}
impl<T: ?Sized> Copy for &T {}
impl Copy for ! {}
}
// endregion:copy
@ -246,6 +247,12 @@ fn clone(&self) -> Self {
f32 f64
bool char
}
impl Clone for ! {
fn clone(&self) {
*self
}
}
// endregion:builtin_impls
// region:derive
@ -319,8 +326,8 @@ fn deref(&self) -> &T {
pub fn drop<T>(_x: T) {}
pub const fn replace<T>(dest: &mut T, src: T) -> T {
unsafe {
let result = *dest;
*dest = src;
let result = crate::ptr::read(dest);
crate::ptr::write(dest, src);
result
}
}
@ -339,6 +346,12 @@ pub mod ptr {
pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
unsafe { drop_in_place(to_drop) }
}
pub const unsafe fn read<T>(src: *const T) -> T {
*src
}
pub const unsafe fn write<T>(dst: *mut T, src: T) {
*dst = src;
}
// endregion:drop
}