Address review comments.

Author: nnethercote

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -817,10 +817,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
 self.has_proc_macro_decls = true;
 }
 
-ifattr::contains_name(&item.attrs, sym::no_mangle){
-ifletSome(ident) = item.kind.ident(){
-self.check_nomangle_item_asciionly(ident, item.span);
-}
+ifletSome(ident) = item.kind.ident()
+&& attr::contains_name(&item.attrs, sym::no_mangle)
+{
+self.check_nomangle_item_asciionly(ident, item.span);
 }
 
 match&item.kind{
@@ -1412,10 +1412,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
 }
 
 fnvisit_assoc_item(&mutself,item:&'aAssocItem,ctxt:AssocCtxt){
-ifattr::contains_name(&item.attrs, sym::no_mangle){
-ifletSome(ident) = item.kind.ident(){
-self.check_nomangle_item_asciionly(ident, item.span);
-}
+ifletSome(ident) = item.kind.ident()
+&& attr::contains_name(&item.attrs, sym::no_mangle)
+{
+self.check_nomangle_item_asciionly(ident, item.span);
 }
 
 if ctxt == AssocCtxt::Trait || self.outer_trait_or_trait_impl.is_none(){

compiler/rustc_builtin_macros/src/proc_macro_harness.rs

@@ -213,10 +213,8 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
 return;
 };
 
-// First up, make sure we're checking a bare function. If we're not then
-// we're just not interested in this item.
-//
-// If we find one, try to locate a `#[proc_macro_derive]` attribute on it.
+// Make sure we're checking a bare function. If we're not then we're
+// just not interested any further in this item.
 let fn_ident = iflet ast::ItemKind::Fn(fn_) = &item.kind{
  fn_.ident
 }else{
@@ -243,6 +241,7 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
 return;
 }
 
+// Try to locate a `#[proc_macro_derive]` attribute.
 if attr.has_name(sym::proc_macro_derive){
 self.collect_custom_derive(item, fn_ident, attr);
 }elseif attr.has_name(sym::proc_macro_attribute){

compiler/rustc_expand/src/expand.rs

@@ -1223,9 +1223,7 @@ impl InvocationCollectorNode for P<ast::Item> {
 
 // Work around borrow checker not seeing through `P`'s deref.
 let(span,mut attrs) = (node.span, mem::take(&mut node.attrs));
-letItemKind::Mod(_, ident, mod_kind) = &mut node.kindelse{unreachable!()};
-let ident = *ident;
-
+letItemKind::Mod(_, ident,refmut mod_kind) = node.kindelse{unreachable!()};
 let ecx = &mut collector.cx;
 let(file_path, dir_path, dir_ownership) = match mod_kind {
 ModKind::Loaded(_, inline, _, _) => {

compiler/rustc_parse/src/parser/item.rs

@@ -243,8 +243,7 @@ impl<'a> Parser<'a> {
 // STATIC ITEM
 self.bump();// `static`
 let mutability = self.parse_mutability();
-let item = self.parse_static_item(safety, mutability)?;
-ItemKind::Static(Box::new(item))
+self.parse_static_item(safety, mutability)?
 }elseifletConst::Yes(const_span) = self.parse_constness(Case::Sensitive){
 // CONST ITEM
 ifself.token.is_keyword(kw::Impl){
@@ -681,7 +680,7 @@ impl<'a> Parser<'a> {
 }
 None => (None, ty_first),// impl Type
 };
-let item_kind = ItemKind::Impl(Box::new(Impl{
+Ok(ItemKind::Impl(Box::new(Impl{
  safety,
  polarity,
  defaultness,
@@ -690,9 +689,7 @@ impl<'a> Parser<'a> {
  of_trait,
  self_ty,
 items: impl_items,
-}));
-
-Ok(item_kind)
+})))
 }
 
 fnparse_item_delegation(&mutself) -> PResult<'a,ItemKind>{
@@ -1222,13 +1219,12 @@ impl<'a> Parser<'a> {
  safety = Safety::Unsafe(self.token.span);
 let _ = self.eat_keyword(exp!(Unsafe));
 }
-let module = ast::ForeignMod{
+Ok(ItemKind::ForeignMod(ast::ForeignMod{
  extern_span,
  safety,
  abi,
 items:self.parse_item_list(attrs, |p| p.parse_foreign_item(ForceCollect::No))?,
-};
-Ok(ItemKind::ForeignMod(module))
+}))
 }
 
 /// Parses a foreign item (one in an `extern { ... }` block).
@@ -1370,7 +1366,8 @@ impl<'a> Parser<'a> {
 Ok(item_kind)
 }
 
-/// Parse a static item with the prefix `"static" "mut"?` already parsed and stored in `mutability`.
+/// Parse a static item with the prefix `"static" "mut"?` already parsed and stored in
+/// `mutability`.
 ///
 /// ```ebnf
 /// Static = "static" "mut"? $ident ":" $ty (= $expr)? ";" ;
@@ -1379,7 +1376,7 @@ impl<'a> Parser<'a> {
 &mutself,
 safety:Safety,
 mutability:Mutability,
-) -> PResult<'a,StaticItem>{
+) -> PResult<'a,ItemKind>{
 let ident = self.parse_ident()?;
 
 ifself.token == TokenKind::Lt && self.may_recover(){
@@ -1391,15 +1388,17 @@ impl<'a> Parser<'a> {
 // FIXME: This could maybe benefit from `.may_recover()`?
 let ty = match(self.eat(exp!(Colon)),self.check(exp!(Eq)) | self.check(exp!(Semi))){
 (true,false) => self.parse_ty()?,
-// If there wasn't a `:` or the colon was followed by a `=` or `;`, recover a missing type.
+// If there wasn't a `:` or the colon was followed by a `=` or `;`, recover a missing
+// type.
 (colon, _) => self.recover_missing_global_item_type(colon,Some(mutability)),
 };
 
 let expr = ifself.eat(exp!(Eq)){Some(self.parse_expr()?)}else{None};
 
 self.expect_semi()?;
 
-Ok(StaticItem{ ident, ty, safety, mutability, expr,define_opaque:None})
+let item = StaticItem{ ident, ty, safety, mutability, expr,define_opaque:None};
+Ok(ItemKind::Static(Box::new(item)))
 }
 
 /// Parse a constant item with the prefix `"const"` already parsed.
@@ -1537,7 +1536,7 @@ impl<'a> Parser<'a> {
 }
 
 let prev_span = self.prev_token.span;
-letid = self.parse_ident()?;
+letident = self.parse_ident()?;
 letmut generics = self.parse_generics()?;
  generics.where_clause = self.parse_where_clause()?;
 
@@ -1548,10 +1547,10 @@ impl<'a> Parser<'a> {
 (thin_vec![],Trailing::No)
 }else{
 self.parse_delim_comma_seq(exp!(OpenBrace),exp!(CloseBrace), |p| {
- p.parse_enum_variant(id.span)
+ p.parse_enum_variant(ident.span)
 })
 .map_err(|mut err| {
- err.span_label(id.span,"while parsing this enum");
+ err.span_label(ident.span,"while parsing this enum");
 ifself.token == token::Colon{
 let snapshot = self.create_snapshot_for_diagnostic();
 self.bump();
@@ -1577,7 +1576,7 @@ impl<'a> Parser<'a> {
 };
 
 let enum_definition = EnumDef{variants: variants.into_iter().flatten().collect()};
-Ok(ItemKind::Enum(id, enum_definition, generics))
+Ok(ItemKind::Enum(ident, enum_definition, generics))
 }
 
 fnparse_enum_variant(&mutself,span:Span) -> PResult<'a,Option<Variant>>{

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -10,8 +10,8 @@ use std::sync::Arc;
 
 use rustc_ast::visit::{self,AssocCtxt,Visitor,WalkItemKind};
 use rustc_ast::{
-selfas ast,AssocItem,AssocItemKind,Block,ConstItem,Delegation,ForeignItem,
-ForeignItemKind,Impl,Item,ItemKind,MetaItemKind,NodeId,StaticItem,StmtKind,
+selfas ast,AssocItem,AssocItemKind,Block,ConstItem,Delegation,Fn,ForeignItem,
+ForeignItemKind,Impl,Item,ItemKind,MetaItemKind,NodeId,StaticItem,StmtKind,TyAlias,
 };
 use rustc_attr_parsing as attr;
 use rustc_expand::base::ResolverExpand;
@@ -764,8 +764,8 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
 ItemKind::ExternCrate(orig_name, ident) => {
 self.build_reduced_graph_for_extern_crate(
  orig_name,
- ident,
  item,
+ ident,
  local_def_id,
  vis,
  parent,
@@ -797,7 +797,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
  | ItemKind::Static(box StaticItem{ ident, .. }) => {
 self.r.define(parent, ident,ValueNS,(res, vis, sp, expansion));
 }
-ItemKind::Fn(box ast::Fn{ ident, .. }) => {
+ItemKind::Fn(boxFn{ ident, .. }) => {
 self.r.define(parent, ident,ValueNS,(res, vis, sp, expansion));
 
 // Functions introducing procedural macros reserve a slot
@@ -806,7 +806,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
 }
 
 // These items live in the type namespace.
-ItemKind::TyAlias(box ast::TyAlias{ ident, .. }) | ItemKind::TraitAlias(ident, ..) => {
+ItemKind::TyAlias(box TyAlias{ ident, .. }) | ItemKind::TraitAlias(ident, ..) => {
 self.r.define(parent, ident,TypeNS,(res, vis, sp, expansion));
 }
 
@@ -900,8 +900,8 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
 fnbuild_reduced_graph_for_extern_crate(
 &mutself,
 orig_name:Option<Symbol>,
-ident:Ident,
 item:&Item,
+ident:Ident,
 local_def_id:LocalDefId,
 vis: ty::Visibility,
 parent:Module<'ra>,
@@ -1362,14 +1362,16 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
 }
 
 fnvisit_foreign_item(&mutself,foreign_item:&'aForeignItem){
-ifletForeignItemKind::MacCall(_) = foreign_item.kind{
-self.visit_invoc_in_module(foreign_item.id);
-return;
-}
+let ident = match foreign_item.kind{
+ForeignItemKind::Static(box StaticItem{ ident, .. })
+ | ForeignItemKind::Fn(boxFn{ ident, .. })
+ | ForeignItemKind::TyAlias(box TyAlias{ ident, .. }) => ident,
+ForeignItemKind::MacCall(_) => {
+self.visit_invoc_in_module(foreign_item.id);
+return;
+}
+};
 
-// `unwrap` is safe because `MacCall` has been excluded, and other foreign item kinds have
-// an ident.
-let ident = foreign_item.kind.ident().unwrap();
 self.build_reduced_graph_for_foreign_item(foreign_item, ident);
  visit::walk_item(self, foreign_item);
 }
@@ -1384,26 +1386,35 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
 }
 
 fnvisit_assoc_item(&mutself,item:&'aAssocItem,ctxt:AssocCtxt){
-ifletAssocItemKind::MacCall(_) = item.kind{
-match ctxt {
-AssocCtxt::Trait => {
-self.visit_invoc_in_module(item.id);
-}
-AssocCtxt::Impl{ .. } => {
-let invoc_id = item.id.placeholder_to_expn_id();
-if !self.r.glob_delegation_invoc_ids.contains(&invoc_id){
-self.r
-.impl_unexpanded_invocations
-.entry(self.r.invocation_parent(invoc_id))
-.or_default()
-.insert(invoc_id);
+let(ident, ns) = match item.kind{
+AssocItemKind::Const(box ConstItem{ ident, .. })
+ | AssocItemKind::Fn(boxFn{ ident, .. })
+ | AssocItemKind::Delegation(box Delegation{ ident, .. }) => (ident,ValueNS),
+
+AssocItemKind::Type(box TyAlias{ ident, .. }) => (ident,TypeNS),
+
+AssocItemKind::MacCall(_) => {
+match ctxt {
+AssocCtxt::Trait => {
+self.visit_invoc_in_module(item.id);
+}
+AssocCtxt::Impl{ .. } => {
+let invoc_id = item.id.placeholder_to_expn_id();
+if !self.r.glob_delegation_invoc_ids.contains(&invoc_id){
+self.r
+.impl_unexpanded_invocations
+.entry(self.r.invocation_parent(invoc_id))
+.or_default()
+.insert(invoc_id);
+}
+self.visit_invoc(item.id);
 }
-self.visit_invoc(item.id);
 }
+return;
 }
-return;
-}
 
+AssocItemKind::DelegationMac(..) => bug!(),
+};
 let vis = self.resolve_visibility(&item.vis);
 let feed = self.r.feed(item.id);
 let local_def_id = feed.key();
@@ -1418,16 +1429,6 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
 self.r.feed_visibility(feed, vis);
 }
 
-let ns = match item.kind{
-AssocItemKind::Const(..) | AssocItemKind::Delegation(..) | AssocItemKind::Fn(..) => {
-ValueNS
-}
-AssocItemKind::Type(..) => TypeNS,
-AssocItemKind::MacCall(_) | AssocItemKind::DelegationMac(..) => bug!(),// handled above
-};
-// `unwrap` is safe because `MacCall`/`DelegationMac` have been excluded, and other foreign
-// item kinds have an ident.
-let ident = item.kind.ident().unwrap();
 if ctxt == AssocCtxt::Trait{
 let parent = self.parent_scope.module;
 let expansion = self.parent_scope.expansion;

compiler/rustc_resolve/src/def_collector.rs

@@ -186,8 +186,11 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
 FnKind::Fn(
  _ctxt,
  _vis,
-Fn{sig:FnSig{ header, decl,span: _ }, generics, contract, body, .. },
+Fn{
+sig:FnSig{ header, decl,span: _ }, ident, generics, contract, body, ..
+},
 )ifletSome(coroutine_kind) = header.coroutine_kind => {
+self.visit_ident(ident);
 self.visit_fn_header(header);
 self.visit_generics(generics);
 ifletSome(contract) = contract {

compiler/rustc_resolve/src/late.rs

@@ -1025,9 +1025,10 @@ impl<'ra: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'r
 match fn_kind {
 // Bail if the function is foreign, and thus cannot validly have
 // a body, or if there's no body for some other reason.
-FnKind::Fn(FnCtxt::Foreign, _,Fn{ sig, generics, .. })
- | FnKind::Fn(_, _,Fn{ sig, generics,body:None, .. }) => {
+FnKind::Fn(FnCtxt::Foreign, _,Fn{ sig,ident,generics, .. })
+ | FnKind::Fn(_, _,Fn{ sig,ident,generics,body:None, .. }) => {
 self.visit_fn_header(&sig.header);
+self.visit_ident(ident);
 self.visit_generics(generics);
 self.with_lifetime_rib(
 LifetimeRibKind::AnonymousCreateParameter{

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -227,14 +227,10 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
  && letFnKind::Fn(_, _, ast::Fn{ sig, .. }) = fn_kind
  && letSome(items) = self.diag_metadata.current_impl_items
  && letSome(item) = items.iter().find(|i| {
-ifletSome(ident) = i.kind.ident()
- && ident.name == item_str.name
-{
+ i.kind.ident().is_some_and(|ident| {
 // Don't suggest if the item is in Fn signature arguments (#112590).
- !sig.span.contains(item_span)
-}else{
-false
-}
+ ident.name == item_str.name && !sig.span.contains(item_span)
+})
 }){
 let sp = item_span.shrink_to_lo();