From fc463540deaeaac68d55b17c18cc2ca0d4a2343b Mon Sep 17 00:00:00 2001 From: lcnr Date: Fri, 1 Aug 2025 10:39:25 +0200 Subject: [PATCH] more strongly dissuade use of `skip_binder` --- compiler/rustc_type_ir/src/binder.rs | 60 ++++++++++++++-------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_type_ir/src/binder.rs b/compiler/rustc_type_ir/src/binder.rs index a7b915c48455b..09ef798211f0b 100644 --- a/compiler/rustc_type_ir/src/binder.rs +++ b/compiler/rustc_type_ir/src/binder.rs @@ -15,13 +15,12 @@ use crate::lift::Lift; use crate::visit::{Flags, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor}; use crate::{self as ty, Interner}; -/// Binder is a binder for higher-ranked lifetimes or types. It is part of the +/// `Binder` is a binder for higher-ranked lifetimes or types. It is part of the /// compiler's representation for things like `for<'a> Fn(&'a isize)` -/// (which would be represented by the type `PolyTraitRef == -/// Binder`). Note that when we instantiate, -/// erase, or otherwise "discharge" these bound vars, we change the -/// type from `Binder` to just `T` (see -/// e.g., `liberate_late_bound_regions`). +/// (which would be represented by the type `PolyTraitRef == Binder`). +/// +/// See +/// for more details. /// /// `Decodable` and `Encodable` are implemented for `Binder` using the `impl_binder_encode_decode!` macro. #[derive_where(Clone; I: Interner, T: Clone)] @@ -154,22 +153,19 @@ impl> TypeSuperVisitable for Binder { } impl Binder { - /// Skips the binder and returns the "bound" value. This is a - /// risky thing to do because it's easy to get confused about - /// De Bruijn indices and the like. It is usually better to - /// discharge the binder using `no_bound_vars` or - /// `instantiate_bound_regions` or something like - /// that. `skip_binder` is only valid when you are either - /// extracting data that has nothing to do with bound vars, you - /// are doing some sort of test that does not involve bound - /// regions, or you are being very careful about your depth - /// accounting. + /// Returns the value contained inside of this `for<'a>`. Accessing generic args + /// in the returned value is generally incorrect. + /// + /// Please read + /// before using this function. It is usually better to discharge the binder using + /// `no_bound_vars` or `instantiate_bound_regions` or something like that. /// - /// Some examples where `skip_binder` is reasonable: + /// `skip_binder` is only valid when you are either extracting data that does not reference + /// any generic arguments, e.g. a `DefId`, or when you're making sure you only pass the + /// value to things which can handle escaping bound vars. /// - /// - extracting the `DefId` from a PolyTraitRef; - /// - comparing the self type of a PolyTraitRef to see if it is equal to - /// a type parameter `X`, since the type `X` does not reference any regions + /// See existing uses of `.skip_binder()` in `rustc_trait_selection::traits::select` + /// or `rustc_next_trait_solver` for examples. pub fn skip_binder(self) -> T { self.value } @@ -336,12 +332,11 @@ impl TypeVisitor for ValidateBoundVars { } } -/// Similar to [`super::Binder`] except that it tracks early bound generics, i.e. `struct Foo(T)` +/// Similar to [`Binder`] except that it tracks early bound generics, i.e. `struct Foo(T)` /// needs `T` instantiated immediately. This type primarily exists to avoid forgetting to call /// `instantiate`. /// -/// If you don't have anything to `instantiate`, you may be looking for -/// [`instantiate_identity`](EarlyBinder::instantiate_identity) or [`skip_binder`](EarlyBinder::skip_binder). +/// See for more details. #[derive_where(Clone; I: Interner, T: Clone)] #[derive_where(Copy; I: Interner, T: Copy)] #[derive_where(PartialEq; I: Interner, T: PartialEq)] @@ -404,17 +399,22 @@ impl EarlyBinder { EarlyBinder { value, _tcx: PhantomData } } - /// Skips the binder and returns the "bound" value. - /// This can be used to extract data that does not depend on generic parameters - /// (e.g., getting the `DefId` of the inner value or getting the number of - /// arguments of an `FnSig`). Otherwise, consider using - /// [`instantiate_identity`](EarlyBinder::instantiate_identity). + /// Skips the binder and returns the "bound" value. Accessing generic args + /// in the returned value is generally incorrect. + /// + /// Please read + /// before using this function. + /// + /// Only use this to extract data that does not depend on generic parameters, e.g. + /// to get the `DefId` of the inner value or the number of arguments ofan `FnSig`, + /// or while making sure to only pass the value to functions which are explicitly + /// set up to handle these uninstantiated generic parameters. /// /// To skip the binder on `x: &EarlyBinder` to obtain `&T`, leverage /// [`EarlyBinder::as_ref`](EarlyBinder::as_ref): `x.as_ref().skip_binder()`. /// - /// See also [`Binder::skip_binder`](super::Binder::skip_binder), which is - /// the analogous operation on [`super::Binder`]. + /// See also [`Binder::skip_binder`](Binder::skip_binder), which is + /// the analogous operation on [`Binder`]. pub fn skip_binder(self) -> T { self.value }