Unsafe borrow of packed fields: account for borrow through ref binding, auto ref function calls

5 files changed, 136 insertions, 38 deletions

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<String>,
     ) {
-        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<HighlightModifier> = (|| {
+                        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<RootDatabase>,
+    db: &RootDatabase,
+    def: Definition,
+    name_ref: Option<ast::NameRef>,
+    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<RootDatabas
         _ => 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<HighlightModifier> = (|| {
+                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<RootDatabas
                     })
                 })
                 .unwrap_or(false);
-            return if is_union { h | HighlightModifier::Unsafe } else { h.into() };
+            if is_union { h | HighlightModifier::Unsafe } else { h.into() }
         }
         PATH_SEGMENT => {
             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<RootDatabas
             };
 
             match parent.kind() {
-                CALL_EXPR => 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();
     }
 }
 "#