From 3514f2f2ab9a59306f75af9515435190ab3ccdbd Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 26 May 2023 17:04:22 +0200 Subject: [PATCH 1/2] Render niches on hover --- crates/hir/src/lib.rs | 13 +++++++++++-- crates/ide/src/hover/render.rs | 27 +++++++++++++++++++-------- crates/ide/src/hover/tests.rs | 6 +++--- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 38add4eda1a..f73c4f31eee 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -62,7 +62,7 @@ use hir_ty::{ consteval::{try_const_usize, unknown_const_as_generic, ConstEvalError, ConstExt}, diagnostics::BodyValidationDiagnostic, display::HexifiedConst, - layout::{Layout, LayoutError, RustcEnumVariantIdx, TagEncoding}, + layout::{LayoutError, RustcEnumVariantIdx, TagEncoding}, method_resolution::{self, TyFingerprint}, mir::{self, interpret_mir}, primitive::UintTy, @@ -133,8 +133,11 @@ pub use { }, hir_ty::{ display::{ClosureStyle, HirDisplay, HirDisplayError, HirWrite}, + // FIXME: This just needs a HIR wrapper + layout::Layout, mir::MirEvalError, - PointerCast, Safety, + PointerCast, + Safety, }, }; @@ -4506,6 +4509,12 @@ impl HasCrate for Union { } } +impl HasCrate for Enum { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.module(db).krate() + } +} + impl HasCrate for Field { fn krate(&self, db: &dyn HirDatabase) -> Crate { self.parent_def(db).module(db).krate() diff --git a/crates/ide/src/hover/render.rs b/crates/ide/src/hover/render.rs index 4cbe7cca5af..d4c310447f2 100644 --- a/crates/ide/src/hover/render.rs +++ b/crates/ide/src/hover/render.rs @@ -3,8 +3,8 @@ use std::fmt::Display; use either::Either; use hir::{ - Adt, AsAssocItem, AttributeTemplate, CaptureKind, HasAttrs, HasSource, HirDisplay, Semantics, - TypeInfo, + db::HirDatabase, Adt, AsAssocItem, AttributeTemplate, CaptureKind, HasAttrs, HasCrate, + HasSource, HirDisplay, Layout, Semantics, TypeInfo, }; use ide_db::{ base_db::SourceDatabase, @@ -404,8 +404,9 @@ pub(super) fn definition( .map(|layout| format!(", offset = {:#X}", layout.fields.offset(id).bytes())), _ => None, }; + let niches = niches(db, it, &layout).unwrap_or_default(); Some(format!( - "size = {:#X}, align = {:#X}{}", + "size = {:#X}, align = {:#X}{}{niches}", layout.size.bytes(), layout.align.abi.bytes(), offset.as_deref().unwrap_or_default() @@ -415,8 +416,9 @@ pub(super) fn definition( Definition::Function(it) => label_and_docs(db, it), Definition::Adt(it) => label_and_layout_info_and_docs(db, it, config, |&it| { let layout = it.layout(db).ok()?; + let niches = niches(db, it, &layout).unwrap_or_default(); Some(format!( - "size = {:#X}, align = {:#X}", + "size = {:#X}, align = {:#X}{niches}", layout.size.bytes(), layout.align.abi.bytes() )) @@ -437,14 +439,15 @@ pub(super) fn definition( None } }, - |it| { + |&it| { let (layout, tag_size) = it.layout(db).ok()?; let size = layout.size.bytes_usize() - tag_size; if size == 0 { // There is no value in showing layout info for fieldless variants return None; } - Some(format!("size = {:#X}", layout.size.bytes())) + let niches = niches(db, it, &layout).unwrap_or_default(); + Some(format!("size = {:#X}{niches}", layout.size.bytes())) }, ), Definition::Const(it) => label_value_and_docs(db, it, |it| { @@ -473,10 +476,11 @@ pub(super) fn definition( Definition::TraitAlias(it) => label_and_docs(db, it), Definition::TypeAlias(it) => label_and_layout_info_and_docs(db, it, config, |&it| { let layout = it.ty(db).layout(db).ok()?; + let niches = niches(db, it, &layout).unwrap_or_default(); Some(format!( - "size = {:#X}, align = {:#X}", + "size = {:#X}, align = {:#X}{niches}", layout.size.bytes(), - layout.align.abi.bytes() + layout.align.abi.bytes(), )) }), Definition::BuiltinType(it) => { @@ -513,6 +517,13 @@ pub(super) fn definition( markup(docs, label, mod_path) } +fn niches(db: &RootDatabase, it: impl HasCrate, layout: &Layout) -> Option { + Some(format!( + ", niches = {}", + layout.largest_niche?.available(&*db.target_data_layout(it.krate(db).into())?) + )) +} + fn type_info( sema: &Semantics<'_, RootDatabase>, config: &HoverConfig, diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs index 4e171867fbf..f8b5b654546 100644 --- a/crates/ide/src/hover/tests.rs +++ b/crates/ide/src/hover/tests.rs @@ -1528,7 +1528,7 @@ fn test_hover_function_pointer_show_identifiers() { ``` ```rust - type foo = fn(a: i32, b: i32) -> i32 // size = 0x8, align = 0x8 + type foo = fn(a: i32, b: i32) -> i32 // size = 0x8, align = 0x8, niches = 1 ``` "#]], ); @@ -1546,7 +1546,7 @@ fn test_hover_function_pointer_no_identifier() { ``` ```rust - type foo = fn(i32, i32) -> i32 // size = 0x8, align = 0x8 + type foo = fn(i32, i32) -> i32 // size = 0x8, align = 0x8, niches = 1 ``` "#]], ); @@ -1904,7 +1904,7 @@ fn test_hover_layout_of_enum() { ``` ```rust - enum Foo // size = 0x10, align = 0x8 + enum Foo // size = 0x10, align = 0x8, niches = 254 ``` "#]], ); From 1275adc2004625cb7650ce5a6e99034e50c153cd Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 26 May 2023 17:18:27 +0200 Subject: [PATCH 2/2] Don't leak rustc Layout in hir layer --- crates/hir/src/lib.rs | 58 ++++++++++++++++++++++++---------- crates/ide/src/hover/render.rs | 40 +++++++---------------- 2 files changed, 54 insertions(+), 44 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index f73c4f31eee..5c0320f7334 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -62,7 +62,7 @@ use hir_ty::{ consteval::{try_const_usize, unknown_const_as_generic, ConstEvalError, ConstExt}, diagnostics::BodyValidationDiagnostic, display::HexifiedConst, - layout::{LayoutError, RustcEnumVariantIdx, TagEncoding}, + layout::{Layout as TyLayout, LayoutError, RustcEnumVariantIdx, TagEncoding}, method_resolution::{self, TyFingerprint}, mir::{self, interpret_mir}, primitive::UintTy, @@ -133,11 +133,8 @@ pub use { }, hir_ty::{ display::{ClosureStyle, HirDisplay, HirDisplayError, HirWrite}, - // FIXME: This just needs a HIR wrapper - layout::Layout, mir::MirEvalError, - PointerCast, - Safety, + PointerCast, Safety, }, }; @@ -964,8 +961,8 @@ impl Field { Type::new(db, var_id, ty) } - pub fn layout(&self, db: &dyn HirDatabase) -> Result, LayoutError> { - db.layout_of_ty(self.ty(db).ty.clone(), self.parent.module(db).krate().into()) + pub fn layout(&self, db: &dyn HirDatabase) -> Result { + db.layout_of_ty(self.ty(db).ty.clone(), self.parent.module(db).krate().into()).map(Layout) } pub fn parent_def(&self, _db: &dyn HirDatabase) -> VariantDef { @@ -1138,10 +1135,10 @@ impl Enum { self.variants(db).iter().any(|v| !matches!(v.kind(db), StructKind::Unit)) } - pub fn layout(self, db: &dyn HirDatabase) -> Result<(Arc, usize), LayoutError> { + pub fn layout(self, db: &dyn HirDatabase) -> Result<(Layout, usize), LayoutError> { let layout = Adt::from(self).layout(db)?; let tag_size = - if let layout::Variants::Multiple { tag, tag_encoding, .. } = &layout.variants { + if let layout::Variants::Multiple { tag, tag_encoding, .. } = &layout.0.variants { match tag_encoding { TagEncoding::Direct => { let target_data_layout = db @@ -1222,11 +1219,11 @@ impl Variant { let parent_enum = self.parent_enum(db); let (parent_layout, tag_size) = parent_enum.layout(db)?; Ok(( - match &parent_layout.variants { + match &parent_layout.0.variants { layout::Variants::Multiple { variants, .. } => { - variants[RustcEnumVariantIdx(self.id)].clone() + Layout(Arc::new(variants[RustcEnumVariantIdx(self.id)].clone())) } - _ => (*parent_layout).clone(), + _ => parent_layout, }, tag_size, )) @@ -1258,11 +1255,11 @@ impl Adt { }) } - pub fn layout(self, db: &dyn HirDatabase) -> Result, LayoutError> { + pub fn layout(self, db: &dyn HirDatabase) -> Result { if db.generic_params(self.into()).iter().count() != 0 { return Err(LayoutError::HasPlaceholder); } - db.layout_of_adt(self.into(), Substitution::empty(Interner), self.krate(db).id) + db.layout_of_adt(self.into(), Substitution::empty(Interner), self.krate(db).id).map(Layout) } /// Turns this ADT into a type. Any type parameters of the ADT will be @@ -4246,8 +4243,8 @@ impl Type { .collect() } - pub fn layout(&self, db: &dyn HirDatabase) -> Result, LayoutError> { - db.layout_of_ty(self.ty.clone(), self.env.krate) + pub fn layout(&self, db: &dyn HirDatabase) -> Result { + db.layout_of_ty(self.ty.clone(), self.env.krate).map(Layout) } } @@ -4358,6 +4355,35 @@ fn closure_source(db: &dyn HirDatabase, closure: ClosureId) -> Option); + +impl Layout { + pub fn size(&self) -> u64 { + self.0.size.bytes() + } + + pub fn align(&self) -> u64 { + self.0.align.abi.bytes() + } + + pub fn niches(&self, db: &dyn HirDatabase, krate: Crate) -> Option { + Some(self.0.largest_niche?.available(&*db.target_data_layout(krate.id)?)) + } + + pub fn field_offset(&self, idx: usize) -> Option { + match self.0.fields { + layout::FieldsShape::Primitive => None, + layout::FieldsShape::Union(_) => Some(0), + layout::FieldsShape::Array { stride, count } => { + let i = u64::try_from(idx).ok()?; + (i < count).then_some((stride * i).bytes()) + } + layout::FieldsShape::Arbitrary { ref offsets, .. } => Some(offsets.get(idx)?.bytes()), + } + } +} + #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum BindingMode { Move, diff --git a/crates/ide/src/hover/render.rs b/crates/ide/src/hover/render.rs index d4c310447f2..96a97ab44c0 100644 --- a/crates/ide/src/hover/render.rs +++ b/crates/ide/src/hover/render.rs @@ -3,8 +3,8 @@ use std::fmt::Display; use either::Either; use hir::{ - db::HirDatabase, Adt, AsAssocItem, AttributeTemplate, CaptureKind, HasAttrs, HasCrate, - HasSource, HirDisplay, Layout, Semantics, TypeInfo, + Adt, AsAssocItem, AttributeTemplate, CaptureKind, HasAttrs, HasCrate, HasSource, HirDisplay, + Layout, Semantics, TypeInfo, }; use ide_db::{ base_db::SourceDatabase, @@ -401,14 +401,14 @@ pub(super) fn definition( hir::VariantDef::Struct(s) => Adt::from(s) .layout(db) .ok() - .map(|layout| format!(", offset = {:#X}", layout.fields.offset(id).bytes())), + .and_then(|layout| Some(format!(", offset = {:#X}", layout.field_offset(id)?))), _ => None, }; let niches = niches(db, it, &layout).unwrap_or_default(); Some(format!( "size = {:#X}, align = {:#X}{}{niches}", - layout.size.bytes(), - layout.align.abi.bytes(), + layout.size(), + layout.align(), offset.as_deref().unwrap_or_default() )) }), @@ -417,11 +417,7 @@ pub(super) fn definition( Definition::Adt(it) => label_and_layout_info_and_docs(db, it, config, |&it| { let layout = it.layout(db).ok()?; let niches = niches(db, it, &layout).unwrap_or_default(); - Some(format!( - "size = {:#X}, align = {:#X}{niches}", - layout.size.bytes(), - layout.align.abi.bytes() - )) + Some(format!("size = {:#X}, align = {:#X}{niches}", layout.size(), layout.align())) }), Definition::Variant(it) => label_value_and_layout_info_and_docs( db, @@ -441,13 +437,13 @@ pub(super) fn definition( }, |&it| { let (layout, tag_size) = it.layout(db).ok()?; - let size = layout.size.bytes_usize() - tag_size; + let size = layout.size() as usize - tag_size; if size == 0 { // There is no value in showing layout info for fieldless variants return None; } let niches = niches(db, it, &layout).unwrap_or_default(); - Some(format!("size = {:#X}{niches}", layout.size.bytes())) + Some(format!("size = {:#X}{niches}", layout.size())) }, ), Definition::Const(it) => label_value_and_docs(db, it, |it| { @@ -477,11 +473,7 @@ pub(super) fn definition( Definition::TypeAlias(it) => label_and_layout_info_and_docs(db, it, config, |&it| { let layout = it.ty(db).layout(db).ok()?; let niches = niches(db, it, &layout).unwrap_or_default(); - Some(format!( - "size = {:#X}, align = {:#X}{niches}", - layout.size.bytes(), - layout.align.abi.bytes(), - )) + Some(format!("size = {:#X}, align = {:#X}{niches}", layout.size(), layout.align(),)) }), Definition::BuiltinType(it) => { return famous_defs @@ -518,10 +510,7 @@ pub(super) fn definition( } fn niches(db: &RootDatabase, it: impl HasCrate, layout: &Layout) -> Option { - Some(format!( - ", niches = {}", - layout.largest_niche?.available(&*db.target_data_layout(it.krate(db).into())?) - )) + Some(format!(", niches = {}", layout.niches(db, it.krate(db).into())?)) } fn type_info( @@ -571,7 +560,7 @@ fn closure_ty( let layout = if config.memory_layout { original .layout(sema.db) - .map(|x| format!(" // size = {}, align = {}", x.size.bytes(), x.align.abi.bytes())) + .map(|x| format!(" // size = {}, align = {}", x.size(), x.align())) .unwrap_or_default() } else { String::default() @@ -782,12 +771,7 @@ fn local(db: &RootDatabase, it: hir::Local, config: &HoverConfig) -> Option