Skip to content

Commit 317c76f

Browse files
VeykrilVeykril
authored
Merge pull request #20106 from Veykril/push-pytuxksnntux
Salsa idiomize `VariantFields` query
2 parents 11c7207 + 4687261 commit 317c76f

33 files changed

+206
-129
lines changed

‎crates/hir-def/src/db.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ use crate::{
2929
signatures::{
3030
ConstSignature, EnumSignature, FunctionSignature, ImplSignature, StaticSignature,
3131
StructSignature, TraitAliasSignature, TraitSignature, TypeAliasSignature, UnionSignature,
32-
VariantFields,
3332
},
3433
tt,
3534
visibility::{self, Visibility},
@@ -113,17 +112,6 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + SourceDatabase {
113112

114113
// region:data
115114

116-
#[salsa::invoke(VariantFields::query)]
117-
fn variant_fields_with_source_map(
118-
&self,
119-
id: VariantId,
120-
) -> (Arc<VariantFields>, Arc<ExpressionStoreSourceMap>);
121-
122-
#[salsa::tracked]
123-
fn variant_fields(&self, id: VariantId) -> Arc<VariantFields> {
124-
self.variant_fields_with_source_map(id).0
125-
}
126-
127115
#[salsa::tracked]
128116
fn trait_signature(&self, trait_: TraitId) -> Arc<TraitSignature> {
129117
self.trait_signature_with_source_map(trait_).0

‎crates/hir-def/src/expr_store.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ pub mod scope;
99
#[cfg(test)]
1010
mod tests;
1111

12-
use std::ops::{Deref, Index};
12+
use std::{
13+
ops::{Deref, Index},
14+
sync::LazyLock,
15+
};
1316

1417
use cfg::{CfgExpr, CfgOptions};
1518
use either::Either;
@@ -19,6 +22,7 @@ use rustc_hash::FxHashMap;
1922
use smallvec::SmallVec;
2023
use span::{Edition, SyntaxContext};
2124
use syntax::{AstPtr, SyntaxNodePtr, ast};
25+
use triomphe::Arc;
2226
use tt::TextRange;
2327

2428
use crate::{
@@ -220,6 +224,12 @@ impl ExpressionStoreBuilder {
220224
}
221225

222226
impl ExpressionStore {
227+
pub fn empty_singleton() -> Arc<Self> {
228+
static EMPTY: LazyLock<Arc<ExpressionStore>> =
229+
LazyLock::new(|| Arc::new(ExpressionStoreBuilder::default().finish()));
230+
EMPTY.clone()
231+
}
232+
223233
/// Returns an iterator over all block expressions in this store that define inner items.
224234
pub fn blocks<'a>(
225235
&'a self,
@@ -636,6 +646,12 @@ impl Index<PathId> for ExpressionStore {
636646
// FIXME: Change `node_` prefix to something more reasonable.
637647
// Perhaps `expr_syntax` and `expr_id`?
638648
impl ExpressionStoreSourceMap {
649+
pub fn empty_singleton() -> Arc<Self> {
650+
static EMPTY: LazyLock<Arc<ExpressionStoreSourceMap>> =
651+
LazyLock::new(|| Arc::new(ExpressionStoreSourceMap::default()));
652+
EMPTY.clone()
653+
}
654+
639655
pub fn expr_or_pat_syntax(&self, id: ExprOrPatId) -> Result<ExprOrPatSource, SyntheticSyntax> {
640656
match id {
641657
ExprOrPatId::ExprId(id) => self.expr_syntax(id),

‎crates/hir-def/src/expr_store/lower.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2250,7 +2250,7 @@ impl ExprCollector<'_> {
22502250
Some(ModuleDefId::ConstId(_)) => (None, Pat::Path(name.into())),
22512251
Some(ModuleDefId::EnumVariantId(variant))
22522252
// FIXME: This can cause a cycle if the user is writing invalid code
2253-
if self.db.variant_fields(variant.into()).shape != FieldsShape::Record =>
2253+
if variant.fields(self.db).shape != FieldsShape::Record =>
22542254
{
22552255
(None, Pat::Path(name.into()))
22562256
}

‎crates/hir-def/src/expr_store/pretty.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ pub fn print_variant_body_hir(db: &dyn DefDatabase, owner: VariantId, edition: E
121121
VariantId::UnionId(it) => format!("union {}", item_name(db, it, "<missing>")),
122122
};
123123

124-
let fields = db.variant_fields(owner);
124+
let fields = owner.fields(db);
125125

126126
let mut p = Printer {
127127
db,

‎crates/hir-def/src/lib.rs

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ use crate::{
8787
attr::Attrs,
8888
builtin_type::BuiltinType,
8989
db::DefDatabase,
90+
expr_store::ExpressionStoreSourceMap,
9091
hir::generics::{LocalLifetimeParamId, LocalTypeOrConstParamId},
9192
nameres::{
9293
LocalDefMap,
@@ -254,9 +255,35 @@ impl_intern!(FunctionId, FunctionLoc, intern_function, lookup_intern_function);
254255
type StructLoc = ItemLoc<ast::Struct>;
255256
impl_intern!(StructId, StructLoc, intern_struct, lookup_intern_struct);
256257

258+
impl StructId {
259+
pub fn fields(self, db: &dyn DefDatabase) -> &VariantFields {
260+
VariantFields::firewall(db, self.into())
261+
}
262+
263+
pub fn fields_with_source_map(
264+
self,
265+
db: &dyn DefDatabase,
266+
) -> (Arc<VariantFields>, Arc<ExpressionStoreSourceMap>) {
267+
VariantFields::query(db, self.into())
268+
}
269+
}
270+
257271
pub type UnionLoc = ItemLoc<ast::Union>;
258272
impl_intern!(UnionId, UnionLoc, intern_union, lookup_intern_union);
259273

274+
impl UnionId {
275+
pub fn fields(self, db: &dyn DefDatabase) -> &VariantFields {
276+
VariantFields::firewall(db, self.into())
277+
}
278+
279+
pub fn fields_with_source_map(
280+
self,
281+
db: &dyn DefDatabase,
282+
) -> (Arc<VariantFields>, Arc<ExpressionStoreSourceMap>) {
283+
VariantFields::query(db, self.into())
284+
}
285+
}
286+
260287
pub type EnumLoc = ItemLoc<ast::Enum>;
261288
impl_intern!(EnumId, EnumLoc, intern_enum, lookup_intern_enum);
262289

@@ -337,6 +364,20 @@ pub struct EnumVariantLoc {
337364
}
338365
impl_intern!(EnumVariantId, EnumVariantLoc, intern_enum_variant, lookup_intern_enum_variant);
339366
impl_loc!(EnumVariantLoc, id: Variant, parent: EnumId);
367+
368+
impl EnumVariantId {
369+
pub fn fields(self, db: &dyn DefDatabase) -> &VariantFields {
370+
VariantFields::firewall(db, self.into())
371+
}
372+
373+
pub fn fields_with_source_map(
374+
self,
375+
db: &dyn DefDatabase,
376+
) -> (Arc<VariantFields>, Arc<ExpressionStoreSourceMap>) {
377+
VariantFields::query(db, self.into())
378+
}
379+
}
380+
340381
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
341382
pub struct Macro2Loc {
342383
pub container: ModuleId,
@@ -1024,8 +1065,15 @@ pub enum VariantId {
10241065
impl_from!(EnumVariantId, StructId, UnionId for VariantId);
10251066

10261067
impl VariantId {
1027-
pub fn variant_data(self, db: &dyn DefDatabase) -> Arc<VariantFields> {
1028-
db.variant_fields(self)
1068+
pub fn fields(self, db: &dyn DefDatabase) -> &VariantFields {
1069+
VariantFields::firewall(db, self)
1070+
}
1071+
1072+
pub fn fields_with_source_map(
1073+
self,
1074+
db: &dyn DefDatabase,
1075+
) -> (Arc<VariantFields>, Arc<ExpressionStoreSourceMap>) {
1076+
VariantFields::query(db, self)
10291077
}
10301078

10311079
pub fn file_id(self, db: &dyn DefDatabase) -> HirFileId {

‎crates/hir-def/src/signatures.rs

Lines changed: 57 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Item signature IR definitions
22
3-
use std::ops::Not as _;
3+
use std::{cell::LazyCell, ops::Not as _};
44

55
use bitflags::bitflags;
66
use cfg::{CfgExpr, CfgOptions};
@@ -731,29 +731,26 @@ pub struct VariantFields {
731731
pub store: Arc<ExpressionStore>,
732732
pub shape: FieldsShape,
733733
}
734+
735+
#[salsa::tracked]
734736
impl VariantFields {
735-
#[inline]
737+
#[salsa::tracked(returns(clone))]
736738
pub(crate) fn query(
737739
db: &dyn DefDatabase,
738740
id: VariantId,
739741
) -> (Arc<Self>, Arc<ExpressionStoreSourceMap>) {
740-
let (shape, (fields, store, source_map)) = match id {
742+
let (shape, result) = match id {
741743
VariantId::EnumVariantId(id) => {
742744
let loc = id.lookup(db);
743745
let parent = loc.parent.lookup(db);
744746
let source = loc.source(db);
745747
let shape = adt_shape(source.value.kind());
746-
let span_map = db.span_map(source.file_id);
747-
let override_visibility = visibility_from_ast(
748-
db,
749-
source.value.parent_enum().visibility(),
750-
&mut |range| span_map.span_for_range(range).ctx,
751-
);
748+
let enum_vis = Some(source.value.parent_enum().visibility());
752749
let fields = lower_field_list(
753750
db,
754751
parent.container,
755752
source.map(|src| src.field_list()),
756-
Some(override_visibility),
753+
enum_vis,
757754
);
758755
(shape, fields)
759756
}
@@ -777,10 +774,29 @@ impl VariantFields {
777774
(FieldsShape::Record, fields)
778775
}
779776
};
777+
match result {
778+
Some((fields, store, source_map)) => (
779+
Arc::new(VariantFields { fields, store: Arc::new(store), shape }),
780+
Arc::new(source_map),
781+
),
782+
None => (
783+
Arc::new(VariantFields {
784+
fields: Arena::default(),
785+
store: ExpressionStore::empty_singleton(),
786+
shape,
787+
}),
788+
ExpressionStoreSourceMap::empty_singleton(),
789+
),
790+
}
791+
}
780792

781-
(Arc::new(VariantFields { fields, store: Arc::new(store), shape }), Arc::new(source_map))
793+
#[salsa::tracked(returns(deref))]
794+
pub(crate) fn firewall(db: &dyn DefDatabase, id: VariantId) -> Arc<Self> {
795+
Self::query(db, id).0
782796
}
797+
}
783798

799+
impl VariantFields {
784800
pub fn len(&self) -> usize {
785801
self.fields.len()
786802
}
@@ -798,31 +814,24 @@ fn lower_field_list(
798814
db: &dyn DefDatabase,
799815
module: ModuleId,
800816
fields: InFile<Option<ast::FieldList>>,
801-
override_visibility: Option<RawVisibility>,
802-
) -> (Arena<FieldData>, ExpressionStore, ExpressionStoreSourceMap) {
817+
override_visibility: Option<Option<ast::Visibility>>,
818+
) -> Option<(Arena<FieldData>, ExpressionStore, ExpressionStoreSourceMap)> {
803819
let file_id = fields.file_id;
804-
match fields.value {
805-
Some(ast::FieldList::RecordFieldList(fields)) => lower_fields(
820+
match fields.value? {
821+
ast::FieldList::RecordFieldList(fields) => lower_fields(
806822
db,
807823
module,
808824
InFile::new(file_id, fields.fields().map(|field| (field.ty(), field))),
809825
|_, field| as_name_opt(field.name()),
810826
override_visibility,
811827
),
812-
Some(ast::FieldList::TupleFieldList(fields)) => lower_fields(
828+
ast::FieldList::TupleFieldList(fields) => lower_fields(
813829
db,
814830
module,
815831
InFile::new(file_id, fields.fields().map(|field| (field.ty(), field))),
816832
|idx, _| Name::new_tuple_field(idx),
817833
override_visibility,
818834
),
819-
None => lower_fields(
820-
db,
821-
module,
822-
InFile::new(file_id, std::iter::empty::<(Option<ast::Type>, ast::RecordField)>()),
823-
|_, _| Name::missing(),
824-
None,
825-
),
826835
}
827836
}
828837

@@ -831,22 +840,34 @@ fn lower_fields<Field: ast::HasAttrs + ast::HasVisibility>(
831840
module: ModuleId,
832841
fields: InFile<impl Iterator<Item = (Option<ast::Type>, Field)>>,
833842
mut field_name: impl FnMut(usize, &Field) -> Name,
834-
override_visibility: Option<RawVisibility>,
835-
) -> (Arena<FieldData>, ExpressionStore, ExpressionStoreSourceMap) {
836-
let mut arena = Arena::new();
843+
override_visibility: Option<Option<ast::Visibility>>,
844+
) -> Option<(Arena<FieldData>, ExpressionStore, ExpressionStoreSourceMap)> {
837845
let cfg_options = module.krate.cfg_options(db);
838846
let mut col = ExprCollector::new(db, module, fields.file_id);
847+
let override_visibility = override_visibility.map(|vis| {
848+
LazyCell::new(|| {
849+
let span_map = db.span_map(fields.file_id);
850+
visibility_from_ast(db, vis, &mut |range| span_map.span_for_range(range).ctx)
851+
})
852+
});
853+
854+
let mut arena = Arena::new();
839855
let mut idx = 0;
856+
let mut has_fields = false;
840857
for (ty, field) in fields.value {
858+
has_fields = true;
841859
match Attrs::is_cfg_enabled_for(db, &field, col.span_map(), cfg_options) {
842860
Ok(()) => {
843861
let type_ref =
844862
col.lower_type_ref_opt(ty, &mut ExprCollector::impl_trait_error_allocator);
845-
let visibility = override_visibility.clone().unwrap_or_else(|| {
846-
visibility_from_ast(db, field.visibility(), &mut |range| {
847-
col.span_map().span_for_range(range).ctx
848-
})
849-
});
863+
let visibility = override_visibility.as_ref().map_or_else(
864+
|| {
865+
visibility_from_ast(db, field.visibility(), &mut |range| {
866+
col.span_map().span_for_range(range).ctx
867+
})
868+
},
869+
|it| RawVisibility::clone(it),
870+
);
850871
let is_unsafe = field
851872
.syntax()
852873
.children_with_tokens()
@@ -867,9 +888,12 @@ fn lower_fields<Field: ast::HasAttrs + ast::HasVisibility>(
867888
}
868889
}
869890
}
891+
if !has_fields {
892+
return None;
893+
}
870894
let store = col.store.finish();
871895
arena.shrink_to_fit();
872-
(arena, store, col.source_map)
896+
Some((arena, store, col.source_map))
873897
}
874898

875899
#[derive(Debug, PartialEq, Eq)]
@@ -948,7 +972,7 @@ impl EnumVariants {
948972
self.variants.iter().all(|&(v, _, _)| {
949973
// The condition check order is slightly modified from rustc
950974
// to improve performance by early returning with relatively fast checks
951-
let variant = &db.variant_fields(v.into());
975+
let variant = v.fields(db);
952976
if !variant.fields().is_empty() {
953977
return false;
954978
}

‎crates/hir-def/src/visibility.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ pub(crate) fn field_visibilities_query(
273273
db: &dyn DefDatabase,
274274
variant_id: VariantId,
275275
) -> Arc<ArenaMap<LocalFieldId, Visibility>> {
276-
let variant_fields = db.variant_fields(variant_id);
276+
let variant_fields = variant_id.fields(db);
277277
let fields = variant_fields.fields();
278278
if fields.is_empty() {
279279
return Arc::default();

‎crates/hir-ty/src/chalk_db.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,7 @@ pub(crate) fn adt_datum_query(
801801

802802
// this slows down rust-analyzer by quite a bit unfortunately, so enabling this is currently not worth it
803803
let _variant_id_to_fields = |id: VariantId| {
804-
let variant_data = &id.variant_data(db);
804+
let variant_data = &id.fields(db);
805805
let fields = if variant_data.fields().is_empty() {
806806
vec![]
807807
} else {

‎crates/hir-ty/src/diagnostics/decl_check.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ impl<'a> DeclValidator<'a> {
307307

308308
/// Check incorrect names for struct fields.
309309
fn validate_struct_fields(&mut self, struct_id: StructId) {
310-
let data = self.db.variant_fields(struct_id.into());
310+
let data = struct_id.fields(self.db);
311311
if data.shape != FieldsShape::Record {
312312
return;
313313
};
@@ -468,7 +468,7 @@ impl<'a> DeclValidator<'a> {
468468

469469
/// Check incorrect names for fields of enum variant.
470470
fn validate_enum_variant_fields(&mut self, variant_id: EnumVariantId) {
471-
let variant_data = self.db.variant_fields(variant_id.into());
471+
let variant_data = variant_id.fields(self.db);
472472
if variant_data.shape != FieldsShape::Record {
473473
return;
474474
};

0 commit comments

Comments
 (0)