From d5f11e530dbf6edbdd0ca32d6cd5fafe634c8c4a Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sat, 27 Jun 2020 17:11:43 -0400 Subject: Unsafe borrow of packed fields: account for borrow through ref binding, auto ref function calls --- crates/ra_ide/src/completion/complete_dot.rs | 2 +- .../ra_ide/src/completion/complete_trait_impl.rs | 2 +- crates/ra_ide/src/completion/presentation.rs | 2 +- crates/ra_ide/src/syntax_highlighting.rs | 137 ++++++++++++++++++--- crates/ra_ide/src/syntax_highlighting/tests.rs | 31 ++--- 5 files changed, 136 insertions(+), 38 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs index 532665285..5488db43f 100644 --- a/crates/ra_ide/src/completion/complete_dot.rs +++ b/crates/ra_ide/src/completion/complete_dot.rs @@ -48,7 +48,7 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: &T let mut seen_methods = FxHashSet::default(); let traits_in_scope = ctx.scope.traits_in_scope(); receiver.iterate_method_candidates(ctx.db, krate, &traits_in_scope, None, |_ty, func| { - if func.has_self_param(ctx.db) + if func.self_param(ctx.db).is_some() && ctx.scope.module().map_or(true, |m| func.is_visible_from(ctx.db, m)) && seen_methods.insert(func.name(ctx.db)) { diff --git a/crates/ra_ide/src/completion/complete_trait_impl.rs b/crates/ra_ide/src/completion/complete_trait_impl.rs index d9a0ef167..e3ba7ebc4 100644 --- a/crates/ra_ide/src/completion/complete_trait_impl.rs +++ b/crates/ra_ide/src/completion/complete_trait_impl.rs @@ -136,7 +136,7 @@ fn add_function_impl( .lookup_by(fn_name) .set_documentation(func.docs(ctx.db)); - let completion_kind = if func.has_self_param(ctx.db) { + let completion_kind = if func.self_param(ctx.db).is_some() { CompletionItemKind::Method } else { CompletionItemKind::Function diff --git a/crates/ra_ide/src/completion/presentation.rs b/crates/ra_ide/src/completion/presentation.rs index 9a94ff476..fc3d1a4bd 100644 --- a/crates/ra_ide/src/completion/presentation.rs +++ b/crates/ra_ide/src/completion/presentation.rs @@ -191,7 +191,7 @@ impl Completions { func: hir::Function, local_name: Option, ) { - let has_self_param = func.has_self_param(ctx.db); + let has_self_param = func.self_param(ctx.db).is_some(); let name = local_name.unwrap_or_else(|| func.name(ctx.db).to_string()); let ast_node = func.source(ctx.db).value; diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs index a4a7aa228..454fef39c 100644 --- a/crates/ra_ide/src/syntax_highlighting.rs +++ b/crates/ra_ide/src/syntax_highlighting.rs @@ -497,9 +497,9 @@ fn highlight_element( match name_kind { Some(NameClass::ExternCrate(_)) => HighlightTag::Module.into(), Some(NameClass::Definition(def)) => { - highlight_name(db, def, false) | HighlightModifier::Definition + highlight_name(sema, db, def, None, false) | HighlightModifier::Definition } - Some(NameClass::ConstReference(def)) => highlight_name(db, def, false), + Some(NameClass::ConstReference(def)) => highlight_name(sema, db, def, None, false), Some(NameClass::FieldShorthand { field, .. }) => { let mut h = HighlightTag::Field.into(); if let Definition::Field(field) = field { @@ -532,7 +532,7 @@ fn highlight_element( binding_hash = Some(calc_binding_hash(&name, *shadow_count)) } }; - highlight_name(db, def, possibly_unsafe) + highlight_name(sema, db, def, Some(name_ref), possibly_unsafe) } NameRefClass::FieldShorthand { .. } => HighlightTag::Field.into(), }, @@ -565,8 +565,8 @@ fn highlight_element( _ => h, } } - REF_EXPR => { - let ref_expr = element.into_node().and_then(ast::RefExpr::cast)?; + T![&] => { + let ref_expr = element.parent().and_then(ast::RefExpr::cast)?; let expr = ref_expr.expr()?; let field_expr = match expr { ast::Expr::FieldExpr(fe) => fe, @@ -668,6 +668,52 @@ fn highlight_element( HighlightTag::SelfKeyword.into() } } + T![ref] => { + let modifier: Option = (|| { + let bind_pat = element.parent().and_then(ast::BindPat::cast)?; + let parent = bind_pat.syntax().parent()?; + + let ty = if let Some(pat_list) = + ast::RecordFieldPatList::cast(parent.clone()) + { + let record_pat = + pat_list.syntax().parent().and_then(ast::RecordPat::cast)?; + sema.type_of_pat(&ast::Pat::RecordPat(record_pat)) + } else if let Some(let_stmt) = ast::LetStmt::cast(parent.clone()) { + let field_expr = + if let ast::Expr::FieldExpr(field_expr) = let_stmt.initializer()? { + field_expr + } else { + return None; + }; + + sema.type_of_expr(&field_expr.expr()?) + } else if let Some(record_field_pat) = ast::RecordFieldPat::cast(parent) { + let record_pat = record_field_pat + .syntax() + .parent() + .and_then(ast::RecordFieldPatList::cast)? + .syntax() + .parent() + .and_then(ast::RecordPat::cast)?; + sema.type_of_pat(&ast::Pat::RecordPat(record_pat)) + } else { + None + }?; + + if !ty.is_packed(db) { + return None; + } + + Some(HighlightModifier::Unsafe) + })(); + + if let Some(modifier) = modifier { + h | modifier + } else { + h + } + } _ => h, } } @@ -697,7 +743,13 @@ fn is_child_of_impl(element: &SyntaxElement) -> bool { } } -fn highlight_name(db: &RootDatabase, def: Definition, possibly_unsafe: bool) -> Highlight { +fn highlight_name( + sema: &Semantics, + db: &RootDatabase, + def: Definition, + name_ref: Option, + possibly_unsafe: bool, +) -> Highlight { match def { Definition::Macro(_) => HighlightTag::Macro, Definition::Field(field) => { @@ -716,6 +768,29 @@ fn highlight_name(db: &RootDatabase, def: Definition, possibly_unsafe: bool) -> let mut h = HighlightTag::Function.into(); if func.is_unsafe(db) { h |= HighlightModifier::Unsafe; + } else { + (|| { + let method_call_expr = + name_ref?.syntax().parent().and_then(ast::MethodCallExpr::cast)?; + let expr = method_call_expr.expr()?; + let field_expr = if let ast::Expr::FieldExpr(field_expr) = expr { + Some(field_expr) + } else { + None + }?; + let ty = sema.type_of_expr(&field_expr.expr()?)?; + if !ty.is_packed(db) { + return None; + } + + let func = sema.resolve_method_call(&method_call_expr)?; + if func.self_param(db)?.is_ref { + Some(HighlightModifier::Unsafe) + } else { + None + } + })() + .map(|modifier| h |= modifier); } return h; } @@ -787,8 +862,33 @@ fn highlight_name_ref_by_syntax(name: ast::NameRef, sema: &Semantics return default.into(), }; - let tag = match parent.kind() { - METHOD_CALL_EXPR => HighlightTag::Function, + match parent.kind() { + METHOD_CALL_EXPR => { + let mut h = Highlight::new(HighlightTag::Function); + let modifier: Option = (|| { + let method_call_expr = ast::MethodCallExpr::cast(parent)?; + let expr = method_call_expr.expr()?; + let field_expr = if let ast::Expr::FieldExpr(field_expr) = expr { + field_expr + } else { + return None; + }; + + let expr = field_expr.expr()?; + let ty = sema.type_of_expr(&expr)?; + if ty.is_packed(sema.db) { + Some(HighlightModifier::Unsafe) + } else { + None + } + })(); + + if let Some(modifier) = modifier { + h |= modifier; + } + + h + } FIELD_EXPR => { let h = HighlightTag::Field; let is_union = ast::FieldExpr::cast(parent) @@ -801,7 +901,7 @@ fn highlight_name_ref_by_syntax(name: ast::NameRef, sema: &Semantics { let path = match parent.parent().and_then(ast::Path::cast) { @@ -826,18 +926,15 @@ fn highlight_name_ref_by_syntax(name: ast::NameRef, sema: &Semantics HighlightTag::Function, - _ => { - if name.text().chars().next().unwrap_or_default().is_uppercase() { - HighlightTag::Struct - } else { - HighlightTag::Constant - } + CALL_EXPR => HighlightTag::Function.into(), + _ => if name.text().chars().next().unwrap_or_default().is_uppercase() { + HighlightTag::Struct.into() + } else { + HighlightTag::Constant } + .into(), } } - _ => default, - }; - - tag.into() + _ => default.into(), + } } diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs index a7f5ad862..a8087635a 100644 --- a/crates/ra_ide/src/syntax_highlighting/tests.rs +++ b/crates/ra_ide/src/syntax_highlighting/tests.rs @@ -301,16 +301,7 @@ trait DoTheAutoref { fn calls_autoref(&self); } -struct NeedsAlign { - a: u16 -} - -#[repr(packed)] -struct HasAligned { - a: NeedsAlign -} - -impl DoTheAutoref for NeedsAlign { +impl DoTheAutoref for u16 { fn calls_autoref(&self) {} } @@ -318,6 +309,7 @@ fn main() { let x = &5 as *const _ as *const usize; let u = Union { b: 0 }; unsafe { + // unsafe fn and method calls unsafe_fn(); let b = u.b; match u { @@ -325,13 +317,22 @@ fn main() { Union { a } => (), } HasUnsafeFn.unsafe_method(); - let _y = *(x); - let z = -x; + + // unsafe deref + let y = *x; + + // unsafe access to a static mut let a = global_mut.a; + + // unsafe ref of packed fields let packed = Packed { a: 0 }; - let _a = &packed.a; - let h = HasAligned{ a: NeedsAlign { a: 1 } }; - h.a.calls_autoref(); + let a = &packed.a; + let ref a = packed.a; + let Packed { ref a } = packed; + let Packed { a: ref _a } = packed; + + // unsafe auto ref of packed field + packed.a.calls_autoref(); } } "# -- cgit v1.2.3