Skip to content

Commit b2ed56f

Browse files
VeykrilVeykril
committed
Address reviews
1 parent 15f13d9 commit b2ed56f

File tree

11 files changed

+70
-77
lines changed

11 files changed

+70
-77
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ use thin_vec::ThinVec;
1010
use triomphe::Arc;
1111

1212
use crate::{
13-
AttrDefId, BlockId, BlockLoc, ConstBlockId, ConstBlockLoc, ConstId, ConstLoc, DefWithBodyId,
14-
EnumId, EnumLoc, EnumVariantId, EnumVariantLoc, ExternBlockId, ExternBlockLoc, ExternCrateId,
15-
ExternCrateLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc, LocalFieldId, Macro2Id,
16-
Macro2Loc, MacroId, MacroRulesId, MacroRulesLoc, MacroRulesLocFlags, ProcMacroId, ProcMacroLoc,
17-
StaticId, StaticLoc, StructId, StructLoc, TraitAliasId, TraitAliasLoc, TraitId, TraitLoc,
18-
TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, UseId, UseLoc, VariantId,
13+
AttrDefId, BlockId, BlockLoc, ConstId, ConstLoc, DefWithBodyId, EnumId, EnumLoc, EnumVariantId,
14+
EnumVariantLoc, ExternBlockId, ExternBlockLoc, ExternCrateId, ExternCrateLoc, FunctionId,
15+
FunctionLoc, GenericDefId, ImplId, ImplLoc, LocalFieldId, Macro2Id, Macro2Loc, MacroId,
16+
MacroRulesId, MacroRulesLoc, MacroRulesLocFlags, ProcMacroId, ProcMacroLoc, StaticId,
17+
StaticLoc, StructId, StructLoc, TraitAliasId, TraitAliasLoc, TraitId, TraitLoc, TypeAliasId,
18+
TypeAliasLoc, UnionId, UnionLoc, UseId, UseLoc, VariantId,
1919
attr::{Attrs, AttrsWithOwner},
2020
expr_store::{
2121
Body, BodySourceMap, ExpressionStore, ExpressionStoreSourceMap, scope::ExprScopes,
@@ -97,8 +97,6 @@ pub trait InternDatabase: RootQueryDb {
9797

9898
#[salsa::interned]
9999
fn intern_block(&self, loc: BlockLoc) -> BlockId;
100-
#[salsa::interned]
101-
fn intern_anonymous_const(&self, id: ConstBlockLoc) -> ConstBlockId;
102100
}
103101

104102
#[query_group::query_group]

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ pub(crate) fn lower_impl(
218218
) -> (ExpressionStore, ExpressionStoreSourceMap, TypeRefId, Option<TraitRef>, Arc<GenericParams>) {
219219
let mut expr_collector = ExprCollector::new(db, module, impl_syntax.file_id);
220220
let self_ty =
221-
expr_collector.lower_type_ref_opt(impl_syntax.value.self_ty(), &mut |_| TypeRef::Error);
221+
expr_collector.lower_type_ref_opt_disallow_impl_trait(impl_syntax.value.self_ty());
222222
let trait_ = impl_syntax.value.trait_().and_then(|it| match &it {
223223
ast::Type::PathType(path_type) => {
224224
let path = expr_collector.lower_path_type(path_type, &mut |_| TypeRef::Error)?;
@@ -580,10 +580,10 @@ impl ExprCollector<'_> {
580580
}
581581
ast::Type::ArrayType(inner) => {
582582
let len = self.lower_const_arg_opt(inner.const_arg());
583-
TypeRef::Array(Box::new(ArrayType {
583+
TypeRef::Array(ArrayType {
584584
ty: self.lower_type_ref_opt(inner.ty(), impl_trait_lower_fn),
585585
len,
586-
}))
586+
})
587587
}
588588
ast::Type::SliceType(inner) => {
589589
TypeRef::Slice(self.lower_type_ref_opt(inner.ty(), impl_trait_lower_fn))
@@ -674,6 +674,10 @@ impl ExprCollector<'_> {
674674
self.alloc_type_ref(ty, AstPtr::new(&node))
675675
}
676676

677+
pub(crate) fn lower_type_ref_disallow_impl_trait(&mut self, node: ast::Type) -> TypeRefId {
678+
self.lower_type_ref(node, &mut |_| TypeRef::Error)
679+
}
680+
677681
pub(crate) fn lower_type_ref_opt(
678682
&mut self,
679683
node: Option<ast::Type>,
@@ -685,6 +689,13 @@ impl ExprCollector<'_> {
685689
}
686690
}
687691

692+
pub(crate) fn lower_type_ref_opt_disallow_impl_trait(
693+
&mut self,
694+
node: Option<ast::Type>,
695+
) -> TypeRefId {
696+
self.lower_type_ref_opt(node, &mut |_| TypeRef::Error)
697+
}
698+
688699
fn alloc_type_ref(&mut self, type_ref: TypeRef, node: TypePtr) -> TypeRefId {
689700
let id = self.store.types.alloc(type_ref);
690701
let ptr = self.expander.in_file(node);
@@ -1194,7 +1205,7 @@ impl ExprCollector<'_> {
11941205
ast::Expr::TryExpr(e) => self.collect_try_operator(syntax_ptr, e),
11951206
ast::Expr::CastExpr(e) => {
11961207
let expr = self.collect_expr_opt(e.expr());
1197-
let type_ref = self.lower_type_ref_opt(e.ty(), &mut |_| TypeRef::Error);
1208+
let type_ref = self.lower_type_ref_opt_disallow_impl_trait(e.ty());
11981209
self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr)
11991210
}
12001211
ast::Expr::RefExpr(e) => {
@@ -1227,15 +1238,15 @@ impl ExprCollector<'_> {
12271238
for param in pl.params() {
12281239
let pat = this.collect_pat_top(param.pat());
12291240
let type_ref =
1230-
param.ty().map(|it| this.lower_type_ref(it, &mut |_| TypeRef::Error));
1241+
param.ty().map(|it| this.lower_type_ref_disallow_impl_trait(it));
12311242
args.push(pat);
12321243
arg_types.push(type_ref);
12331244
}
12341245
}
12351246
let ret_type = e
12361247
.ret_type()
12371248
.and_then(|r| r.ty())
1238-
.map(|it| this.lower_type_ref(it, &mut |_| TypeRef::Error));
1249+
.map(|it| this.lower_type_ref_disallow_impl_trait(it));
12391250

12401251
let prev_is_lowering_coroutine = mem::take(&mut this.is_lowering_coroutine);
12411252
let prev_try_block_label = this.current_try_block_label.take();
@@ -1362,7 +1373,7 @@ impl ExprCollector<'_> {
13621373
ast::Expr::UnderscoreExpr(_) => self.alloc_expr(Expr::Underscore, syntax_ptr),
13631374
ast::Expr::AsmExpr(e) => self.lower_inline_asm(e, syntax_ptr),
13641375
ast::Expr::OffsetOfExpr(e) => {
1365-
let container = self.lower_type_ref_opt(e.ty(), &mut |_| TypeRef::Error);
1376+
let container = self.lower_type_ref_opt_disallow_impl_trait(e.ty());
13661377
let fields = e.fields().map(|it| it.as_name()).collect();
13671378
self.alloc_expr(Expr::OffsetOf(OffsetOf { container, fields }), syntax_ptr)
13681379
}
@@ -1966,7 +1977,7 @@ impl ExprCollector<'_> {
19661977
return;
19671978
}
19681979
let pat = self.collect_pat_top(stmt.pat());
1969-
let type_ref = stmt.ty().map(|it| self.lower_type_ref(it, &mut |_| TypeRef::Error));
1980+
let type_ref = stmt.ty().map(|it| self.lower_type_ref_disallow_impl_trait(it));
19701981
let initializer = stmt.initializer().map(|e| self.collect_expr(e));
19711982
let else_branch = stmt
19721983
.let_else()

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

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use la_arena::{Arena, ArenaMap, Idx, IdxRange, RawIdx};
44
use triomphe::Arc;
55

66
use crate::{
7-
BlockId, ConstBlockId, DefWithBodyId,
7+
BlockId, DefWithBodyId,
88
db::DefDatabase,
99
expr_store::{Body, ExpressionStore, HygieneId},
1010
hir::{Binding, BindingId, Expr, ExprId, Item, LabelId, Pat, PatId, Statement},
@@ -53,9 +53,7 @@ pub struct ScopeData {
5353
impl ExprScopes {
5454
pub(crate) fn expr_scopes_query(db: &dyn DefDatabase, def: DefWithBodyId) -> Arc<ExprScopes> {
5555
let body = db.body(def);
56-
let mut scopes = ExprScopes::new_body(&body, |const_block| {
57-
db.lookup_intern_anonymous_const(const_block).root
58-
});
56+
let mut scopes = ExprScopes::new_body(&body);
5957
scopes.shrink_to_fit();
6058
Arc::new(scopes)
6159
}
@@ -104,10 +102,7 @@ fn empty_entries(idx: usize) -> IdxRange<ScopeEntry> {
104102
}
105103

106104
impl ExprScopes {
107-
fn new_body(
108-
body: &Body,
109-
resolve_const_block: impl (Fn(ConstBlockId) -> ExprId) + Copy,
110-
) -> ExprScopes {
105+
fn new_body(body: &Body) -> ExprScopes {
111106
let mut scopes = ExprScopes {
112107
scopes: Arena::default(),
113108
scope_entries: Arena::default(),
@@ -118,7 +113,7 @@ impl ExprScopes {
118113
scopes.add_bindings(body, root, self_param, body.binding_hygiene(self_param));
119114
}
120115
scopes.add_params_bindings(body, root, &body.params);
121-
compute_expr_scopes(body.body_expr, body, &mut scopes, &mut root, resolve_const_block);
116+
compute_expr_scopes(body.body_expr, body, &mut scopes, &mut root);
122117
scopes
123118
}
124119

@@ -221,23 +216,22 @@ fn compute_block_scopes(
221216
store: &ExpressionStore,
222217
scopes: &mut ExprScopes,
223218
scope: &mut ScopeId,
224-
resolve_const_block: impl (Fn(ConstBlockId) -> ExprId) + Copy,
225219
) {
226220
for stmt in statements {
227221
match stmt {
228222
Statement::Let { pat, initializer, else_branch, .. } => {
229223
if let Some(expr) = initializer {
230-
compute_expr_scopes(*expr, store, scopes, scope, resolve_const_block);
224+
compute_expr_scopes(*expr, store, scopes, scope);
231225
}
232226
if let Some(expr) = else_branch {
233-
compute_expr_scopes(*expr, store, scopes, scope, resolve_const_block);
227+
compute_expr_scopes(*expr, store, scopes, scope);
234228
}
235229

236230
*scope = scopes.new_scope(*scope);
237231
scopes.add_pat_bindings(store, *scope, *pat);
238232
}
239233
Statement::Expr { expr, .. } => {
240-
compute_expr_scopes(*expr, store, scopes, scope, resolve_const_block);
234+
compute_expr_scopes(*expr, store, scopes, scope);
241235
}
242236
Statement::Item(Item::MacroDef(macro_id)) => {
243237
*scope = scopes.new_macro_def_scope(*scope, macro_id.clone());
@@ -246,7 +240,7 @@ fn compute_block_scopes(
246240
}
247241
}
248242
if let Some(expr) = tail {
249-
compute_expr_scopes(expr, store, scopes, scope, resolve_const_block);
243+
compute_expr_scopes(expr, store, scopes, scope);
250244
}
251245
}
252246

@@ -255,13 +249,12 @@ fn compute_expr_scopes(
255249
store: &ExpressionStore,
256250
scopes: &mut ExprScopes,
257251
scope: &mut ScopeId,
258-
resolve_const_block: impl (Fn(ConstBlockId) -> ExprId) + Copy,
259252
) {
260253
let make_label =
261254
|label: &Option<LabelId>| label.map(|label| (label, store.labels[label].name.clone()));
262255

263256
let compute_expr_scopes = |scopes: &mut ExprScopes, expr: ExprId, scope: &mut ScopeId| {
264-
compute_expr_scopes(expr, store, scopes, scope, resolve_const_block)
257+
compute_expr_scopes(expr, store, scopes, scope)
265258
};
266259

267260
scopes.set_scope(expr, *scope);
@@ -271,7 +264,7 @@ fn compute_expr_scopes(
271264
// Overwrite the old scope for the block expr, so that every block scope can be found
272265
// via the block itself (important for blocks that only contain items, no expressions).
273266
scopes.set_scope(expr, scope);
274-
compute_block_scopes(statements, *tail, store, scopes, &mut scope, resolve_const_block);
267+
compute_block_scopes(statements, *tail, store, scopes, &mut scope);
275268
}
276269
Expr::Const(id) => {
277270
let mut scope = scopes.root_scope();
@@ -282,7 +275,7 @@ fn compute_expr_scopes(
282275
// Overwrite the old scope for the block expr, so that every block scope can be found
283276
// via the block itself (important for blocks that only contain items, no expressions).
284277
scopes.set_scope(expr, scope);
285-
compute_block_scopes(statements, *tail, store, scopes, &mut scope, resolve_const_block);
278+
compute_block_scopes(statements, *tail, store, scopes, &mut scope);
286279
}
287280
Expr::Loop { body: body_expr, label } => {
288281
let mut scope = scopes.new_labeled_scope(*scope, make_label(label));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ pub enum TypeRef {
129129
Path(Path),
130130
RawPtr(TypeRefId, Mutability),
131131
Reference(Box<RefType>),
132-
Array(Box<ArrayType>),
132+
Array(ArrayType),
133133
Slice(TypeRefId),
134134
/// A fn pointer. Last element of the vector is the return type.
135135
Fn(Box<FnType>),

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,7 @@ impl Printer<'_> {
232232
let Function { name, visibility, ast_id } = &self.tree[it];
233233
self.print_ast_id(ast_id.erase());
234234
self.print_visibility(*visibility);
235-
w!(self, "fn {}", name.display(self.db.upcast(), self.edition));
236-
w!(self, "(");
237-
w!(self, ")");
238-
wln!(self, ";");
235+
wln!(self, "fn {};", name.display(self.db.upcast(), self.edition));
239236
}
240237
ModItem::Struct(it) => {
241238
let Struct { visibility, name, fields, shape: kind, ast_id } = &self.tree[it];
@@ -299,7 +296,7 @@ impl Printer<'_> {
299296
let Trait { name, visibility, items, ast_id } = &self.tree[it];
300297
self.print_ast_id(ast_id.erase());
301298
self.print_visibility(*visibility);
302-
w!(self, "trait {}", name.display(self.db.upcast(), self.edition));
299+
w!(self, "trait {} {{", name.display(self.db.upcast(), self.edition));
303300
self.indented(|this| {
304301
for item in &**items {
305302
this.print_mod_item((*item).into());
@@ -311,14 +308,12 @@ impl Printer<'_> {
311308
let TraitAlias { name, visibility, ast_id } = &self.tree[it];
312309
self.print_ast_id(ast_id.erase());
313310
self.print_visibility(*visibility);
314-
w!(self, "trait {}", name.display(self.db.upcast(), self.edition));
315-
w!(self, ";");
316-
wln!(self);
311+
wln!(self, "trait {} = ..;", name.display(self.db.upcast(), self.edition));
317312
}
318313
ModItem::Impl(it) => {
319314
let Impl { items, ast_id } = &self.tree[it];
320315
self.print_ast_id(ast_id.erase());
321-
w!(self, "impl");
316+
w!(self, "impl {{");
322317
self.indented(|this| {
323318
for item in &**items {
324319
this.print_mod_item((*item).into());

‎crates/hir-def/src/item_tree/tests.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ extern "C" {
8787
8888
#[on_extern_fn]
8989
// AstId: 4
90-
pub(self) fn ex_fn();
90+
pub(self) fn ex_fn;
9191
}
9292
"##]],
9393
);
@@ -194,15 +194,15 @@ trait Tr: SuperTrait + 'lifetime {
194194
#[attr]
195195
#[inner_attr_in_fn]
196196
// AstId: 3
197-
pub(self) fn f();
197+
pub(self) fn f;
198198
199199
// AstId: 4
200-
pub(self) trait Tr
200+
pub(self) trait Tr {
201201
// AstId: 6
202202
pub(self) type Assoc;
203203
204204
// AstId: 7
205-
pub(self) fn method();
205+
pub(self) fn method;
206206
}
207207
"#]],
208208
);
@@ -232,7 +232,7 @@ mod outline;
232232
pub(self) use super::*;
233233
234234
// AstId: 4
235-
pub(self) fn fn_in_module();
235+
pub(self) fn fn_in_module;
236236
}
237237
238238
// AstId: 2

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -317,19 +317,6 @@ pub struct BlockLoc {
317317
}
318318
impl_intern!(BlockId, BlockLoc, intern_block, lookup_intern_block);
319319

320-
// Id of the anonymous const block expression and patterns. This is very similar to `ClosureId` and
321-
// shouldn't be a `DefWithBodyId` since its type inference is dependent on its parent.
322-
impl_intern!(ConstBlockId, ConstBlockLoc, intern_anonymous_const, lookup_intern_anonymous_const);
323-
324-
#[derive(Debug, Hash, PartialEq, Eq, Clone)]
325-
pub struct ConstBlockLoc {
326-
/// The parent of the anonymous const block.
327-
pub parent: GenericDefId,
328-
/// The root expression of this const block in the parent body.
329-
pub root: hir::ExprId,
330-
pub(crate) signature_owner: bool,
331-
}
332-
333320
/// A `ModuleId` that is always a crate's root module.
334321
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
335322
pub struct CrateRootModuleId {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,7 @@ impl Resolver {
597597
res.map
598598
}
599599

600+
/// Note: Not to be used directly within hir-def/hir-ty
600601
pub fn extern_crate_decls_in_scope<'a>(
601602
&'a self,
602603
db: &'a dyn DefDatabase,
@@ -605,7 +606,6 @@ impl Resolver {
605606
.scope
606607
.extern_crate_decls()
607608
.filter_map(|id| {
608-
// FIXME: this should probably not hit the item tree?
609609
let loc = id.lookup(db);
610610
let tree = loc.item_tree_id().item_tree(db);
611611
match &tree[loc.id.value].alias {

0 commit comments

Comments
 (0)