5081: Fix a panic with malformed inner items r=jonas-schievink a=jonas-schievink

bors r+

Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>
This commit is contained in:
bors[bot] 2020-06-26 16:12:22 +00:00 committed by GitHub
commit 89277e7a42
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 61 deletions

View file

@ -5,7 +5,7 @@ use either::Either;
use hir_expand::{ use hir_expand::{
hygiene::Hygiene, hygiene::Hygiene,
name::{name, AsName, Name}, name::{name, AsName, Name},
AstId, HirFileId, MacroDefId, MacroDefKind, HirFileId, MacroDefId, MacroDefKind,
}; };
use ra_arena::Arena; use ra_arena::Arena;
use ra_syntax::{ use ra_syntax::{
@ -27,7 +27,7 @@ use crate::{
LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
}, },
item_scope::BuiltinShadowMode, item_scope::BuiltinShadowMode,
item_tree::{FileItemTreeId, ItemTree, ItemTreeNode}, item_tree::{ItemTree, ItemTreeId, ItemTreeNode},
path::{GenericArgs, Path}, path::{GenericArgs, Path},
type_ref::{Mutability, Rawness, TypeRef}, type_ref::{Mutability, Rawness, TypeRef},
AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId, AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId,
@ -37,7 +37,7 @@ use crate::{
use super::{ExprSource, PatSource}; use super::{ExprSource, PatSource};
use ast::AstChildren; use ast::AstChildren;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use std::sync::Arc; use std::{any::type_name, sync::Arc};
pub(crate) struct LowerCtx { pub(crate) struct LowerCtx {
hygiene: Hygiene, hygiene: Hygiene,
@ -561,17 +561,30 @@ impl ExprCollector<'_> {
} }
} }
fn find_inner_item<S: ItemTreeNode>(&self, id: AstId<ast::ModuleItem>) -> FileItemTreeId<S> { fn find_inner_item<N: ItemTreeNode>(&self, ast: &N::Source) -> Option<ItemTreeId<N>> {
let id = self.expander.ast_id(ast);
let tree = &self.item_trees[&id.file_id]; let tree = &self.item_trees[&id.file_id];
// FIXME: This probably breaks with `use` items, since they produce multiple item tree nodes // FIXME: This probably breaks with `use` items, since they produce multiple item tree nodes
// Root file (non-macro). // Root file (non-macro).
tree.all_inner_items() let item_tree_id = tree
.all_inner_items()
.chain(tree.top_level_items().iter().copied()) .chain(tree.top_level_items().iter().copied())
.filter_map(|mod_item| mod_item.downcast::<S>()) .filter_map(|mod_item| mod_item.downcast::<N>())
.find(|tree_id| tree[*tree_id].ast_id().upcast() == id.value) .find(|tree_id| tree[*tree_id].ast_id().upcast() == id.value.upcast())
.unwrap_or_else(|| panic!("couldn't find inner item for {:?}", id)) .or_else(|| {
log::debug!(
"couldn't find inner {} item for {:?} (AST: `{}` - {:?})",
type_name::<N>(),
id,
ast.syntax(),
ast.syntax(),
);
None
})?;
Some(ItemTreeId::new(id.file_id, item_tree_id))
} }
fn collect_expr_opt(&mut self, expr: Option<ast::Expr>) -> ExprId { fn collect_expr_opt(&mut self, expr: Option<ast::Expr>) -> ExprId {
@ -611,82 +624,45 @@ impl ExprCollector<'_> {
.filter_map(|item| { .filter_map(|item| {
let (def, name): (ModuleDefId, Option<ast::Name>) = match item { let (def, name): (ModuleDefId, Option<ast::Name>) = match item {
ast::ModuleItem::FnDef(def) => { ast::ModuleItem::FnDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
( (
FunctionLoc { container: container.into(), id: ast_id.with_value(id) } FunctionLoc { container: container.into(), id }.intern(self.db).into(),
.intern(self.db)
.into(),
def.name(), def.name(),
) )
} }
ast::ModuleItem::TypeAliasDef(def) => { ast::ModuleItem::TypeAliasDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
( (
TypeAliasLoc { container: container.into(), id: ast_id.with_value(id) } TypeAliasLoc { container: container.into(), id }.intern(self.db).into(),
.intern(self.db)
.into(),
def.name(), def.name(),
) )
} }
ast::ModuleItem::ConstDef(def) => { ast::ModuleItem::ConstDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
( (
ConstLoc { container: container.into(), id: ast_id.with_value(id) } ConstLoc { container: container.into(), id }.intern(self.db).into(),
.intern(self.db)
.into(),
def.name(), def.name(),
) )
} }
ast::ModuleItem::StaticDef(def) => { ast::ModuleItem::StaticDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast())); (StaticLoc { container, id }.intern(self.db).into(), def.name())
(
StaticLoc { container, id: ast_id.with_value(id) }
.intern(self.db)
.into(),
def.name(),
)
} }
ast::ModuleItem::StructDef(def) => { ast::ModuleItem::StructDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast())); (StructLoc { container, id }.intern(self.db).into(), def.name())
(
StructLoc { container, id: ast_id.with_value(id) }
.intern(self.db)
.into(),
def.name(),
)
} }
ast::ModuleItem::EnumDef(def) => { ast::ModuleItem::EnumDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast())); (EnumLoc { container, id }.intern(self.db).into(), def.name())
(
EnumLoc { container, id: ast_id.with_value(id) }.intern(self.db).into(),
def.name(),
)
} }
ast::ModuleItem::UnionDef(def) => { ast::ModuleItem::UnionDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast())); (UnionLoc { container, id }.intern(self.db).into(), def.name())
(
UnionLoc { container, id: ast_id.with_value(id) }
.intern(self.db)
.into(),
def.name(),
)
} }
ast::ModuleItem::TraitDef(def) => { ast::ModuleItem::TraitDef(def) => {
let ast_id = self.expander.ast_id(&def); let id = self.find_inner_item(&def)?;
let id = self.find_inner_item(ast_id.map(|id| id.upcast())); (TraitLoc { container, id }.intern(self.db).into(), def.name())
(
TraitLoc { container, id: ast_id.with_value(id) }
.intern(self.db)
.into(),
def.name(),
)
} }
ast::ModuleItem::ExternBlock(_) => return None, // FIXME: collect from extern blocks ast::ModuleItem::ExternBlock(_) => return None, // FIXME: collect from extern blocks
ast::ModuleItem::ImplDef(_) ast::ModuleItem::ImplDef(_)

View file

@ -337,6 +337,19 @@ fn foo() {
); );
} }
#[test]
fn broken_inner_item() {
do_check(
r"
fn foo() {
trait {}
<|>
}
",
&[],
);
}
fn do_check_local_name(ra_fixture: &str, expected_offset: u32) { fn do_check_local_name(ra_fixture: &str, expected_offset: u32) {
let (db, position) = TestDB::with_position(ra_fixture); let (db, position) = TestDB::with_position(ra_fixture);
let file_id = position.file_id; let file_id = position.file_id;