From cebc018e2ab1779ab68bfe8f4d0bb38986c52596 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 22 Apr 2023 14:29:28 +0200 Subject: [PATCH] Remove unnecessary is_derive field from MacroCallKind::Attr --- crates/hir-def/src/data.rs | 1 - crates/hir-def/src/lib.rs | 2 - crates/hir-def/src/nameres/attr_resolution.rs | 1 - crates/hir-def/src/nameres/collector.rs | 12 +- crates/hir-expand/src/builtin_attr_macro.rs | 2 +- crates/hir-expand/src/db.rs | 17 ++- crates/hir-expand/src/lib.rs | 119 ++++++++++-------- 7 files changed, 75 insertions(+), 79 deletions(-) diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 58becd603b2..7c6c089d65b 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -623,7 +623,6 @@ fn collect(&mut self, item_tree: &ItemTree, tree_id: TreeId, assoc_items: &[Asso ast_id, attr_args: Arc::new((tt::Subtree::empty(), Default::default())), invoc_attr_index: attr.id, - is_derive: false, }, attr.path().clone(), )); diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index b7f0e229eef..8e69a64e7e9 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -984,7 +984,6 @@ fn attr_macro_as_call_id( macro_attr: &Attr, krate: CrateId, def: MacroDefId, - is_derive: bool, ) -> MacroCallId { let arg = match macro_attr.input.as_deref() { Some(AttrInput::TokenTree(tt, map)) => ( @@ -1005,7 +1004,6 @@ fn attr_macro_as_call_id( ast_id: item_attr.ast_id, attr_args: Arc::new(arg), invoc_attr_index: macro_attr.id, - is_derive, }, ) } diff --git a/crates/hir-def/src/nameres/attr_resolution.rs b/crates/hir-def/src/nameres/attr_resolution.rs index 6fad01ec2f7..84271ef51d8 100644 --- a/crates/hir-def/src/nameres/attr_resolution.rs +++ b/crates/hir-def/src/nameres/attr_resolution.rs @@ -61,7 +61,6 @@ pub(crate) fn resolve_attr_macro( attr, self.krate, macro_id_to_def_id(db, def), - false, ))) } diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index ba93b2c70ef..51879dd5553 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -481,7 +481,6 @@ fn reseed_with_unresolved_attribute(&mut self) -> ReachedFixedPoint { Default::default(), )), invoc_attr_index: attr.id, - is_derive: false, }, attr.path().clone(), )); @@ -1273,7 +1272,6 @@ fn resolve_macros(&mut self) -> ReachedFixedPoint { attr, self.def_map.krate, def, - true, ); self.def_map.modules[directive.module_id] .scope @@ -1293,14 +1291,8 @@ fn resolve_macros(&mut self) -> ReachedFixedPoint { } // Not resolved to a derive helper or the derive attribute, so try to treat as a normal attribute. - let call_id = attr_macro_as_call_id( - self.db, - file_ast_id, - attr, - self.def_map.krate, - def, - false, - ); + let call_id = + attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); let loc: MacroCallLoc = self.db.lookup_intern_macro_call(call_id); // If proc attribute macro expansion is disabled, skip expanding it here diff --git a/crates/hir-expand/src/builtin_attr_macro.rs b/crates/hir-expand/src/builtin_attr_macro.rs index 277ecd93994..80695bc0656 100644 --- a/crates/hir-expand/src/builtin_attr_macro.rs +++ b/crates/hir-expand/src/builtin_attr_macro.rs @@ -96,7 +96,7 @@ fn derive_attr_expand( ) -> ExpandResult { let loc = db.lookup_intern_macro_call(id); let derives = match &loc.kind { - MacroCallKind::Attr { attr_args, is_derive: true, .. } => &attr_args.0, + MacroCallKind::Attr { attr_args, .. } if loc.def.is_attribute_derive() => &attr_args.0, _ => return ExpandResult::ok(tt::Subtree::empty()), }; pseudo_derive_attr_expansion(tt, derives) diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index bed04b3a343..c8de6954ee7 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -172,8 +172,8 @@ pub fn expand_speculative( ); let (attr_arg, token_id) = match loc.kind { - MacroCallKind::Attr { invoc_attr_index, is_derive, .. } => { - let attr = if is_derive { + MacroCallKind::Attr { invoc_attr_index, .. } => { + let attr = if loc.def.is_attribute_derive() { // for pseudo-derive expansion we actually pass the attribute itself only ast::Attr::cast(speculative_args.clone()) } else { @@ -285,8 +285,8 @@ fn parse_macro_expansion( // Note: // The final goal we would like to make all parse_macro success, // such that the following log will not call anyway. - let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id); - let node = loc.kind.to_node(db); + let loc = db.lookup_intern_macro_call(macro_file.macro_call_id); + let node = loc.to_node(db); // collect parent information for warning log let parents = std::iter::successors(loc.kind.file_id().call_node(db), |it| { @@ -360,7 +360,7 @@ fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet return None, + MacroCallKind::Attr { .. } if loc.def.is_attribute_derive() => return None, MacroCallKind::Attr { invoc_attr_index, .. } => { cov_mark::hit!(attribute_macro_attr_censoring); ast::Item::cast(node.clone())? @@ -442,7 +442,7 @@ fn macro_def( fn macro_expand(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult> { let _p = profile::span("macro_expand"); - let loc: MacroCallLoc = db.lookup_intern_macro_call(id); + let loc = db.lookup_intern_macro_call(id); if let Some(eager) = &loc.eager { return ExpandResult { value: eager.arg_or_expansion.clone(), err: eager.error.clone() }; } @@ -511,7 +511,7 @@ fn parse_macro_expansion_error( } fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult { - let loc: MacroCallLoc = db.lookup_intern_macro_call(id); + let loc = db.lookup_intern_macro_call(id); let Some(macro_arg) = db.macro_arg(id) else { return ExpandResult { value: tt::Subtree { @@ -547,8 +547,7 @@ fn hygiene_frame(db: &dyn ExpandDatabase, file_id: HirFileId) -> Arc ExpandTo { - let loc: MacroCallLoc = db.lookup_intern_macro_call(id); - loc.kind.expand_to() + db.lookup_intern_macro_call(id).expand_to() } fn token_tree_to_syntax_node( diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index cba448df2d9..ce0006d8422 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -168,8 +168,6 @@ pub enum MacroCallKind { /// Outer attributes are counted first, then inner attributes. This does not support /// out-of-line modules, which may have attributes spread across 2 files! invoc_attr_index: AttrId, - /// Whether this attribute is the `#[derive]` attribute. - is_derive: bool, }, } @@ -232,18 +230,17 @@ pub fn expansion_level(self, db: &dyn db::ExpandDatabase) -> u32 { pub fn call_node(self, db: &dyn db::ExpandDatabase) -> Option> { let macro_file = self.macro_file()?; let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id); - Some(loc.kind.to_node(db)) + Some(loc.to_node(db)) } /// If this is a macro call, returns the syntax node of the very first macro call this file resides in. pub fn original_call_node(self, db: &dyn db::ExpandDatabase) -> Option<(FileId, SyntaxNode)> { - let mut call = - db.lookup_intern_macro_call(self.macro_file()?.macro_call_id).kind.to_node(db); + let mut call = db.lookup_intern_macro_call(self.macro_file()?.macro_call_id).to_node(db); loop { match call.file_id.repr() { HirFileIdRepr::FileId(file_id) => break Some((file_id, call.value)), HirFileIdRepr::MacroFile(MacroFile { macro_call_id }) => { - call = db.lookup_intern_macro_call(macro_call_id).kind.to_node(db); + call = db.lookup_intern_macro_call(macro_call_id).to_node(db); } } } @@ -306,7 +303,7 @@ pub fn is_builtin_derive(&self, db: &dyn db::ExpandDatabase) -> Option loc.kind.to_node(db), + MacroDefKind::BuiltInDerive(..) => loc.to_node(db), _ => return None, }; Some(attr.with_value(ast::Attr::cast(attr.value.clone())?)) @@ -350,7 +347,7 @@ pub fn is_derive_attr_pseudo_expansion(&self, db: &dyn db::ExpandDatabase) -> bo match self.macro_file() { Some(macro_file) => { let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id); - matches!(loc.kind, MacroCallKind::Attr { is_derive: true, .. }) + loc.def.is_attribute_derive() } None => false, } @@ -421,22 +418,15 @@ pub fn is_attribute(&self) -> bool { MacroDefKind::BuiltInAttr(..) | MacroDefKind::ProcMacro(_, ProcMacroKind::Attr, _) ) } + + pub fn is_attribute_derive(&self) -> bool { + matches!(self.kind, MacroDefKind::BuiltInAttr(expander, ..) if expander.is_derive()) + } } -// FIXME: attribute indices do not account for nested `cfg_attr` - -impl MacroCallKind { - /// Returns the file containing the macro invocation. - fn file_id(&self) -> HirFileId { - match *self { - MacroCallKind::FnLike { ast_id: InFile { file_id, .. }, .. } - | MacroCallKind::Derive { ast_id: InFile { file_id, .. }, .. } - | MacroCallKind::Attr { ast_id: InFile { file_id, .. }, .. } => file_id, - } - } - +impl MacroCallLoc { pub fn to_node(&self, db: &dyn db::ExpandDatabase) -> InFile { - match self { + match self.kind { MacroCallKind::FnLike { ast_id, .. } => { ast_id.with_value(ast_id.to_node(db).syntax().clone()) } @@ -452,23 +442,49 @@ pub fn to_node(&self, db: &dyn db::ExpandDatabase) -> InFile { .unwrap_or_else(|| it.syntax().clone()) }) } - MacroCallKind::Attr { ast_id, is_derive: true, invoc_attr_index, .. } => { - // FIXME: handle `cfg_attr` - ast_id.with_value(ast_id.to_node(db)).map(|it| { - it.doc_comments_and_attrs() - .nth(invoc_attr_index.ast_index()) - .and_then(|it| match it { - Either::Left(attr) => Some(attr.syntax().clone()), - Either::Right(_) => None, - }) - .unwrap_or_else(|| it.syntax().clone()) - }) + MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => { + if self.def.is_attribute_derive() { + // FIXME: handle `cfg_attr` + ast_id.with_value(ast_id.to_node(db)).map(|it| { + it.doc_comments_and_attrs() + .nth(invoc_attr_index.ast_index()) + .and_then(|it| match it { + Either::Left(attr) => Some(attr.syntax().clone()), + Either::Right(_) => None, + }) + .unwrap_or_else(|| it.syntax().clone()) + }) + } else { + ast_id.with_value(ast_id.to_node(db).syntax().clone()) + } } - MacroCallKind::Attr { ast_id, .. } => { - ast_id.with_value(ast_id.to_node(db).syntax().clone()) + } + } + + fn expand_to(&self) -> ExpandTo { + match self.kind { + MacroCallKind::FnLike { expand_to, .. } => expand_to, + MacroCallKind::Derive { .. } => ExpandTo::Items, + MacroCallKind::Attr { .. } if self.def.is_attribute_derive() => ExpandTo::Statements, + MacroCallKind::Attr { .. } => { + // is this always correct? + ExpandTo::Items } } } +} + +// FIXME: attribute indices do not account for nested `cfg_attr` + +impl MacroCallKind { + /// Returns the file containing the macro invocation. + fn file_id(&self) -> HirFileId { + match *self { + MacroCallKind::FnLike { ast_id: InFile { file_id, .. }, .. } + | MacroCallKind::Derive { ast_id: InFile { file_id, .. }, .. } + | MacroCallKind::Attr { ast_id: InFile { file_id, .. }, .. } => file_id, + } + } /// Returns the original file range that best describes the location of this macro call. /// @@ -546,15 +562,6 @@ fn arg(&self, db: &dyn db::ExpandDatabase) -> Option { MacroCallKind::Attr { ast_id, .. } => Some(ast_id.to_node(db).syntax().clone()), } } - - fn expand_to(&self) -> ExpandTo { - match self { - MacroCallKind::FnLike { expand_to, .. } => *expand_to, - MacroCallKind::Derive { .. } => ExpandTo::Items, - MacroCallKind::Attr { is_derive: true, .. } => ExpandTo::Statements, - MacroCallKind::Attr { .. } => ExpandTo::Items, // is this always correct? - } - } } impl MacroCallId { @@ -618,7 +625,7 @@ pub fn map_token_down( let token_range = token.value.text_range(); match &loc.kind { - MacroCallKind::Attr { attr_args, invoc_attr_index, is_derive, .. } => { + MacroCallKind::Attr { attr_args, invoc_attr_index, .. } => { // FIXME: handle `cfg_attr` let attr = item .doc_comments_and_attrs() @@ -634,7 +641,8 @@ pub fn map_token_down( token.value.text_range().checked_sub(attr_input_start)?; // shift by the item's tree's max id let token_id = attr_args.1.token_by_range(relative_range)?; - let token_id = if *is_derive { + + let token_id = if loc.def.is_attribute_derive() { // we do not shift for `#[derive]`, as we only need to downmap the derive attribute tokens token_id } else { @@ -697,18 +705,19 @@ pub fn map_token_up( // Attributes are a bit special for us, they have two inputs, the input tokentree and the annotated item. let (token_map, tt) = match &loc.kind { - MacroCallKind::Attr { attr_args, is_derive: true, .. } => { - (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned()) - } MacroCallKind::Attr { attr_args, .. } => { - // try unshifting the token id, if unshifting fails, the token resides in the non-item attribute input - // note that the `TokenExpander::map_id_up` earlier only unshifts for declarative macros, so we don't double unshift with this - match self.macro_arg_shift.unshift(token_id) { - Some(unshifted) => { - token_id = unshifted; - (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned()) + if loc.def.is_attribute_derive() { + (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned()) + } else { + // try unshifting the token id, if unshifting fails, the token resides in the non-item attribute input + // note that the `TokenExpander::map_id_up` earlier only unshifts for declarative macros, so we don't double unshift with this + match self.macro_arg_shift.unshift(token_id) { + Some(unshifted) => { + token_id = unshifted; + (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned()) + } + None => (&self.macro_arg.1, self.arg.clone()), } - None => (&self.macro_arg.1, self.arg.clone()), } } _ => match origin {