3949: Cleanup cfg handling in items r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2020-04-11 15:55:26 +00:00 committed by GitHub
commit e7a68c8f55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 102 additions and 53 deletions

View File

@ -4,7 +4,6 @@ use std::sync::Arc;
use either::Either;
use hir_expand::{
hygiene::Hygiene,
name::{AsName, Name},
InFile,
};
@ -13,7 +12,7 @@ use ra_prof::profile;
use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner};
use crate::{
attr::Attrs, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace,
body::CfgExpander, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace,
type_ref::TypeRef, visibility::RawVisibility, EnumId, HasModule, LocalEnumVariantId,
LocalStructFieldId, Lookup, ModuleId, StructId, UnionId, VariantId,
};
@ -125,8 +124,9 @@ fn lower_enum(
impl VariantData {
fn new(db: &dyn DefDatabase, flavor: InFile<ast::StructKind>, module_id: ModuleId) -> Self {
let mut expander = CfgExpander::new(db, flavor.file_id, module_id.krate);
let mut trace = Trace::new_for_arena();
match lower_struct(db, &mut trace, &flavor, module_id) {
match lower_struct(db, &mut expander, &mut trace, &flavor) {
StructKind::Tuple => VariantData::Tuple(trace.into_arena()),
StructKind::Record => VariantData::Record(trace.into_arena()),
StructKind::Unit => VariantData::Unit,
@ -178,8 +178,9 @@ impl HasChildSource for VariantId {
it.lookup(db).container.module(db),
),
};
let mut expander = CfgExpander::new(db, src.file_id, module_id.krate);
let mut trace = Trace::new_for_map();
lower_struct(db, &mut trace, &src, module_id);
lower_struct(db, &mut expander, &mut trace, &src);
src.with_value(trace.into_map())
}
}
@ -193,16 +194,15 @@ pub enum StructKind {
fn lower_struct(
db: &dyn DefDatabase,
expander: &mut CfgExpander,
trace: &mut Trace<StructFieldData, Either<ast::TupleFieldDef, ast::RecordFieldDef>>,
ast: &InFile<ast::StructKind>,
module_id: ModuleId,
) -> StructKind {
let crate_graph = db.crate_graph();
match &ast.value {
ast::StructKind::Tuple(fl) => {
for (i, fd) in fl.fields().enumerate() {
let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id));
if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
let attrs = expander.parse_attrs(&fd);
if !expander.is_cfg_enabled(&attrs) {
continue;
}
@ -219,8 +219,8 @@ fn lower_struct(
}
ast::StructKind::Record(fl) => {
for fd in fl.fields() {
let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id));
if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
let attrs = expander.parse_attrs(&fd);
if !expander.is_cfg_enabled(&attrs) {
continue;
}

View File

@ -93,6 +93,7 @@ impl Attrs {
}
pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> bool {
// FIXME: handle cfg_attr :-)
self.by_key("cfg").tt_values().all(|tt| cfg_options.is_cfg_enabled(tt) != Some(false))
}
}

View File

@ -14,6 +14,7 @@ use ra_syntax::{ast, AstNode, AstPtr};
use rustc_hash::FxHashMap;
use crate::{
attr::Attrs,
db::DefDatabase,
expr::{Expr, ExprId, Pat, PatId},
item_scope::BuiltinShadowMode,
@ -23,26 +24,62 @@ use crate::{
src::HasSource,
AsMacroCall, DefWithBodyId, HasModule, Lookup, ModuleId,
};
use ra_cfg::CfgOptions;
use ra_db::CrateId;
/// A subser of Exander that only deals with cfg attributes. We only need it to
/// avoid cyclic queries in crate def map during enum processing.
pub(crate) struct CfgExpander {
cfg_options: CfgOptions,
hygiene: Hygiene,
}
pub(crate) struct Expander {
cfg_expander: CfgExpander,
crate_def_map: Arc<CrateDefMap>,
current_file_id: HirFileId,
hygiene: Hygiene,
ast_id_map: Arc<AstIdMap>,
module: ModuleId,
recursive_limit: usize,
}
impl CfgExpander {
pub(crate) fn new(
db: &dyn DefDatabase,
current_file_id: HirFileId,
krate: CrateId,
) -> CfgExpander {
let hygiene = Hygiene::new(db.upcast(), current_file_id);
let cfg_options = db.crate_graph()[krate].cfg_options.clone();
CfgExpander { cfg_options, hygiene }
}
pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs {
Attrs::new(owner, &self.hygiene)
}
pub(crate) fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
attrs.is_cfg_enabled(&self.cfg_options)
}
}
impl Expander {
pub(crate) fn new(
db: &dyn DefDatabase,
current_file_id: HirFileId,
module: ModuleId,
) -> Expander {
let cfg_expander = CfgExpander::new(db, current_file_id, module.krate);
let crate_def_map = db.crate_def_map(module.krate);
let hygiene = Hygiene::new(db.upcast(), current_file_id);
let ast_id_map = db.ast_id_map(current_file_id);
Expander { crate_def_map, current_file_id, hygiene, ast_id_map, module, recursive_limit: 0 }
Expander {
cfg_expander,
crate_def_map,
current_file_id,
ast_id_map,
module,
recursive_limit: 0,
}
}
pub(crate) fn enter_expand<T: ast::AstNode>(
@ -75,7 +112,7 @@ impl Expander {
ast_id_map: mem::take(&mut self.ast_id_map),
bomb: DropBomb::new("expansion mark dropped"),
};
self.hygiene = Hygiene::new(db.upcast(), file_id);
self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id);
self.current_file_id = file_id;
self.ast_id_map = db.ast_id_map(file_id);
self.recursive_limit += 1;
@ -91,7 +128,7 @@ impl Expander {
}
pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
self.hygiene = Hygiene::new(db.upcast(), mark.file_id);
self.cfg_expander.hygiene = Hygiene::new(db.upcast(), mark.file_id);
self.current_file_id = mark.file_id;
self.ast_id_map = mem::take(&mut mark.ast_id_map);
self.recursive_limit -= 1;
@ -102,8 +139,16 @@ impl Expander {
InFile { file_id: self.current_file_id, value }
}
pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs {
self.cfg_expander.parse_attrs(owner)
}
pub(crate) fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
self.cfg_expander.is_cfg_enabled(attrs)
}
fn parse_path(&mut self, path: ast::Path) -> Option<Path> {
Path::from_src(path, &self.hygiene)
Path::from_src(path, &self.cfg_expander.hygiene)
}
fn resolve_path_as_macro(&self, db: &dyn DefDatabase, path: &ModPath) -> Option<MacroDefId> {

View File

@ -2,9 +2,7 @@
//! representation.
use either::Either;
use hir_expand::{
hygiene::Hygiene,
name::{name, AsName, Name},
MacroDefId, MacroDefKind,
};
@ -18,10 +16,8 @@ use ra_syntax::{
};
use test_utils::tested_by;
use super::{ExprSource, PatSource};
use crate::{
adt::StructKind,
attr::Attrs,
body::{Body, BodySourceMap, Expander, PatPtr, SyntheticSyntax},
builtin_type::{BuiltinFloat, BuiltinInt},
db::DefDatabase,
@ -33,10 +29,12 @@ use crate::{
path::GenericArgs,
path::Path,
type_ref::{Mutability, TypeRef},
AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, HasModule, Intern,
ModuleDefId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId,
StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
};
use super::{ExprSource, PatSource};
pub(super) fn lower(
db: &dyn DefDatabase,
def: DefWithBodyId,
@ -300,7 +298,6 @@ impl ExprCollector<'_> {
self.alloc_expr(Expr::Return { expr }, syntax_ptr)
}
ast::Expr::RecordLit(e) => {
let crate_graph = self.db.crate_graph();
let path = e.path().and_then(|path| self.expander.parse_path(path));
let mut field_ptrs = Vec::new();
let record_lit = if let Some(nfl) = e.record_field_list() {
@ -308,13 +305,8 @@ impl ExprCollector<'_> {
.fields()
.inspect(|field| field_ptrs.push(AstPtr::new(field)))
.filter_map(|field| {
let module_id = ContainerId::DefWithBodyId(self.def).module(self.db);
let attrs = Attrs::new(
&field,
&Hygiene::new(self.db.upcast(), self.expander.current_file_id),
);
if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
let attrs = self.expander.parse_attrs(&field);
if !self.expander.is_cfg_enabled(&attrs) {
return None;
}

View File

@ -20,7 +20,7 @@ use crate::{
type_ref::{Mutability, TypeBound, TypeRef},
visibility::RawVisibility,
AssocContainerId, AssocItemId, ConstId, ConstLoc, Expander, FunctionId, FunctionLoc, HasModule,
ImplId, Intern, Lookup, ModuleId, StaticId, TraitId, TypeAliasId, TypeAliasLoc,
ImplId, Intern, Lookup, StaticId, TraitId, TypeAliasId, TypeAliasLoc,
};
#[derive(Debug, Clone, PartialEq, Eq)]
@ -218,10 +218,17 @@ impl ImplData {
let mut items = Vec::new();
if let Some(item_list) = src.value.item_list() {
items.extend(collect_impl_items(db, item_list.impl_items(), src.file_id, id));
let mut expander = Expander::new(db, impl_loc.ast_id.file_id, module_id);
items.extend(collect_impl_items(
db,
&mut expander,
item_list.impl_items(),
src.file_id,
id,
));
items.extend(collect_impl_items_in_macros(
db,
module_id,
&mut expander,
&src.with_value(item_list),
id,
));
@ -268,18 +275,17 @@ impl ConstData {
fn collect_impl_items_in_macros(
db: &dyn DefDatabase,
module_id: ModuleId,
expander: &mut Expander,
impl_def: &InFile<ast::ItemList>,
id: ImplId,
) -> Vec<AssocItemId> {
let mut expander = Expander::new(db, impl_def.file_id, module_id);
let mut res = Vec::new();
// We set a limit to protect against infinite recursion
let limit = 100;
for m in impl_def.value.syntax().children().filter_map(ast::MacroCall::cast) {
res.extend(collect_impl_items_in_macro(db, &mut expander, m, id, limit))
res.extend(collect_impl_items_in_macro(db, expander, m, id, limit))
}
res
@ -300,6 +306,7 @@ fn collect_impl_items_in_macro(
let items: InFile<ast::MacroItems> = expander.to_source(items);
let mut res = collect_impl_items(
db,
expander,
items.value.items().filter_map(|it| ImplItem::cast(it.syntax().clone())),
items.file_id,
id,
@ -319,32 +326,26 @@ fn collect_impl_items_in_macro(
fn collect_impl_items(
db: &dyn DefDatabase,
expander: &mut Expander,
impl_items: impl Iterator<Item = ImplItem>,
file_id: crate::HirFileId,
id: ImplId,
) -> Vec<AssocItemId> {
let items = db.ast_id_map(file_id);
let crate_graph = db.crate_graph();
let module_id = id.lookup(db).container.module(db);
impl_items
.filter_map(|item_node| match item_node {
ast::ImplItem::FnDef(it) => {
let attrs = expander.parse_attrs(&it);
if !expander.is_cfg_enabled(&attrs) {
return None;
}
let def = FunctionLoc {
container: AssocContainerId::ImplId(id),
ast_id: AstId::new(file_id, items.ast_id(&it)),
}
.intern(db);
if !db
.function_data(def)
.attrs
.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options)
{
None
} else {
Some(def.into())
}
Some(def.into())
}
ast::ImplItem::ConstDef(it) => {
let def = ConstLoc {

View File

@ -462,6 +462,14 @@ impl DefCollector<'_> {
Some(ModuleDefId::AdtId(AdtId::EnumId(e))) => {
tested_by!(glob_enum);
// glob import from enum => just import all the variants
// XXX: urgh, so this works by accident! Here, we look at
// the enum data, and, in theory, this might require us to
// look back at the crate_def_map, creating a cycle. For
// example, `enum E { crate::some_macro!(); }`. Luckely, the
// only kind of macro that is allowed inside enum is a
// `cfg_macro`, and we don't need to run name resolution for
// it, but this is sheer luck!
let enum_data = self.db.enum_data(e);
let resolutions = enum_data
.variants
@ -977,11 +985,7 @@ impl ModCollector<'_, '_> {
}
fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
// FIXME: handle cfg_attr :-)
attrs
.by_key("cfg")
.tt_values()
.all(|tt| self.def_collector.cfg_options.is_cfg_enabled(tt) != Some(false))
attrs.is_cfg_enabled(self.def_collector.cfg_options)
}
}

View File

@ -32,6 +32,9 @@ fn typing_inside_a_function_should_not_invalidate_def_map() {
use crate::foo::bar::Baz;
enum E { A, B }
use E::*;
fn foo() -> i32 {
1 + 1
}
@ -46,6 +49,9 @@ fn typing_inside_a_function_should_not_invalidate_def_map() {
use crate::foo::bar::Baz;
enum E { A, B }
use E::*;
fn foo() -> i32 { 92 }
",
);