From 263f9a7f231a474dd56d02adbcd7c57d079e88fd Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Wed, 3 Jun 2020 23:38:25 -0400 Subject: Add tracking of packed repr, use it to highlight unsafe refs Taking a reference to a misaligned field on a packed struct is an unsafe operation. Highlight that behavior. Currently, the misaligned part isn't tracked, so this highlight is a bit too aggressive. --- crates/ra_hir_def/src/adt.rs | 56 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 6cb56a1cd..6d59c8642 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -9,11 +9,13 @@ 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::{ + attr::AttrInput, 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 +31,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 +61,71 @@ 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).iter().find_map(|a| { + if a.path.segments[0].to_string() == "repr" { + if let Some(AttrInput::TokenTree(subtree)) = &a.input { + parse_repr_tt(subtree) + } else { + None + } + } else { + None + } + }) +} + +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() { + None => None, + Some(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, + }) } } -- cgit v1.2.3 From 38440d53d8329ac9f3f2013c6e32b3f69b069c72 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sat, 27 Jun 2020 14:42:42 -0400 Subject: Cleanup repr check, fix packed repr check and test --- crates/ra_hir_def/src/adt.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 6d59c8642..4ba694480 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -12,10 +12,11 @@ use ra_syntax::ast::{self, NameOwner, VisibilityOwner}; use tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree}; use crate::{ - attr::AttrInput, + attr::{Attr, AttrInput}, body::{CfgExpander, LowerCtx}, db::DefDatabase, item_tree::{AttrOwner, Field, Fields, ItemTree, ModItem}, + path::{ModPath, PathKind}, src::HasChildSource, src::HasSource, trace::Trace, @@ -69,8 +70,12 @@ pub enum ReprKind { fn repr_from_value(item_tree: &ItemTree, of: AttrOwner) -> Option { item_tree.attrs(of).iter().find_map(|a| { - if a.path.segments[0].to_string() == "repr" { - if let Some(AttrInput::TokenTree(subtree)) = &a.input { + if let Attr { + path: ModPath { kind: PathKind::Plain, segments }, + input: Some(AttrInput::TokenTree(subtree)), + } = a + { + if segments.len() == 1 && segments[0].to_string() == "repr" { parse_repr_tt(subtree) } else { None -- cgit v1.2.3 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_hir_def/src/data.rs | 6 +++--- crates/ra_hir_def/src/item_tree.rs | 8 +++++++- crates/ra_hir_def/src/item_tree/lower.rs | 9 ++++++--- 3 files changed, 16 insertions(+), 7 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index 88a8ef9bf..2a26b0183 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/crates/ra_hir_def/src/data.rs @@ -10,7 +10,7 @@ use crate::{ 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, pub is_unsafe: bool, pub is_varargs: bool, pub visibility: RawVisibility, @@ -42,7 +42,7 @@ impl FunctionData { 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(), diff --git a/crates/ra_hir_def/src/item_tree.rs b/crates/ra_hir_def/src/item_tree.rs index a67e75dac..1eaea66e4 100644 --- a/crates/ra_hir_def/src/item_tree.rs +++ b/crates/ra_hir_def/src/item_tree.rs @@ -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, pub is_unsafe: bool, pub params: Box<[TypeRef]>, pub is_varargs: bool, @@ -508,6 +508,12 @@ pub struct Function { pub ast_id: FileAstId, } +#[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, diff --git a/crates/ra_hir_def/src/item_tree/lower.rs b/crates/ra_hir_def/src/item_tree/lower.rs index 450ef8798..89ad91d37 100644 --- a/crates/ra_hir_def/src/item_tree/lower.rs +++ b/crates/ra_hir_def/src/item_tree/lower.rs @@ -283,7 +283,7 @@ impl Ctx { 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 @@ impl Ctx { } }; 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 @@ impl Ctx { 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, -- cgit v1.2.3 From c5cc24cb312c70159e63315ea49769b575e8cb65 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 28 Jun 2020 16:04:00 -0400 Subject: Revert function structs back to using bool to track self param, use first param for self information in syntax highlighting instead --- crates/ra_hir_def/src/data.rs | 6 +++--- crates/ra_hir_def/src/item_tree.rs | 8 +------- crates/ra_hir_def/src/item_tree/lower.rs | 9 +++------ 3 files changed, 7 insertions(+), 16 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index 2a26b0183..88a8ef9bf 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/crates/ra_hir_def/src/data.rs @@ -10,7 +10,7 @@ use crate::{ attr::Attrs, body::Expander, db::DefDatabase, - item_tree::{AssocItem, ItemTreeId, ModItem, SelfParam}, + item_tree::{AssocItem, ItemTreeId, ModItem}, 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 self_param: Option, + pub has_self_param: bool, pub is_unsafe: bool, pub is_varargs: bool, pub visibility: RawVisibility, @@ -42,7 +42,7 @@ impl FunctionData { params: func.params.to_vec(), ret_type: func.ret_type.clone(), attrs: item_tree.attrs(ModItem::from(loc.id.value).into()).clone(), - self_param: func.self_param, + has_self_param: func.has_self_param, is_unsafe: func.is_unsafe, is_varargs: func.is_varargs, visibility: item_tree[func.visibility].clone(), diff --git a/crates/ra_hir_def/src/item_tree.rs b/crates/ra_hir_def/src/item_tree.rs index 1eaea66e4..a67e75dac 100644 --- a/crates/ra_hir_def/src/item_tree.rs +++ b/crates/ra_hir_def/src/item_tree.rs @@ -500,7 +500,7 @@ pub struct Function { pub name: Name, pub visibility: RawVisibilityId, pub generic_params: GenericParamsId, - pub self_param: Option, + pub has_self_param: bool, pub is_unsafe: bool, pub params: Box<[TypeRef]>, pub is_varargs: bool, @@ -508,12 +508,6 @@ pub struct Function { pub ast_id: FileAstId, } -#[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, diff --git a/crates/ra_hir_def/src/item_tree/lower.rs b/crates/ra_hir_def/src/item_tree/lower.rs index 89ad91d37..450ef8798 100644 --- a/crates/ra_hir_def/src/item_tree/lower.rs +++ b/crates/ra_hir_def/src/item_tree/lower.rs @@ -283,7 +283,7 @@ impl Ctx { let name = func.name()?.as_name(); let mut params = Vec::new(); - let mut func_self_param = None; + let mut has_self_param = false; 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,10 +302,7 @@ impl Ctx { } }; params.push(self_type); - func_self_param = Some(SelfParam { - is_ref: self_param.amp_token().is_some(), - is_mut: self_param.mut_token().is_some(), - }); + has_self_param = true; } for param in param_list.params() { let type_ref = TypeRef::from_ast_opt(&self.body_ctx, param.ty()); @@ -338,7 +335,7 @@ impl Ctx { name, visibility, generic_params: GenericParamsId::EMPTY, - self_param: func_self_param, + has_self_param, is_unsafe: func.unsafe_token().is_some(), params: params.into_boxed_slice(), is_varargs, -- cgit v1.2.3 From 08182aa9fad4021e60cdc80ee0a578929507e115 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 19 Jul 2020 11:45:46 -0400 Subject: Move unsafe packed ref logic to Semantics, use `Attrs::by_key` to simplify repr attr lookup --- crates/ra_hir_def/src/adt.rs | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 4ba694480..35c3a9140 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -12,11 +12,9 @@ use ra_syntax::ast::{self, NameOwner, VisibilityOwner}; use tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree}; use crate::{ - attr::{Attr, AttrInput}, body::{CfgExpander, LowerCtx}, db::DefDatabase, item_tree::{AttrOwner, Field, Fields, ItemTree, ModItem}, - path::{ModPath, PathKind}, src::HasChildSource, src::HasSource, trace::Trace, @@ -69,21 +67,7 @@ pub enum ReprKind { } fn repr_from_value(item_tree: &ItemTree, of: AttrOwner) -> Option { - item_tree.attrs(of).iter().find_map(|a| { - if let Attr { - path: ModPath { kind: PathKind::Plain, segments }, - input: Some(AttrInput::TokenTree(subtree)), - } = a - { - if segments.len() == 1 && segments[0].to_string() == "repr" { - parse_repr_tt(subtree) - } else { - None - } - } else { - None - } - }) + item_tree.attrs(of).by_key("repr").tt_values().find_map(parse_repr_tt) } fn parse_repr_tt(tt: &Subtree) -> Option { @@ -93,11 +77,8 @@ fn parse_repr_tt(tt: &Subtree) -> Option { } let mut it = tt.token_trees.iter(); - match it.next() { - None => None, - Some(TokenTree::Leaf(Leaf::Ident(ident))) if ident.text == "packed" => { - Some(ReprKind::Packed) - } + match it.next()? { + TokenTree::Leaf(Leaf::Ident(ident)) if ident.text == "packed" => Some(ReprKind::Packed), _ => Some(ReprKind::Other), } } -- cgit v1.2.3