5684: Semantic highlighting for unsafe union field access r=jonas-schievink a=Nashenas88

This change adds support for unions in inference and lowering, then extends on that to add the unsafe semantic modifier on field access only. The `is_possibly_unsafe` function in `syntax_highlighting.rs` could be extended to support fns and static muts so that their definitions are not highlighted as unsafe, but only their usage.

Also, each commit of this PR updates the tests. By reviewing the files by commit, it's easy to see how the changes in the code affected the tests.

Co-authored-by: Paul Daniel Faria <Nashenas88@users.noreply.github.com>
This commit is contained in:
bors[bot] 2020-08-08 16:45:37 +00:00 committed by GitHub
commit 8a57afe5a4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 122 additions and 20 deletions

View file

@ -440,6 +440,12 @@ impl<'a> InferenceContext<'a> {
let ty = self.insert_type_vars(ty.subst(&substs)); let ty = self.insert_type_vars(ty.subst(&substs));
forbid_unresolved_segments((ty, Some(strukt.into())), unresolved) forbid_unresolved_segments((ty, Some(strukt.into())), unresolved)
} }
TypeNs::AdtId(AdtId::UnionId(u)) => {
let substs = Ty::substs_from_path(&ctx, path, u.into(), true);
let ty = self.db.ty(u.into());
let ty = self.insert_type_vars(ty.subst(&substs));
forbid_unresolved_segments((ty, Some(u.into())), unresolved)
}
TypeNs::EnumVariantId(var) => { TypeNs::EnumVariantId(var) => {
let substs = Ty::substs_from_path(&ctx, path, var.into(), true); let substs = Ty::substs_from_path(&ctx, path, var.into(), true);
let ty = self.db.ty(var.parent.into()); let ty = self.db.ty(var.parent.into());
@ -490,10 +496,7 @@ impl<'a> InferenceContext<'a> {
// FIXME potentially resolve assoc type // FIXME potentially resolve assoc type
(Ty::Unknown, None) (Ty::Unknown, None)
} }
TypeNs::AdtId(AdtId::EnumId(_)) TypeNs::AdtId(AdtId::EnumId(_)) | TypeNs::BuiltinType(_) | TypeNs::TraitId(_) => {
| TypeNs::AdtId(AdtId::UnionId(_))
| TypeNs::BuiltinType(_)
| TypeNs::TraitId(_) => {
// FIXME diagnostic // FIXME diagnostic
(Ty::Unknown, None) (Ty::Unknown, None)
} }

View file

@ -518,6 +518,7 @@ impl Ty {
let (segment, generic_def) = match resolved { let (segment, generic_def) = match resolved {
ValueTyDefId::FunctionId(it) => (last, Some(it.into())), ValueTyDefId::FunctionId(it) => (last, Some(it.into())),
ValueTyDefId::StructId(it) => (last, Some(it.into())), ValueTyDefId::StructId(it) => (last, Some(it.into())),
ValueTyDefId::UnionId(it) => (last, Some(it.into())),
ValueTyDefId::ConstId(it) => (last, Some(it.into())), ValueTyDefId::ConstId(it) => (last, Some(it.into())),
ValueTyDefId::StaticId(_) => (last, None), ValueTyDefId::StaticId(_) => (last, None),
ValueTyDefId::EnumVariantId(var) => { ValueTyDefId::EnumVariantId(var) => {
@ -1148,11 +1149,12 @@ impl_from!(BuiltinType, AdtId(StructId, EnumId, UnionId), TypeAliasId for TyDefI
pub enum ValueTyDefId { pub enum ValueTyDefId {
FunctionId(FunctionId), FunctionId(FunctionId),
StructId(StructId), StructId(StructId),
UnionId(UnionId),
EnumVariantId(EnumVariantId), EnumVariantId(EnumVariantId),
ConstId(ConstId), ConstId(ConstId),
StaticId(StaticId), StaticId(StaticId),
} }
impl_from!(FunctionId, StructId, EnumVariantId, ConstId, StaticId for ValueTyDefId); impl_from!(FunctionId, StructId, UnionId, EnumVariantId, ConstId, StaticId for ValueTyDefId);
/// Build the declared type of an item. This depends on the namespace; e.g. for /// Build the declared type of an item. This depends on the namespace; e.g. for
/// `struct Foo(usize)`, we have two types: The type of the struct itself, and /// `struct Foo(usize)`, we have two types: The type of the struct itself, and
@ -1179,6 +1181,7 @@ pub(crate) fn value_ty_query(db: &dyn HirDatabase, def: ValueTyDefId) -> Binders
match def { match def {
ValueTyDefId::FunctionId(it) => type_for_fn(db, it), ValueTyDefId::FunctionId(it) => type_for_fn(db, it),
ValueTyDefId::StructId(it) => type_for_struct_constructor(db, it), ValueTyDefId::StructId(it) => type_for_struct_constructor(db, it),
ValueTyDefId::UnionId(it) => type_for_adt(db, it.into()),
ValueTyDefId::EnumVariantId(it) => type_for_enum_variant_constructor(db, it), ValueTyDefId::EnumVariantId(it) => type_for_enum_variant_constructor(db, it),
ValueTyDefId::ConstId(it) => type_for_const(db, it), ValueTyDefId::ConstId(it) => type_for_const(db, it),
ValueTyDefId::StaticId(it) => type_for_static(db, it), ValueTyDefId::StaticId(it) => type_for_static(db, it),

View file

@ -334,16 +334,44 @@ fn infer_union() {
bar: f32, bar: f32,
} }
fn test() {
let u = MyUnion { foo: 0 };
unsafe { baz(u); }
let u = MyUnion { bar: 0.0 };
unsafe { baz(u); }
}
unsafe fn baz(u: MyUnion) { unsafe fn baz(u: MyUnion) {
let inner = u.foo; let inner = u.foo;
let inner = u.bar;
} }
"#, "#,
expect![[r#" expect![[r#"
61..62 'u': MyUnion 57..172 '{ ...); } }': ()
73..99 '{ ...foo; }': () 67..68 'u': MyUnion
83..88 'inner': u32 71..89 'MyUnio...o: 0 }': MyUnion
91..92 'u': MyUnion 86..87 '0': u32
91..96 'u.foo': u32 95..113 'unsafe...(u); }': ()
102..113 '{ baz(u); }': ()
104..107 'baz': fn baz(MyUnion)
104..110 'baz(u)': ()
108..109 'u': MyUnion
122..123 'u': MyUnion
126..146 'MyUnio... 0.0 }': MyUnion
141..144 '0.0': f32
152..170 'unsafe...(u); }': ()
159..170 '{ baz(u); }': ()
161..164 'baz': fn baz(MyUnion)
161..167 'baz(u)': ()
165..166 'u': MyUnion
188..189 'u': MyUnion
200..249 '{ ...bar; }': ()
210..215 'inner': u32
218..219 'u': MyUnion
218..223 'u.foo': u32
233..238 'inner': f32
241..242 'u': MyUnion
241..246 'u.bar': f32
"#]], "#]],
); );
} }

View file

@ -4,7 +4,7 @@ mod injection;
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
use hir::{Name, Semantics}; use hir::{Name, Semantics, VariantDef};
use ra_ide_db::{ use ra_ide_db::{
defs::{classify_name, classify_name_ref, Definition, NameClass, NameRefClass}, defs::{classify_name, classify_name_ref, Definition, NameClass, NameRefClass},
RootDatabase, RootDatabase,
@ -455,6 +455,18 @@ fn macro_call_range(macro_call: &ast::MacroCall) -> Option<TextRange> {
Some(TextRange::new(range_start, range_end)) Some(TextRange::new(range_start, range_end))
} }
fn is_possibly_unsafe(name_ref: &ast::NameRef) -> bool {
name_ref
.syntax()
.parent()
.and_then(|parent| {
ast::FieldExpr::cast(parent.clone())
.map(|_| true)
.or_else(|| ast::RecordPatField::cast(parent).map(|_| true))
})
.unwrap_or(false)
}
fn highlight_element( fn highlight_element(
sema: &Semantics<RootDatabase>, sema: &Semantics<RootDatabase>,
bindings_shadow_count: &mut FxHashMap<Name, u32>, bindings_shadow_count: &mut FxHashMap<Name, u32>,
@ -484,10 +496,19 @@ fn highlight_element(
match name_kind { match name_kind {
Some(NameClass::Definition(def)) => { Some(NameClass::Definition(def)) => {
highlight_name(db, def) | HighlightModifier::Definition highlight_name(db, def, false) | HighlightModifier::Definition
}
Some(NameClass::ConstReference(def)) => highlight_name(db, def, false),
Some(NameClass::FieldShorthand { field, .. }) => {
let mut h = HighlightTag::Field.into();
if let Definition::Field(field) = field {
if let VariantDef::Union(_) = field.parent_def(db) {
h |= HighlightModifier::Unsafe;
}
}
h
} }
Some(NameClass::ConstReference(def)) => highlight_name(db, def),
Some(NameClass::FieldShorthand { .. }) => HighlightTag::Field.into(),
None => highlight_name_by_syntax(name) | HighlightModifier::Definition, None => highlight_name_by_syntax(name) | HighlightModifier::Definition,
} }
} }
@ -498,6 +519,7 @@ fn highlight_element(
} }
NAME_REF => { NAME_REF => {
let name_ref = element.into_node().and_then(ast::NameRef::cast).unwrap(); let name_ref = element.into_node().and_then(ast::NameRef::cast).unwrap();
let possibly_unsafe = is_possibly_unsafe(&name_ref);
match classify_name_ref(sema, &name_ref) { match classify_name_ref(sema, &name_ref) {
Some(name_kind) => match name_kind { Some(name_kind) => match name_kind {
NameRefClass::Definition(def) => { NameRefClass::Definition(def) => {
@ -508,11 +530,13 @@ fn highlight_element(
binding_hash = Some(calc_binding_hash(&name, *shadow_count)) binding_hash = Some(calc_binding_hash(&name, *shadow_count))
} }
}; };
highlight_name(db, def) highlight_name(db, def, possibly_unsafe)
} }
NameRefClass::FieldShorthand { .. } => HighlightTag::Field.into(), NameRefClass::FieldShorthand { .. } => HighlightTag::Field.into(),
}, },
None if syntactic_name_ref_highlighting => highlight_name_ref_by_syntax(name_ref), None if syntactic_name_ref_highlighting => {
highlight_name_ref_by_syntax(name_ref, sema)
}
None => HighlightTag::UnresolvedReference.into(), None => HighlightTag::UnresolvedReference.into(),
} }
} }
@ -652,10 +676,19 @@ fn is_child_of_impl(element: &SyntaxElement) -> bool {
} }
} }
fn highlight_name(db: &RootDatabase, def: Definition) -> Highlight { fn highlight_name(db: &RootDatabase, def: Definition, possibly_unsafe: bool) -> Highlight {
match def { match def {
Definition::Macro(_) => HighlightTag::Macro, Definition::Macro(_) => HighlightTag::Macro,
Definition::Field(_) => HighlightTag::Field, Definition::Field(field) => {
let mut h = HighlightTag::Field.into();
if possibly_unsafe {
if let VariantDef::Union(_) = field.parent_def(db) {
h |= HighlightModifier::Unsafe;
}
}
return h;
}
Definition::ModuleDef(def) => match def { Definition::ModuleDef(def) => match def {
hir::ModuleDef::Module(_) => HighlightTag::Module, hir::ModuleDef::Module(_) => HighlightTag::Module,
hir::ModuleDef::Function(func) => { hir::ModuleDef::Function(func) => {
@ -725,7 +758,7 @@ fn highlight_name_by_syntax(name: ast::Name) -> Highlight {
tag.into() tag.into()
} }
fn highlight_name_ref_by_syntax(name: ast::NameRef) -> Highlight { fn highlight_name_ref_by_syntax(name: ast::NameRef, sema: &Semantics<RootDatabase>) -> Highlight {
let default = HighlightTag::UnresolvedReference; let default = HighlightTag::UnresolvedReference;
let parent = match name.syntax().parent() { let parent = match name.syntax().parent() {
@ -735,7 +768,20 @@ fn highlight_name_ref_by_syntax(name: ast::NameRef) -> Highlight {
let tag = match parent.kind() { let tag = match parent.kind() {
METHOD_CALL_EXPR => HighlightTag::Function, METHOD_CALL_EXPR => HighlightTag::Function,
FIELD_EXPR => HighlightTag::Field, FIELD_EXPR => {
let h = HighlightTag::Field;
let is_union = ast::FieldExpr::cast(parent)
.and_then(|field_expr| {
let field = sema.resolve_field(&field_expr)?;
Some(if let VariantDef::Union(_) = field.parent_def(sema.db) {
true
} else {
false
})
})
.unwrap_or(false);
return if is_union { h | HighlightModifier::Unsafe } else { h.into() };
}
PATH_SEGMENT => { PATH_SEGMENT => {
let path = match parent.parent().and_then(ast::Path::cast) { let path = match parent.parent().and_then(ast::Path::cast) {
Some(it) => it, Some(it) => it,

View file

@ -275,6 +275,11 @@ fn test_unsafe_highlighting() {
r#" r#"
unsafe fn unsafe_fn() {} unsafe fn unsafe_fn() {}
union Union {
a: u32,
b: f32,
}
struct HasUnsafeFn; struct HasUnsafeFn;
impl HasUnsafeFn { impl HasUnsafeFn {
@ -289,8 +294,14 @@ static mut global_mut: TypeForStaticMut = TypeForStaticMut { a: 0 };
fn main() { fn main() {
let x = &5 as *const usize; let x = &5 as *const usize;
let u = Union { b: 0 };
unsafe { unsafe {
unsafe_fn(); unsafe_fn();
let b = u.b;
match u {
Union { b: 0 } => (),
Union { a } => (),
}
HasUnsafeFn.unsafe_method(); HasUnsafeFn.unsafe_method();
let y = *(x); let y = *(x);
let z = -x; let z = -x;

View file

@ -37,6 +37,11 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
</style> </style>
<pre><code><span class="keyword unsafe">unsafe</span> <span class="keyword">fn</span> <span class="function declaration unsafe">unsafe_fn</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="punctuation">{</span><span class="punctuation">}</span> <pre><code><span class="keyword unsafe">unsafe</span> <span class="keyword">fn</span> <span class="function declaration unsafe">unsafe_fn</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="punctuation">{</span><span class="punctuation">}</span>
<span class="keyword">union</span> <span class="union declaration">Union</span> <span class="punctuation">{</span>
<span class="field declaration">a</span><span class="punctuation">:</span> <span class="builtin_type">u32</span><span class="punctuation">,</span>
<span class="field declaration">b</span><span class="punctuation">:</span> <span class="builtin_type">f32</span><span class="punctuation">,</span>
<span class="punctuation">}</span>
<span class="keyword">struct</span> <span class="struct declaration">HasUnsafeFn</span><span class="punctuation">;</span> <span class="keyword">struct</span> <span class="struct declaration">HasUnsafeFn</span><span class="punctuation">;</span>
<span class="keyword">impl</span> <span class="struct">HasUnsafeFn</span> <span class="punctuation">{</span> <span class="keyword">impl</span> <span class="struct">HasUnsafeFn</span> <span class="punctuation">{</span>
@ -51,8 +56,14 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
<span class="keyword">fn</span> <span class="function declaration">main</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="punctuation">{</span> <span class="keyword">fn</span> <span class="function declaration">main</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="punctuation">{</span>
<span class="keyword">let</span> <span class="variable declaration">x</span> <span class="operator">=</span> <span class="operator">&</span><span class="numeric_literal">5</span> <span class="keyword">as</span> <span class="keyword">*</span><span class="keyword">const</span> <span class="builtin_type">usize</span><span class="punctuation">;</span> <span class="keyword">let</span> <span class="variable declaration">x</span> <span class="operator">=</span> <span class="operator">&</span><span class="numeric_literal">5</span> <span class="keyword">as</span> <span class="keyword">*</span><span class="keyword">const</span> <span class="builtin_type">usize</span><span class="punctuation">;</span>
<span class="keyword">let</span> <span class="variable declaration">u</span> <span class="operator">=</span> <span class="union">Union</span> <span class="punctuation">{</span> <span class="field">b</span><span class="punctuation">:</span> <span class="numeric_literal">0</span> <span class="punctuation">}</span><span class="punctuation">;</span>
<span class="keyword unsafe">unsafe</span> <span class="punctuation">{</span> <span class="keyword unsafe">unsafe</span> <span class="punctuation">{</span>
<span class="function unsafe">unsafe_fn</span><span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">;</span> <span class="function unsafe">unsafe_fn</span><span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">;</span>
<span class="keyword">let</span> <span class="variable declaration">b</span> <span class="operator">=</span> <span class="variable">u</span><span class="punctuation">.</span><span class="field unsafe">b</span><span class="punctuation">;</span>
<span class="keyword control">match</span> <span class="variable">u</span> <span class="punctuation">{</span>
<span class="union">Union</span> <span class="punctuation">{</span> <span class="field unsafe">b</span><span class="punctuation">:</span> <span class="numeric_literal">0</span> <span class="punctuation">}</span> <span class="operator">=&gt;</span> <span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">,</span>
<span class="union">Union</span> <span class="punctuation">{</span> <span class="field unsafe">a</span> <span class="punctuation">}</span> <span class="operator">=&gt;</span> <span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">,</span>
<span class="punctuation">}</span>
<span class="struct">HasUnsafeFn</span><span class="punctuation">.</span><span class="function unsafe">unsafe_method</span><span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">;</span> <span class="struct">HasUnsafeFn</span><span class="punctuation">.</span><span class="function unsafe">unsafe_method</span><span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">;</span>
<span class="keyword">let</span> <span class="variable declaration">y</span> <span class="operator">=</span> <span class="operator unsafe">*</span><span class="punctuation">(</span><span class="variable">x</span><span class="punctuation">)</span><span class="punctuation">;</span> <span class="keyword">let</span> <span class="variable declaration">y</span> <span class="operator">=</span> <span class="operator unsafe">*</span><span class="punctuation">(</span><span class="variable">x</span><span class="punctuation">)</span><span class="punctuation">;</span>
<span class="keyword">let</span> <span class="variable declaration">z</span> <span class="operator">=</span> <span class="numeric_literal">-</span><span class="variable">x</span><span class="punctuation">;</span> <span class="keyword">let</span> <span class="variable declaration">z</span> <span class="operator">=</span> <span class="numeric_literal">-</span><span class="variable">x</span><span class="punctuation">;</span>