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

This commit is contained in:
Paul Daniel Faria 2020-06-27 17:11:43 -04:00
parent 38440d53d8
commit d5f11e530d
10 changed files with 156 additions and 48 deletions

View File

@ -11,6 +11,7 @@
docs::Documentation,
expr::{BindingAnnotation, Pat, PatId},
import_map,
item_tree::SelfParam,
per_ns::PerNs,
resolver::{HasResolver, Resolver},
src::HasSource as _,
@ -670,8 +671,8 @@ pub fn name(self, db: &dyn HirDatabase) -> Name {
db.function_data(self.id).name.clone()
}
pub fn has_self_param(self, db: &dyn HirDatabase) -> bool {
db.function_data(self.id).has_self_param
pub fn self_param(self, db: &dyn HirDatabase) -> Option<SelfParam> {
db.function_data(self.id).self_param
}
pub fn params(self, db: &dyn HirDatabase) -> Vec<TypeRef> {

View File

@ -10,7 +10,7 @@
attr::Attrs,
body::Expander,
db::DefDatabase,
item_tree::{AssocItem, ItemTreeId, ModItem},
item_tree::{AssocItem, ItemTreeId, ModItem, SelfParam},
type_ref::{TypeBound, TypeRef},
visibility::RawVisibility,
AssocContainerId, AssocItemId, ConstId, ConstLoc, FunctionId, FunctionLoc, HasModule, ImplId,
@ -25,7 +25,7 @@ pub struct FunctionData {
pub attrs: Attrs,
/// True if the first param is `self`. This is relevant to decide whether this
/// can be called as a method.
pub has_self_param: bool,
pub self_param: Option<SelfParam>,
pub is_unsafe: bool,
pub is_varargs: bool,
pub visibility: RawVisibility,
@ -42,7 +42,7 @@ pub(crate) fn fn_data_query(db: &dyn DefDatabase, func: FunctionId) -> Arc<Funct
params: func.params.to_vec(),
ret_type: func.ret_type.clone(),
attrs: item_tree.attrs(ModItem::from(loc.id.value).into()).clone(),
has_self_param: func.has_self_param,
self_param: func.self_param,
is_unsafe: func.is_unsafe,
is_varargs: func.is_varargs,
visibility: item_tree[func.visibility].clone(),

View File

@ -500,7 +500,7 @@ pub struct Function {
pub name: Name,
pub visibility: RawVisibilityId,
pub generic_params: GenericParamsId,
pub has_self_param: bool,
pub self_param: Option<SelfParam>,
pub is_unsafe: bool,
pub params: Box<[TypeRef]>,
pub is_varargs: bool,
@ -508,6 +508,12 @@ pub struct Function {
pub ast_id: FileAstId<ast::Fn>,
}
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub struct SelfParam {
pub is_ref: bool,
pub is_mut: bool,
}
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct Struct {
pub name: Name,

View File

@ -283,7 +283,7 @@ fn lower_function(&mut self, func: &ast::Fn) -> Option<FileItemTreeId<Function>>
let name = func.name()?.as_name();
let mut params = Vec::new();
let mut has_self_param = false;
let mut func_self_param = None;
if let Some(param_list) = func.param_list() {
if let Some(self_param) = param_list.self_param() {
let self_type = match self_param.ty() {
@ -302,7 +302,10 @@ fn lower_function(&mut self, func: &ast::Fn) -> Option<FileItemTreeId<Function>>
}
};
params.push(self_type);
has_self_param = true;
func_self_param = Some(SelfParam {
is_ref: self_param.amp_token().is_some(),
is_mut: self_param.mut_token().is_some(),
});
}
for param in param_list.params() {
let type_ref = TypeRef::from_ast_opt(&self.body_ctx, param.ty());
@ -335,7 +338,7 @@ fn lower_function(&mut self, func: &ast::Fn) -> Option<FileItemTreeId<Function>>
name,
visibility,
generic_params: GenericParamsId::EMPTY,
has_self_param,
self_param: func_self_param,
is_unsafe: func.unsafe_token().is_some(),
params: params.into_boxed_slice(),
is_varargs,

View File

@ -640,7 +640,7 @@ fn is_valid_candidate(
}
}
if let Some(receiver_ty) = receiver_ty {
if !data.has_self_param {
if data.self_param.is_none() {
return false;
}
let transformed_receiver_ty = match transform_receiver_ty(db, m, self_ty) {

View File

@ -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))
{

View File

@ -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

View File

@ -191,7 +191,7 @@ pub(crate) fn add_function(
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;

View File

@ -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(),
}
}

View File

@ -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();
}
}
"#