fix up subst_identity vs skip_binder; add some FIXMEs as identified in review

kylematsuda · kylematsuda · commit a969c194d87f · 2023-01-26T20:28:31.000-07:00
diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs
@@ -1136,7 +1136,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             && let self_ty = infcx.replace_bound_vars_with_fresh_vars(
                                 fn_call_span,
                                 LateBoundRegionConversionTime::FnCall,
-                                tcx.fn_sig(method_did).subst_identity().input(0),
+                                // FIXME: should use `subst` with the method substs.
+                                // Probably need to add `method_substs` to `CallKind`
+                                tcx.fn_sig(method_did).skip_binder().input(0),
                             )
                             && infcx.can_eq(self.param_env, ty, self_ty).is_ok()
                         {
diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
@@ -422,8 +422,8 @@ fn extract_bad_args_for_implies_lint<'tcx>(
 
     // Map late-bound regions from trait to impl, so the names are right.
     let mapping = std::iter::zip(
-        tcx.fn_sig(trait_m.def_id).subst_identity().bound_vars(),
-        tcx.fn_sig(impl_m.def_id).subst_identity().bound_vars(),
+        tcx.fn_sig(trait_m.def_id).skip_binder().bound_vars(),
+        tcx.fn_sig(impl_m.def_id).skip_binder().bound_vars(),
     )
     .filter_map(|(impl_bv, trait_bv)| {
         if let ty::BoundVariableKind::Region(impl_bv) = impl_bv
@@ -540,7 +540,7 @@ fn compare_asyncness<'tcx>(
     trait_item_span: Option<Span>,
 ) -> Result<(), ErrorGuaranteed> {
     if tcx.asyncness(trait_m.def_id) == hir::IsAsync::Async {
-        match tcx.fn_sig(impl_m.def_id).subst_identity().skip_binder().output().kind() {
+        match tcx.fn_sig(impl_m.def_id).skip_binder().skip_binder().output().kind() {
             ty::Alias(ty::Opaque, ..) => {
                 // allow both `async fn foo()` and `fn foo() -> impl Future`
             }
diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs
@@ -867,8 +867,8 @@ fn infer_placeholder_type<'a>(
             }
 
             match ty.kind() {
-                ty::FnDef(def_id, _) => {
-                    self.tcx.mk_fn_ptr(self.tcx.fn_sig(*def_id).subst_identity())
+                ty::FnDef(def_id, substs) => {
+                    self.tcx.mk_fn_ptr(self.tcx.fn_sig(*def_id).subst(self.tcx, substs))
                 }
                 // FIXME: non-capturing closures should also suggest a function pointer
                 ty::Closure(..) | ty::Generator(..) => {
diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs
@@ -603,6 +603,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 let substs = ty::InternalSubsts::for_item(self.tcx, m.def_id, |param, _| {
                     self.var_for_def(deref.span, param)
                 });
+                let mutability =
+                    match self.tcx.fn_sig(m.def_id).skip_binder().input(0).skip_binder().kind() {
+                        ty::Ref(_, _, hir::Mutability::Mut) => "&mut ",
+                        ty::Ref(_, _, _) => "&",
+                        _ => "",
+                    };
                 vec![
                     (
                         deref.span.until(base.span),
@@ -611,18 +617,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             with_no_trimmed_paths!(
                                 self.tcx.def_path_str_with_substs(m.def_id, substs,)
                             ),
-                            match self
-                                .tcx
-                                .fn_sig(m.def_id)
-                                .subst_identity()
-                                .input(0)
-                                .skip_binder()
-                                .kind()
-                            {
-                                ty::Ref(_, _, hir::Mutability::Mut) => "&mut ",
-                                ty::Ref(_, _, _) => "&",
-                                _ => "",
-                            },
+                            mutability,
                         ),
                     ),
                     match &args[..] {
diff --git a/compiler/rustc_hir_typeck/src/method/confirm.rs b/compiler/rustc_hir_typeck/src/method/confirm.rs
@@ -503,9 +503,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
 
         debug!("method_predicates after subst = {:?}", method_predicates);
 
-        let sig = self.tcx.fn_sig(def_id);
-
-        let sig = sig.subst(self.tcx, all_substs);
+        let sig = self.tcx.fn_sig(def_id).subst(self.tcx, all_substs);
         debug!("type scheme substituted, sig={:?}", sig);
 
         let sig = self.replace_bound_vars_with_fresh_vars(sig);
diff --git a/compiler/rustc_hir_typeck/src/method/mod.rs b/compiler/rustc_hir_typeck/src/method/mod.rs
@@ -399,8 +399,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // N.B., instantiate late-bound regions before normalizing the
         // function signature so that normalization does not need to deal
         // with bound regions.
-        let fn_sig = tcx.fn_sig(def_id);
-        let fn_sig = fn_sig.subst(self.tcx, substs);
+        let fn_sig = tcx.fn_sig(def_id).subst(self.tcx, substs);
         let fn_sig =
             self.replace_bound_vars_with_fresh_vars(obligation.cause.span, infer::FnCall, fn_sig);
 
diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs
@@ -921,26 +921,22 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         expected: Ty<'tcx>,
     ) -> bool {
         match method.kind {
-            ty::AssocKind::Fn => {
-                let fty = self.tcx.fn_sig(method.def_id);
-                self.probe(|_| {
-                    let substs = self.fresh_substs_for_item(self.span, method.def_id);
-                    let fty = fty.subst(self.tcx, substs);
-                    let fty =
-                        self.replace_bound_vars_with_fresh_vars(self.span, infer::FnCall, fty);
-
-                    if let Some(self_ty) = self_ty {
-                        if self
-                            .at(&ObligationCause::dummy(), self.param_env)
-                            .sup(fty.inputs()[0], self_ty)
-                            .is_err()
-                        {
-                            return false;
-                        }
+            ty::AssocKind::Fn => self.probe(|_| {
+                let substs = self.fresh_substs_for_item(self.span, method.def_id);
+                let fty = self.tcx.fn_sig(method.def_id).subst(self.tcx, substs);
+                let fty = self.replace_bound_vars_with_fresh_vars(self.span, infer::FnCall, fty);
+
+                if let Some(self_ty) = self_ty {
+                    if self
+                        .at(&ObligationCause::dummy(), self.param_env)
+                        .sup(fty.inputs()[0], self_ty)
+                        .is_err()
+                    {
+                        return false;
                     }
-                    self.can_sub(self.param_env, fty.output(), expected).is_ok()
-                })
-            }
+                }
+                self.can_sub(self.param_env, fty.output(), expected).is_ok()
+            }),
             _ => false,
         }
     }
diff --git a/compiler/rustc_mir_transform/src/function_item_references.rs b/compiler/rustc_mir_transform/src/function_item_references.rs
@@ -161,7 +161,8 @@ impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
             .as_ref()
             .assert_crate_local()
             .lint_root;
-        let fn_sig = self.tcx.fn_sig(fn_id).skip_binder();
+        // FIXME: use existing printing routines to print the function signature
+        let fn_sig = self.tcx.fn_sig(fn_id).subst(self.tcx, fn_substs);
         let unsafety = fn_sig.unsafety().prefix_str();
         let abi = match fn_sig.abi() {
             Abi::Rust => String::from(""),
diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs
@@ -198,6 +198,7 @@ where
                 // Something like `fn() -> Priv {my_func}` is considered a private type even if
                 // `my_func` is public, so we need to visit signatures.
                 if let ty::FnDef(..) = ty.kind() {
+                    // FIXME: this should probably use `substs` from `FnDef`
                     tcx.fn_sig(def_id).subst_identity().visit_with(self)?;
                 }
                 // Inherent static methods don't have self type in substs.
diff --git a/compiler/rustc_ty_utils/src/implied_bounds.rs b/compiler/rustc_ty_utils/src/implied_bounds.rs
@@ -9,12 +9,12 @@ pub fn provide(providers: &mut ty::query::Providers) {
 fn assumed_wf_types(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::List<Ty<'_>> {
     match tcx.def_kind(def_id) {
         DefKind::Fn => {
-            let sig = tcx.fn_sig(def_id).skip_binder();
+            let sig = tcx.fn_sig(def_id).subst_identity();
             let liberated_sig = tcx.liberate_late_bound_regions(def_id, sig);
             liberated_sig.inputs_and_output
         }
         DefKind::AssocFn => {
-            let sig = tcx.fn_sig(def_id).skip_binder();
+            let sig = tcx.fn_sig(def_id).subst_identity();
             let liberated_sig = tcx.liberate_late_bound_regions(def_id, sig);
             let mut assumed_wf_types: Vec<_> =
                 tcx.assumed_wf_types(tcx.parent(def_id)).as_slice().into();

Original file line number	Diff line number	Diff line change
`@@ -1136,7 +1136,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {`
`1136`	`1136`	`&& let self_ty = infcx.replace_bound_vars_with_fresh_vars(`
`1137`	`1137`	`fn_call_span,`
`1138`	`1138`	`LateBoundRegionConversionTime::FnCall,`
`1139`		`- tcx.fn_sig(method_did).subst_identity().input(0),`
	`1139`	+ // FIXME: should use `subst` with the method substs.
	`1140`	+ // Probably need to add `method_substs` to `CallKind`
	`1141`	`+ tcx.fn_sig(method_did).skip_binder().input(0),`
`1140`	`1142`	`)`
`1141`	`1143`	`&& infcx.can_eq(self.param_env, ty, self_ty).is_ok()`
`1142`	`1144`	`{`
Original file line number	Diff line number	Diff line change
`@@ -867,8 +867,8 @@ fn infer_placeholder_type<'a>(`
`867`	`867`	`}`
`868`	`868`
`869`	`869`	`match ty.kind() {`
`870`		`- ty::FnDef(def_id, _) => {`
`871`		`- self.tcx.mk_fn_ptr(self.tcx.fn_sig(*def_id).subst_identity())`
	`870`	`+ ty::FnDef(def_id, substs) => {`
	`871`	`+ self.tcx.mk_fn_ptr(self.tcx.fn_sig(*def_id).subst(self.tcx, substs))`
`872`	`872`	`}`
`873`	`873`	`// FIXME: non-capturing closures should also suggest a function pointer`
`874`	`874`	`ty::Closure(..) \| ty::Generator(..) => {`
Original file line number	Diff line number	Diff line change
`@@ -198,6 +198,7 @@ where`
`198`	`198`	// Something like `fn() -> Priv {my_func}` is considered a private type even if
`199`	`199`	// `my_func` is public, so we need to visit signatures.
`200`	`200`	`if let ty::FnDef(..) = ty.kind() {`
	`201`	+ // FIXME: this should probably use `substs` from `FnDef`
`201`	`202`	`tcx.fn_sig(def_id).subst_identity().visit_with(self)?;`
`202`	`203`	`}`
`203`	`204`	`// Inherent static methods don't have self type in substs.`