Skip to content

Commit 8a57afe

Browse files
bors[bot]Nashenas88
bors[bot]
and
Nashenas88
authored
Merge #5684
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 <[email protected]>
2 parents eed05a9 + be935b2 commit 8a57afe

File tree

6 files changed

+122
-20
lines changed

6 files changed

+122
-20
lines changed

‎crates/ra_hir_ty/src/infer.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,12 @@ impl<'a> InferenceContext<'a> {
440440
let ty = self.insert_type_vars(ty.subst(&substs));
441441
forbid_unresolved_segments((ty, Some(strukt.into())), unresolved)
442442
}
443+
TypeNs::AdtId(AdtId::UnionId(u)) => {
444+
let substs = Ty::substs_from_path(&ctx, path, u.into(), true);
445+
let ty = self.db.ty(u.into());
446+
let ty = self.insert_type_vars(ty.subst(&substs));
447+
forbid_unresolved_segments((ty, Some(u.into())), unresolved)
448+
}
443449
TypeNs::EnumVariantId(var) => {
444450
let substs = Ty::substs_from_path(&ctx, path, var.into(), true);
445451
let ty = self.db.ty(var.parent.into());
@@ -490,10 +496,7 @@ impl<'a> InferenceContext<'a> {
490496
// FIXME potentially resolve assoc type
491497
(Ty::Unknown, None)
492498
}
493-
TypeNs::AdtId(AdtId::EnumId(_))
494-
| TypeNs::AdtId(AdtId::UnionId(_))
495-
| TypeNs::BuiltinType(_)
496-
| TypeNs::TraitId(_) => {
499+
TypeNs::AdtId(AdtId::EnumId(_)) | TypeNs::BuiltinType(_) | TypeNs::TraitId(_) => {
497500
// FIXME diagnostic
498501
(Ty::Unknown, None)
499502
}

‎crates/ra_hir_ty/src/lower.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ impl Ty {
518518
let (segment, generic_def) = match resolved {
519519
ValueTyDefId::FunctionId(it) => (last, Some(it.into())),
520520
ValueTyDefId::StructId(it) => (last, Some(it.into())),
521+
ValueTyDefId::UnionId(it) => (last, Some(it.into())),
521522
ValueTyDefId::ConstId(it) => (last, Some(it.into())),
522523
ValueTyDefId::StaticId(_) => (last, None),
523524
ValueTyDefId::EnumVariantId(var) => {
@@ -1148,11 +1149,12 @@ impl_from!(BuiltinType, AdtId(StructId, EnumId, UnionId), TypeAliasId for TyDefI
11481149
pub enum ValueTyDefId {
11491150
FunctionId(FunctionId),
11501151
StructId(StructId),
1152+
UnionId(UnionId),
11511153
EnumVariantId(EnumVariantId),
11521154
ConstId(ConstId),
11531155
StaticId(StaticId),
11541156
}
1155-
impl_from!(FunctionId, StructId, EnumVariantId, ConstId, StaticId for ValueTyDefId);
1157+
impl_from!(FunctionId, StructId, UnionId, EnumVariantId, ConstId, StaticId for ValueTyDefId);
11561158

11571159
/// Build the declared type of an item. This depends on the namespace; e.g. for
11581160
/// `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
11791181
match def {
11801182
ValueTyDefId::FunctionId(it) => type_for_fn(db, it),
11811183
ValueTyDefId::StructId(it) => type_for_struct_constructor(db, it),
1184+
ValueTyDefId::UnionId(it) => type_for_adt(db, it.into()),
11821185
ValueTyDefId::EnumVariantId(it) => type_for_enum_variant_constructor(db, it),
11831186
ValueTyDefId::ConstId(it) => type_for_const(db, it),
11841187
ValueTyDefId::StaticId(it) => type_for_static(db, it),

‎crates/ra_hir_ty/src/tests/simple.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,16 +334,44 @@ fn infer_union() {
334334
bar: f32,
335335
}
336336
337+
fn test() {
338+
let u = MyUnion { foo: 0 };
339+
unsafe { baz(u); }
340+
let u = MyUnion { bar: 0.0 };
341+
unsafe { baz(u); }
342+
}
343+
337344
unsafe fn baz(u: MyUnion) {
338345
let inner = u.foo;
346+
let inner = u.bar;
339347
}
340348
"#,
341349
expect![[r#"
342-
61..62 'u': MyUnion
343-
73..99 '{ ...foo; }': ()
344-
83..88 'inner': u32
345-
91..92 'u': MyUnion
346-
91..96 'u.foo': u32
350+
57..172 '{ ...); } }': ()
351+
67..68 'u': MyUnion
352+
71..89 'MyUnio...o: 0 }': MyUnion
353+
86..87 '0': u32
354+
95..113 'unsafe...(u); }': ()
355+
102..113 '{ baz(u); }': ()
356+
104..107 'baz': fn baz(MyUnion)
357+
104..110 'baz(u)': ()
358+
108..109 'u': MyUnion
359+
122..123 'u': MyUnion
360+
126..146 'MyUnio... 0.0 }': MyUnion
361+
141..144 '0.0': f32
362+
152..170 'unsafe...(u); }': ()
363+
159..170 '{ baz(u); }': ()
364+
161..164 'baz': fn baz(MyUnion)
365+
161..167 'baz(u)': ()
366+
165..166 'u': MyUnion
367+
188..189 'u': MyUnion
368+
200..249 '{ ...bar; }': ()
369+
210..215 'inner': u32
370+
218..219 'u': MyUnion
371+
218..223 'u.foo': u32
372+
233..238 'inner': f32
373+
241..242 'u': MyUnion
374+
241..246 'u.bar': f32
347375
"#]],
348376
);
349377
}

‎crates/ra_ide/src/syntax_highlighting.rs

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod injection;
44
#[cfg(test)]
55
mod tests;
66

7-
use hir::{Name, Semantics};
7+
use hir::{Name, Semantics, VariantDef};
88
use ra_ide_db::{
99
defs::{classify_name, classify_name_ref, Definition, NameClass, NameRefClass},
1010
RootDatabase,
@@ -455,6 +455,18 @@ fn macro_call_range(macro_call: &ast::MacroCall) -> Option<TextRange> {
455455
Some(TextRange::new(range_start, range_end))
456456
}
457457

458+
fn is_possibly_unsafe(name_ref: &ast::NameRef) -> bool {
459+
name_ref
460+
.syntax()
461+
.parent()
462+
.and_then(|parent| {
463+
ast::FieldExpr::cast(parent.clone())
464+
.map(|_| true)
465+
.or_else(|| ast::RecordPatField::cast(parent).map(|_| true))
466+
})
467+
.unwrap_or(false)
468+
}
469+
458470
fn highlight_element(
459471
sema: &Semantics<RootDatabase>,
460472
bindings_shadow_count: &mut FxHashMap<Name, u32>,
@@ -484,10 +496,19 @@ fn highlight_element(
484496

485497
match name_kind {
486498
Some(NameClass::Definition(def)) => {
487-
highlight_name(db, def) | HighlightModifier::Definition
499+
highlight_name(db, def, false) | HighlightModifier::Definition
500+
}
501+
Some(NameClass::ConstReference(def)) => highlight_name(db, def, false),
502+
Some(NameClass::FieldShorthand { field, .. }) => {
503+
let mut h = HighlightTag::Field.into();
504+
if let Definition::Field(field) = field {
505+
if let VariantDef::Union(_) = field.parent_def(db) {
506+
h |= HighlightModifier::Unsafe;
507+
}
508+
}
509+
510+
h
488511
}
489-
Some(NameClass::ConstReference(def)) => highlight_name(db, def),
490-
Some(NameClass::FieldShorthand { .. }) => HighlightTag::Field.into(),
491512
None => highlight_name_by_syntax(name) | HighlightModifier::Definition,
492513
}
493514
}
@@ -498,6 +519,7 @@ fn highlight_element(
498519
}
499520
NAME_REF => {
500521
let name_ref = element.into_node().and_then(ast::NameRef::cast).unwrap();
522+
let possibly_unsafe = is_possibly_unsafe(&name_ref);
501523
match classify_name_ref(sema, &name_ref) {
502524
Some(name_kind) => match name_kind {
503525
NameRefClass::Definition(def) => {
@@ -508,11 +530,13 @@ fn highlight_element(
508530
binding_hash = Some(calc_binding_hash(&name, *shadow_count))
509531
}
510532
};
511-
highlight_name(db, def)
533+
highlight_name(db, def, possibly_unsafe)
512534
}
513535
NameRefClass::FieldShorthand { .. } => HighlightTag::Field.into(),
514536
},
515-
None if syntactic_name_ref_highlighting => highlight_name_ref_by_syntax(name_ref),
537+
None if syntactic_name_ref_highlighting => {
538+
highlight_name_ref_by_syntax(name_ref, sema)
539+
}
516540
None => HighlightTag::UnresolvedReference.into(),
517541
}
518542
}
@@ -652,10 +676,19 @@ fn is_child_of_impl(element: &SyntaxElement) -> bool {
652676
}
653677
}
654678

655-
fn highlight_name(db: &RootDatabase, def: Definition) -> Highlight {
679+
fn highlight_name(db: &RootDatabase, def: Definition, possibly_unsafe: bool) -> Highlight {
656680
match def {
657681
Definition::Macro(_) => HighlightTag::Macro,
658-
Definition::Field(_) => HighlightTag::Field,
682+
Definition::Field(field) => {
683+
let mut h = HighlightTag::Field.into();
684+
if possibly_unsafe {
685+
if let VariantDef::Union(_) = field.parent_def(db) {
686+
h |= HighlightModifier::Unsafe;
687+
}
688+
}
689+
690+
return h;
691+
}
659692
Definition::ModuleDef(def) => match def {
660693
hir::ModuleDef::Module(_) => HighlightTag::Module,
661694
hir::ModuleDef::Function(func) => {
@@ -725,7 +758,7 @@ fn highlight_name_by_syntax(name: ast::Name) -> Highlight {
725758
tag.into()
726759
}
727760

728-
fn highlight_name_ref_by_syntax(name: ast::NameRef) -> Highlight {
761+
fn highlight_name_ref_by_syntax(name: ast::NameRef, sema: &Semantics<RootDatabase>) -> Highlight {
729762
let default = HighlightTag::UnresolvedReference;
730763

731764
let parent = match name.syntax().parent() {
@@ -735,7 +768,20 @@ fn highlight_name_ref_by_syntax(name: ast::NameRef) -> Highlight {
735768

736769
let tag = match parent.kind() {
737770
METHOD_CALL_EXPR => HighlightTag::Function,
738-
FIELD_EXPR => HighlightTag::Field,
771+
FIELD_EXPR => {
772+
let h = HighlightTag::Field;
773+
let is_union = ast::FieldExpr::cast(parent)
774+
.and_then(|field_expr| {
775+
let field = sema.resolve_field(&field_expr)?;
776+
Some(if let VariantDef::Union(_) = field.parent_def(sema.db) {
777+
true
778+
} else {
779+
false
780+
})
781+
})
782+
.unwrap_or(false);
783+
return if is_union { h | HighlightModifier::Unsafe } else { h.into() };
784+
}
739785
PATH_SEGMENT => {
740786
let path = match parent.parent().and_then(ast::Path::cast) {
741787
Some(it) => it,

‎crates/ra_ide/src/syntax_highlighting/tests.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,11 @@ fn test_unsafe_highlighting() {
275275
r#"
276276
unsafe fn unsafe_fn() {}
277277
278+
union Union {
279+
a: u32,
280+
b: f32,
281+
}
282+
278283
struct HasUnsafeFn;
279284
280285
impl HasUnsafeFn {
@@ -289,8 +294,14 @@ static mut global_mut: TypeForStaticMut = TypeForStaticMut { a: 0 };
289294
290295
fn main() {
291296
let x = &5 as *const usize;
297+
let u = Union { b: 0 };
292298
unsafe {
293299
unsafe_fn();
300+
let b = u.b;
301+
match u {
302+
Union { b: 0 } => (),
303+
Union { a } => (),
304+
}
294305
HasUnsafeFn.unsafe_method();
295306
let y = *(x);
296307
let z = -x;

‎crates/ra_ide/test_data/highlight_unsafe.html

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@
3737
</style>
3838
<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>
3939

40+
<span class="keyword">union</span> <span class="union declaration">Union</span> <span class="punctuation">{</span>
41+
<span class="field declaration">a</span><span class="punctuation">:</span> <span class="builtin_type">u32</span><span class="punctuation">,</span>
42+
<span class="field declaration">b</span><span class="punctuation">:</span> <span class="builtin_type">f32</span><span class="punctuation">,</span>
43+
<span class="punctuation">}</span>
44+
4045
<span class="keyword">struct</span> <span class="struct declaration">HasUnsafeFn</span><span class="punctuation">;</span>
4146

4247
<span class="keyword">impl</span> <span class="struct">HasUnsafeFn</span> <span class="punctuation">{</span>
@@ -51,8 +56,14 @@
5156

5257
<span class="keyword">fn</span> <span class="function declaration">main</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="punctuation">{</span>
5358
<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>
59+
<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>
5460
<span class="keyword unsafe">unsafe</span> <span class="punctuation">{</span>
5561
<span class="function unsafe">unsafe_fn</span><span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">;</span>
62+
<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>
63+
<span class="keyword control">match</span> <span class="variable">u</span> <span class="punctuation">{</span>
64+
<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>
65+
<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>
66+
<span class="punctuation">}</span>
5667
<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>
5768
<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>
5869
<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>

0 commit comments

Comments
 (0)