diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 44456e49e2b..0007d7fa881 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -4,6 +4,7 @@ use std::{iter, sync::Arc}; use arrayvec::ArrayVec; use either::Either; use hir_def::{ + adt::ReprKind, adt::StructKind, adt::VariantData, builtin_type::BuiltinType, @@ -431,6 +432,10 @@ impl Struct { Type::from_def(db, self.id.lookup(db.upcast()).container.module(db.upcast()).krate, self.id) } + pub fn repr(self, db: &dyn HirDatabase) -> Option { + db.struct_data(self.id).repr.clone() + } + fn variant_data(self, db: &dyn HirDatabase) -> Arc { db.struct_data(self.id).variant_data.clone() } @@ -1253,6 +1258,19 @@ impl Type { ) } + pub fn is_packed(&self, db: &dyn HirDatabase) -> bool { + let adt_id = match self.ty.value { + Ty::Apply(ApplicationTy { ctor: TypeCtor::Adt(adt_id), .. }) => adt_id, + _ => return false, + }; + + let adt = adt_id.into(); + match adt { + Adt::Struct(s) => matches!(s.repr(db), Some(ReprKind::Packed)), + _ => false, + } + } + pub fn is_raw_ptr(&self) -> bool { matches!(&self.ty.value, Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. })) } diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index 31f3241c9ed..34b02c5365f 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -49,7 +49,7 @@ pub use hir_def::{ docs::Documentation, nameres::ModuleSource, path::{ModPath, Path, PathKind}, - type_ref::Mutability, + type_ref::{Mutability, TypeRef}, }; pub use hir_expand::{ hygiene::Hygiene, name::Name, HirFileId, InFile, MacroCallId, MacroCallLoc, diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index e392130ab6d..872f5fa4c79 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -25,7 +25,8 @@ use crate::{ semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx}, source_analyzer::{resolve_hir_path, resolve_hir_path_qualifier, SourceAnalyzer}, AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef, - Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, VariantDef, + Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, TypeRef, + VariantDef, }; use resolver::TypeNs; @@ -279,6 +280,18 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { pub fn assert_contains_node(&self, node: &SyntaxNode) { self.imp.assert_contains_node(node) } + + pub fn is_unsafe_method_call(&self, method_call_expr: ast::MethodCallExpr) -> bool { + self.imp.is_unsafe_method_call(method_call_expr) + } + + pub fn is_unsafe_ref_expr(&self, ref_expr: &ast::RefExpr) -> bool { + self.imp.is_unsafe_ref_expr(ref_expr) + } + + pub fn is_unsafe_ident_pat(&self, ident_pat: &ast::IdentPat) -> bool { + self.imp.is_unsafe_ident_pat(ident_pat) + } } impl<'db> SemanticsImpl<'db> { @@ -574,6 +587,90 @@ impl<'db> SemanticsImpl<'db> { }); InFile::new(file_id, node) } + + pub fn is_unsafe_method_call(&self, method_call_expr: ast::MethodCallExpr) -> bool { + method_call_expr + .expr() + .and_then(|expr| { + let field_expr = if let ast::Expr::FieldExpr(field_expr) = expr { + field_expr + } else { + return None; + }; + let ty = self.type_of_expr(&field_expr.expr()?)?; + if !ty.is_packed(self.db) { + return None; + } + + let func = self.resolve_method_call(&method_call_expr).map(Function::from)?; + let is_unsafe = func.has_self_param(self.db) + && matches!(func.params(self.db).first(), Some(TypeRef::Reference(..))); + Some(is_unsafe) + }) + .unwrap_or(false) + } + + pub fn is_unsafe_ref_expr(&self, ref_expr: &ast::RefExpr) -> bool { + ref_expr + .expr() + .and_then(|expr| { + let field_expr = match expr { + ast::Expr::FieldExpr(field_expr) => field_expr, + _ => return None, + }; + let expr = field_expr.expr()?; + self.type_of_expr(&expr) + }) + // Binding a reference to a packed type is possibly unsafe. + .map(|ty| ty.is_packed(self.db)) + .unwrap_or(false) + + // FIXME This needs layout computation to be correct. It will highlight + // more than it should with the current implementation. + } + + pub fn is_unsafe_ident_pat(&self, ident_pat: &ast::IdentPat) -> bool { + if !ident_pat.ref_token().is_some() { + return false; + } + + ident_pat + .syntax() + .parent() + .and_then(|parent| { + // `IdentPat` can live under `RecordPat` directly under `RecordPatField` or + // `RecordPatFieldList`. `RecordPatField` also lives under `RecordPatFieldList`, + // so this tries to lookup the `IdentPat` anywhere along that structure to the + // `RecordPat` so we can get the containing type. + let record_pat = ast::RecordPatField::cast(parent.clone()) + .and_then(|record_pat| record_pat.syntax().parent()) + .or_else(|| Some(parent.clone())) + .and_then(|parent| { + ast::RecordPatFieldList::cast(parent)? + .syntax() + .parent() + .and_then(ast::RecordPat::cast) + }); + + // If this doesn't match a `RecordPat`, fallback to a `LetStmt` to see if + // this is initialized from a `FieldExpr`. + if let Some(record_pat) = record_pat { + self.type_of_pat(&ast::Pat::RecordPat(record_pat)) + } else if let Some(let_stmt) = ast::LetStmt::cast(parent) { + let field_expr = match let_stmt.initializer()? { + ast::Expr::FieldExpr(field_expr) => field_expr, + _ => return None, + }; + + self.type_of_expr(&field_expr.expr()?) + } else { + None + } + }) + // Binding a reference to a packed type is possibly unsafe. + .map(|ty| ty.is_packed(self.db)) + .unwrap_or(false) + } } pub trait ToDef: AstNode + Clone { diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 6cb56a1cd00..35c3a914025 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -9,11 +9,12 @@ use hir_expand::{ }; use ra_arena::{map::ArenaMap, Arena}; use ra_syntax::ast::{self, NameOwner, VisibilityOwner}; +use tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree}; use crate::{ body::{CfgExpander, LowerCtx}, db::DefDatabase, - item_tree::{Field, Fields, ItemTree}, + item_tree::{AttrOwner, Field, Fields, ItemTree, ModItem}, src::HasChildSource, src::HasSource, trace::Trace, @@ -29,6 +30,7 @@ use ra_cfg::CfgOptions; pub struct StructData { pub name: Name, pub variant_data: Arc, + pub repr: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -58,26 +60,58 @@ pub struct FieldData { pub visibility: RawVisibility, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ReprKind { + Packed, + Other, +} + +fn repr_from_value(item_tree: &ItemTree, of: AttrOwner) -> Option { + item_tree.attrs(of).by_key("repr").tt_values().find_map(parse_repr_tt) +} + +fn parse_repr_tt(tt: &Subtree) -> Option { + match tt.delimiter { + Some(Delimiter { kind: DelimiterKind::Parenthesis, .. }) => {} + _ => return None, + } + + let mut it = tt.token_trees.iter(); + match it.next()? { + TokenTree::Leaf(Leaf::Ident(ident)) if ident.text == "packed" => Some(ReprKind::Packed), + _ => Some(ReprKind::Other), + } +} + impl StructData { pub(crate) fn struct_data_query(db: &dyn DefDatabase, id: StructId) -> Arc { let loc = id.lookup(db); let item_tree = db.item_tree(loc.id.file_id); + let repr = repr_from_value(&item_tree, ModItem::from(loc.id.value).into()); let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone(); let strukt = &item_tree[loc.id.value]; let variant_data = lower_fields(&item_tree, &cfg_options, &strukt.fields); - - Arc::new(StructData { name: strukt.name.clone(), variant_data: Arc::new(variant_data) }) + Arc::new(StructData { + name: strukt.name.clone(), + variant_data: Arc::new(variant_data), + repr, + }) } pub(crate) fn union_data_query(db: &dyn DefDatabase, id: UnionId) -> Arc { let loc = id.lookup(db); let item_tree = db.item_tree(loc.id.file_id); + let repr = repr_from_value(&item_tree, ModItem::from(loc.id.value).into()); let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone(); let union = &item_tree[loc.id.value]; let variant_data = lower_fields(&item_tree, &cfg_options, &union.fields); - Arc::new(StructData { name: union.name.clone(), variant_data: Arc::new(variant_data) }) + Arc::new(StructData { + name: union.name.clone(), + variant_data: Arc::new(variant_data), + repr, + }) } } diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs index 6b7874460a2..c10e15db8b6 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(), }, @@ -566,9 +566,20 @@ fn highlight_element( } } p if p.is_punct() => match p { - T![::] | T![->] | T![=>] | T![&] | T![..] | T![=] | T![@] => { - HighlightTag::Operator.into() + T![&] => { + let h = HighlightTag::Operator.into(); + let is_unsafe = element + .parent() + .and_then(ast::RefExpr::cast) + .map(|ref_expr| sema.is_unsafe_ref_expr(&ref_expr)) + .unwrap_or(false); + if is_unsafe { + h | HighlightModifier::Unsafe + } else { + h + } } + T![::] | T![->] | T![=>] | T![..] | T![=] | T![@] => HighlightTag::Operator.into(), T![!] if element.parent().and_then(ast::MacroCall::cast).is_some() => { HighlightTag::Macro.into() } @@ -649,6 +660,18 @@ fn highlight_element( HighlightTag::SelfKeyword.into() } } + T![ref] => element + .parent() + .and_then(ast::IdentPat::cast) + .and_then(|ident_pat| { + if sema.is_unsafe_ident_pat(&ident_pat) { + Some(HighlightModifier::Unsafe) + } else { + None + } + }) + .map(|modifier| h | modifier) + .unwrap_or(h), _ => h, } } @@ -678,7 +701,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) => { @@ -697,6 +726,15 @@ 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 is_unsafe = name_ref + .and_then(|name_ref| name_ref.syntax().parent()) + .and_then(ast::MethodCallExpr::cast) + .map(|method_call_expr| sema.is_unsafe_method_call(method_call_expr)) + .unwrap_or(false); + if is_unsafe { + h |= HighlightModifier::Unsafe; + } } return h; } @@ -768,8 +806,18 @@ 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 is_unsafe = ast::MethodCallExpr::cast(parent) + .map(|method_call_expr| sema.is_unsafe_method_call(method_call_expr)) + .unwrap_or(false); + if is_unsafe { + h |= HighlightModifier::Unsafe; + } + + h + } FIELD_EXPR => { let h = HighlightTag::Field; let is_union = ast::FieldExpr::cast(parent) @@ -782,7 +830,11 @@ fn highlight_name_ref_by_syntax(name: ast::NameRef, sema: &Semantics { let path = match parent.parent().and_then(ast::Path::cast) { @@ -807,18 +859,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 09062c38e75..a8087635a87 100644 --- a/crates/ra_ide/src/syntax_highlighting/tests.rs +++ b/crates/ra_ide/src/syntax_highlighting/tests.rs @@ -292,10 +292,24 @@ struct TypeForStaticMut { static mut global_mut: TypeForStaticMut = TypeForStaticMut { a: 0 }; +#[repr(packed)] +struct Packed { + a: u16, +} + +trait DoTheAutoref { + fn calls_autoref(&self); +} + +impl DoTheAutoref for u16 { + fn calls_autoref(&self) {} +} + fn main() { - let x = &5 as *const usize; + 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 { @@ -303,9 +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 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(); } } "# diff --git a/crates/ra_ide/test_data/highlight_unsafe.html b/crates/ra_ide/test_data/highlight_unsafe.html index 79409fe816b..552fea66892 100644 --- a/crates/ra_ide/test_data/highlight_unsafe.html +++ b/crates/ra_ide/test_data/highlight_unsafe.html @@ -54,10 +54,24 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd static mut global_mut: TypeForStaticMut = TypeForStaticMut { a: 0 }; +#[repr(packed)] +struct Packed { + a: u16, +} + +trait DoTheAutoref { + fn calls_autoref(&self); +} + +impl DoTheAutoref for u16 { + fn calls_autoref(&self) {} +} + fn main() { - let x = &5 as *const usize; + 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 { @@ -65,8 +79,21 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd 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 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(); } } \ No newline at end of file