Do not always generate first delegation argument

Author: aerooneqq

compiler/rustc_ast/src/ast.rs

@@ -3909,7 +3909,7 @@ pub struct Delegation {
     pub rename: Option<Ident>,
     pub body: Option<Box<Block>>,
     /// The item was expanded from a glob delegation item.
-    pub from_glob: bool,
+    pub from_glob_or_list: bool,
 }
 
 #[derive(Clone, Encodable, Decodable, Debug, Walkable)]

compiler/rustc_ast/src/visit.rs

@@ -428,7 +428,6 @@ macro_rules! common_visitor_and_walkers {
             ConstItem,
             ConstItemRhsKind,
             Defaultness,
-            Delegation,
             DelegationMac,
             DelegationSuffixes,
             DelimArgs,
@@ -573,6 +572,7 @@ macro_rules! common_visitor_and_walkers {
                 fn visit_fn_decl(FnDecl);
                 fn visit_fn_header(FnHeader);
                 fn visit_fn_ret_ty(FnRetTy);
+                fn visit_delegation(Delegation);
                 //fn visit_foreign_item(ForeignItem);
                 fn visit_foreign_mod(ForeignMod);
                 fn visit_format_args(FormatArgs);

compiler/rustc_ast_lowering/src/delegation.rs

@@ -50,13 +50,16 @@ use rustc_hir::attrs::{AttributeKind, InlineAttr};
 use rustc_hir::def_id::DefId;
 use rustc_hir::{self as hir, FnDeclFlags};
 use rustc_middle::span_bug;
-use rustc_middle::ty::Asyncness;
+use rustc_middle::ty::{Asyncness, TyCtxt};
 use rustc_span::symbol::kw;
 use rustc_span::{Ident, Span, Symbol};
 use smallvec::SmallVec;
 
 use crate::delegation::generics::{GenericsGenerationResult, GenericsGenerationResults};
-use crate::errors::{CycleInDelegationSignatureResolution, UnresolvedDelegationCallee};
+use crate::errors::{
+    CycleInDelegationSignatureResolution, DelegationBlockSpecifiedWhenNoParams,
+    UnresolvedDelegationCallee,
+};
 use crate::{
     AllowReturnTypeNotation, GenericArgsMode, ImplTraitContext, ImplTraitPosition, LoweringContext,
     ParamMode, ResolverAstLoweringExt,
@@ -105,6 +108,12 @@ static ATTRS_ADDITIONS: &[AttrAdditionInfo] = &[
     },
 ];
 
+// Function parameter count, including C variadic `...` if present.
+pub(crate) fn param_count(tcx: TyCtxt<'_>, def_id: DefId) -> (usize, bool /*c_variadic*/) {
+    let sig = tcx.fn_sig(def_id).skip_binder().skip_binder();
+    (sig.inputs().len() + usize::from(sig.c_variadic()), sig.c_variadic())
+}
+
 impl<'hir> LoweringContext<'_, 'hir> {
     fn is_method(&self, def_id: DefId, span: Span) -> bool {
         match self.tcx.def_kind(def_id) {
@@ -140,13 +149,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                 let is_method = self.is_method(sig_id, span);
 
-                let (param_count, c_variadic) = self.param_count(sig_id);
+                let (param_count, c_variadic) = param_count(self.tcx, sig_id);
 
                 let mut generics = self.uplift_delegation_generics(delegation, sig_id, is_method);
 
                 let body_id = self.lower_delegation_body(
                     delegation,
-                    is_method,
+                    sig_id,
                     param_count,
                     &mut generics,
                     span,
@@ -267,12 +276,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
         self.get_partial_res(node_id).and_then(|r| r.expect_full_res().opt_def_id())
     }
 
-    // Function parameter count, including C variadic `...` if present.
-    fn param_count(&self, def_id: DefId) -> (usize, bool /*c_variadic*/) {
-        let sig = self.tcx.fn_sig(def_id).skip_binder().skip_binder();
-        (sig.inputs().len() + usize::from(sig.c_variadic()), sig.c_variadic())
-    }
-
     fn lower_delegation_decl(
         &mut self,
         sig_id: DefId,
@@ -396,7 +399,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
     fn lower_delegation_body(
         &mut self,
         delegation: &Delegation,
-        is_method: bool,
+        sig_id: DefId,
         param_count: usize,
         generics: &mut GenericsGenerationResults<'hir>,
         span: Span,
@@ -406,13 +409,21 @@ impl<'hir> LoweringContext<'_, 'hir> {
         self.lower_body(|this| {
             let mut parameters: Vec<hir::Param<'_>> = Vec::with_capacity(param_count);
             let mut args: Vec<hir::Expr<'_>> = Vec::with_capacity(param_count);
+            let is_method = this.is_method(sig_id, span);
 
             for idx in 0..param_count {
                 let (param, pat_node_id) = this.generate_param(is_method, idx, span);
                 parameters.push(param);
 
+                // Should be in sync with conditions in `lower_delayed_owner::is_dead_code`.
+                let generate_block = is_method
+                    || matches!(this.tcx.def_kind(sig_id), DefKind::Fn)
+                    || !delegation.from_glob_or_list;
+
+                // Generate block only if we are method and from glob or list OR we are not from glob or list.
                 let arg = if let Some(block) = block
                     && idx == 0
+                    && generate_block
                 {
                     let mut self_resolver = SelfResolver {
                         ctxt: this,
@@ -429,15 +440,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 args.push(arg);
             }
 
-            // If we have no params in signature function but user still wrote some code in
-            // delegation body, then add this code as first arg, eventually an error will be shown,
-            // also nested delegations may need to access information about this code (#154332),
-            // so it is better to leave this code as opposed to bodies of extern functions,
-            // which are completely erased from existence.
+            // Report an error if user has explicitly specified delegation's block
+            // in a single delegation when reused function has no params.
             if param_count == 0
+                && !delegation.from_glob_or_list
                 && let Some(block) = block
             {
-                args.push(this.lower_target_expr(&block));
+                this.dcx().emit_err(DelegationBlockSpecifiedWhenNoParams { span: block.span });
             }
 
             let final_expr = this.finalize_body_lowering(delegation, args, generics, span);

compiler/rustc_ast_lowering/src/errors.rs

@@ -535,3 +535,10 @@ pub(crate) struct CycleInDelegationSignatureResolution {
     #[primary_span]
     pub span: Span,
 }
+
+#[derive(Diagnostic)]
+#[diag("delegation block is specified for function with no params")]
+pub(crate) struct DelegationBlockSpecifiedWhenNoParams {
+    #[primary_span]
+    pub span: Span,
+}

compiler/rustc_ast_lowering/src/lib.rs

@@ -555,13 +555,21 @@ pub fn lower_to_hir(tcx: TyCtxt<'_>, (): ()) -> mid_hir::Crate<'_> {
     let mut delayed_ids: FxIndexSet<LocalDefId> = Default::default();
 
     for def_id in ast_index.indices() {
-        match &ast_index[def_id] {
+        // Check if this def is inside delegation (or it is delegation itself), if so,
+        // we should lower it in delayed mode, as we need to resolve delegation in order
+        // to understand whether to generate block as first arg or not.
+        let def_inside_delegation = resolver.defs_in_delegations_blocks.contains_key(&def_id);
+        let is_delegation = matches!(
+            ast_index[def_id],
             AstOwner::Item(Item { kind: ItemKind::Delegation { .. }, .. })
-            | AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation { .. }, .. }, _) => {
-                delayed_ids.insert(def_id);
-            }
-            _ => lowerer.lower_node(def_id),
-        };
+                | AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation { .. }, .. }, _)
+        );
+
+        if def_inside_delegation || is_delegation {
+            delayed_ids.insert(def_id);
+        } else {
+            lowerer.lower_node(def_id);
+        }
     }
 
     // Don't hash unless necessary, because it's expensive.
@@ -590,10 +598,53 @@ pub fn lower_delayed_owner(tcx: TyCtxt<'_>, def_id: LocalDefId) {
         owners: Owners::Map(&mut map),
     };
 
-    lowerer.lower_node(def_id);
+    let is_dead_code = || -> bool {
+        // Try to find enclosing delegation (delegation in which block this `def_id` is placed).
+        let Some(&parent_del) = resolver.defs_in_delegations_blocks.get(&def_id) else {
+            return false;
+        };
+
+        let from_glob_or_list = match &ast_index[parent_del] {
+            AstOwner::Item(Item { kind: ItemKind::Delegation(d), .. })
+            | AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation(d), .. }, _) => {
+                d.from_glob_or_list
+            }
+            _ => unreachable!(),
+        };
 
-    for (child_def_id, owner) in map {
-        tcx.feed_delayed_owner(child_def_id, owner);
+        let is_method_or_free = if let Some(info) = resolver.delegation_infos.get(&parent_del)
+            && let Some(sig_id) = resolver
+                .partial_res_map
+                .get(&info.resolution_node)
+                .and_then(|r| r.expect_full_res().opt_def_id())
+        {
+            tcx.opt_associated_item(sig_id).is_some_and(|a| a.is_method())
+                || matches!(tcx.def_kind(sig_id), DefKind::Fn)
+        } else {
+            // If delegation is unresolved for some reason we will generate an error delegation
+            // and some errors will be certainly emitted, so no delayed bugs should happen.
+            return false;
+        };
+
+        let (param_count, _) = delegation::param_count(tcx, parent_del.to_def_id());
+
+        // Should be in sync with conditions in `lower_delegation_body`.
+        (!is_method_or_free && from_glob_or_list) || param_count == 0
+    };
+
+    if !is_dead_code() {
+        lowerer.lower_node(def_id);
+
+        for (child_def_id, owner) in map {
+            // We can encounter `NonOwner` which will result in a phantom being written
+            // to the map, however this `NonOwner` could be inserted from lowering of
+            // other owner (for example use trees), so we do not feed delayed_owner
+            // with Phantom as (1) it is Phantom by default, (2) we want to avoid
+            // consistency assert during query feeding.
+            if !matches!(owner, hir::MaybeOwner::Phantom) {
+                tcx.feed_delayed_owner(child_def_id, owner);
+            }
+        }
     }
 }
 

compiler/rustc_expand/src/expand.rs

@@ -2066,7 +2066,7 @@ fn build_single_delegations<'a, Node: InvocationCollectorNode>(
                 ident: rename.unwrap_or(ident),
                 rename,
                 body: deleg.body.clone(),
-                from_glob,
+                from_glob_or_list: true,
             })),
             tokens: None,
         }

compiler/rustc_middle/src/ty/mod.rs

@@ -247,6 +247,7 @@ pub struct ResolverAstLowering<'tcx> {
 
     // Information about delegations which is used when handling recursive delegations
     pub delegation_infos: LocalDefIdMap<DelegationInfo>,
+    pub defs_in_delegations_blocks: LocalDefIdMap<LocalDefId>,
 
     pub disambiguators: LocalDefIdMap<Steal<PerParentDisambiguatorState>>,
 }

compiler/rustc_parse/src/parser/item.rs

@@ -906,7 +906,7 @@ impl<'a> Parser<'a> {
                 ident,
                 rename,
                 body: self.parse_delegation_body()?,
-                from_glob: false,
+                from_glob_or_list: false,
             }))
         })
     }

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -1462,7 +1462,7 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
             let parent = self.parent_scope.module.expect_local();
             let expansion = self.parent_scope.expansion;
             self.r.define_local(parent, ident, ns, self.res(def_id), vis, item.span, expansion);
-        } else if !matches!(&item.kind, AssocItemKind::Delegation(deleg) if deleg.from_glob)
+        } else if !matches!(&item.kind, AssocItemKind::Delegation(deleg) if deleg.from_glob_or_list)
             && ident.name != kw::Underscore
         {
             // Don't add underscore names, they cannot be looked up anyway.

compiler/rustc_resolve/src/def_collector.rs

@@ -50,21 +50,39 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
         name: Option<Symbol>,
         def_kind: DefKind,
         span: Span,
+    ) -> TyCtxtFeed<'tcx, LocalDefId> {
+        self.create_feed(node_id, name, def_kind, span, false)
+    }
+
+    fn create_feed(
+        &mut self,
+        node_id: NodeId,
+        name: Option<Symbol>,
+        def_kind: DefKind,
+        span: Span,
+        is_owner: bool,
     ) -> TyCtxtFeed<'tcx, LocalDefId> {
         let parent_def = self.invocation_parent.parent_def;
         debug!(
             "create_def(node_id={:?}, def_kind={:?}, parent_def={:?})",
             node_id, def_kind, parent_def
         );
-        self.r.create_def(
+
+        let feed = self.r.create_def(
             parent_def,
             node_id,
             name,
             def_kind,
             self.parent_scope.expansion.to_expn_id(),
             span.with_parent(None),
-            false,
-        )
+            is_owner,
+        );
+
+        if let Some(last_delegation) = self.invocation_parent.last_delegation {
+            self.r.defs_in_delegations_blocks.insert(feed.def_id(), last_delegation);
+        }
+
+        feed
     }
 
     fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_def: LocalDefId, f: F) {
@@ -86,16 +104,8 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
         // We only get here if the owner didn't exist yet. After the owner has been created,
         // future invocations of `collect_definitions` will get the owner out of the `owners`
         // table.
-        let parent_def = self.invocation_parent.parent_def;
-        let feed = self.r.create_def(
-            parent_def,
-            owner,
-            name,
-            def_kind,
-            self.parent_scope.expansion.to_expn_id(),
-            span.with_parent(None),
-            true,
-        );
+
+        let feed = self.create_feed(owner, name, def_kind, span, true);
         let tables = PerOwnerResolverData::new(owner, feed.key());
 
         let orig_invoc_owner = mem::replace(&mut self.invocation_parent.owner, owner);
@@ -240,6 +250,22 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
         );
     }
 
+    fn visit_delegation(&mut self, node: &'a Delegation) {
+        // Do not map defs in path (i.e., consts with blocks), consider only
+        // delegation's block which is its first argument.
+        visit::walk_path(self, &node.path);
+        node.qself.as_ref().inspect(|qself| visit::walk_qself(self, qself));
+
+        let orig_last_delegation = mem::replace(
+            &mut self.invocation_parent.last_delegation,
+            Some(self.invocation_parent.parent_def),
+        );
+
+        node.body.as_ref().inspect(|block| self.brg_visit_block(block));
+
+        self.invocation_parent.last_delegation = orig_last_delegation;
+    }
+
     fn visit_block(&mut self, block: &'a Block) {
         self.brg_visit_block(block);
     }