From 609b9a67c992dadf19bd7d8cdc77237d82d95152 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 9 May 2024 11:46:30 +1000 Subject: [PATCH 1/6] Correct a comment. I tried simplifying `RegionCtxt`, which led me to finding that the fields are printed in `sccs_info`. --- compiler/rustc_borrowck/src/renumber.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/renumber.rs b/compiler/rustc_borrowck/src/renumber.rs index f5757bcaa1d11..d382f264c37bd 100644 --- a/compiler/rustc_borrowck/src/renumber.rs +++ b/compiler/rustc_borrowck/src/renumber.rs @@ -26,9 +26,7 @@ pub fn renumber_mir<'tcx>( renumberer.visit_body(body); } -// FIXME(@lcnr): A lot of these variants overlap and it seems like -// this type is only used to decide which region should be used -// as representative. This should be cleaned up. +// The fields are used only for debugging output in `sccs_info`. #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] pub(crate) enum RegionCtxt { Location(Location), From d6c63bdb2188d4c90cf0e9958d7744834a132072 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 9 May 2024 11:56:38 +1000 Subject: [PATCH 2/6] Fix up `DescriptionCtx::new`. The comment mentions that `ReBound` and `ReVar` aren't expected here. Experimentation with the full test suite indicates this is true, and that `ReErased` also doesn't occur. So the commit introduces `bug!` for those cases. (If any of them show up later on, at least we'll have a test case.) The commit also remove the first sentence in the comment. `RePlaceholder` is now handled in the match arm above this comment and nothing is printed for it, so that sentence is just wrong. Furthermore, issue #13998 was closed some time ago. --- compiler/rustc_infer/src/errors/note_and_explain.rs | 10 +--------- compiler/rustc_infer/src/infer/error_reporting/mod.rs | 8 +++----- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_infer/src/errors/note_and_explain.rs b/compiler/rustc_infer/src/errors/note_and_explain.rs index 70d94873530e7..c7f07ebed973f 100644 --- a/compiler/rustc_infer/src/errors/note_and_explain.rs +++ b/compiler/rustc_infer/src/errors/note_and_explain.rs @@ -69,16 +69,8 @@ impl<'a> DescriptionCtx<'a> { ty::RePlaceholder(_) | ty::ReError(_) => return None, - // FIXME(#13998) RePlaceholder should probably print like - // ReLateParam rather than dumping Debug output on the user. - // - // We shouldn't really be having unification failures with ReVar - // and ReBound though. - // - // FIXME(@lcnr): figure out why we have to handle `ReBound` - // here, this feels somewhat off. ty::ReVar(_) | ty::ReBound(..) | ty::ReErased => { - (alt_span, "revar", format!("{region:?}")) + bug!("unexpected region for DescriptionCtx: {:?}", region); } }; Some(DescriptionCtx { span, kind, arg }) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 635bbca37ecc2..cb5be47e3046e 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -172,11 +172,9 @@ pub(super) fn note_and_explain_region<'tcx>( ty::ReError(_) => return, - // We shouldn't really be having unification failures with ReVar - // and ReBound though. - // - // FIXME(@lcnr): Figure out whether this is reachable and if so, why. - ty::ReVar(_) | ty::ReBound(..) | ty::ReErased => (format!("lifetime `{region}`"), alt_span), + ty::ReVar(_) | ty::ReBound(..) | ty::ReErased => { + bug!("unexpected region for note_and_explain_region: {:?}", region); + } }; emit_msg_span(err, prefix, description, span, suffix); From 5b5dd1b3de76161d2079eca9d3c4fe280190ca1c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 9 May 2024 15:43:39 +1000 Subject: [PATCH 3/6] Fix out-of-date comment. The type name has changed. --- compiler/rustc_infer/src/infer/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index bb53aec0b4d13..9ce3108d0c9e8 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -528,8 +528,8 @@ pub enum RegionVariableOrigin { /// Region variables created as the values for early-bound regions. /// - /// FIXME(@lcnr): This can also store a `DefId`, similar to - /// `TypeVariableOriginKind::TypeParameterDefinition`. + /// FIXME(@lcnr): This should also store a `DefId`, similar to + /// `TypeVariableOrigin`. RegionParameterDefinition(Span, Symbol), /// Region variables created when instantiating a binder with From d13612bce75ead7509a45356fe6ffcdcc14f3055 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 9 May 2024 15:52:58 +1000 Subject: [PATCH 4/6] Remove `TyCtxt::try_normalize_erasing_late_bound_regions`. It's unused. --- .../src/ty/normalize_erasing_regions.rs | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/compiler/rustc_middle/src/ty/normalize_erasing_regions.rs b/compiler/rustc_middle/src/ty/normalize_erasing_regions.rs index 791f27a97896d..115cf3eeb226c 100644 --- a/compiler/rustc_middle/src/ty/normalize_erasing_regions.rs +++ b/compiler/rustc_middle/src/ty/normalize_erasing_regions.rs @@ -100,8 +100,7 @@ impl<'tcx> TyCtxt<'tcx> { /// codegen, we need to normalize the contents. // FIXME(@lcnr): This method should not be necessary, we now normalize // inside of binders. We should be able to only use - // `tcx.instantiate_bound_regions_with_erased`. Same for the `try_X` - // variant. + // `tcx.instantiate_bound_regions_with_erased`. #[tracing::instrument(level = "debug", skip(self, param_env))] pub fn normalize_erasing_late_bound_regions( self, @@ -115,26 +114,6 @@ impl<'tcx> TyCtxt<'tcx> { self.normalize_erasing_regions(param_env, value) } - /// If you have a `Binder<'tcx, T>`, you can do this to strip out the - /// late-bound regions and then normalize the result, yielding up - /// a `T` (with regions erased). This is appropriate when the - /// binder is being instantiated at the call site. - /// - /// N.B., currently, higher-ranked type bounds inhibit - /// normalization. Therefore, each time we erase them in - /// codegen, we need to normalize the contents. - pub fn try_normalize_erasing_late_bound_regions( - self, - param_env: ty::ParamEnv<'tcx>, - value: ty::Binder<'tcx, T>, - ) -> Result> - where - T: TypeFoldable>, - { - let value = self.instantiate_bound_regions_with_erased(value); - self.try_normalize_erasing_regions(param_env, value) - } - /// Monomorphizes a type from the AST by first applying the /// in-scope instantiations and then normalizing any associated /// types. From 24445d3b6a0e89b211c564d29e12e9cca09a9580 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 9 May 2024 16:33:26 +1000 Subject: [PATCH 5/6] Remove out-of-date comment. The use of `Binder` was removed in the recent #123900, but the comment wasn't removed at the same time. --- .../src/traits/error_reporting/type_err_ctxt_ext.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index e50cb2af4b857..1f079c6f2e174 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -3443,8 +3443,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { self.dcx().try_steal_replace_and_emit_err(self.tcx.def_span(def_id), StashKey::Cycle, err) } - // FIXME(@lcnr): This function could be changed to trait `TraitRef` directly - // instead of using a `Binder`. fn report_signature_mismatch_error( &self, obligation: &PredicateObligation<'tcx>, From df6f7133ee20b545a362f0419807c4beb0ea76ff Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 9 May 2024 16:42:26 +1000 Subject: [PATCH 6/6] De-tuple two `vtable_trait_first_method_offset` args. Thus eliminating a `FIXME` comment. --- .../src/traits/select/confirmation.rs | 3 ++- compiler/rustc_trait_selection/src/traits/vtable.rs | 9 ++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 4fa2455c42de1..3d9d12bbd94da 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -693,7 +693,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let vtable_base = vtable_trait_first_method_offset( tcx, - (unnormalized_upcast_trait_ref, ty::Binder::dummy(object_trait_ref)), + unnormalized_upcast_trait_ref, + ty::Binder::dummy(object_trait_ref), ); Ok(ImplSource::Builtin(BuiltinImplSource::Object { vtable_base: vtable_base }, nested)) diff --git a/compiler/rustc_trait_selection/src/traits/vtable.rs b/compiler/rustc_trait_selection/src/traits/vtable.rs index 178f3c63ef73c..3f1ba80acd3d9 100644 --- a/compiler/rustc_trait_selection/src/traits/vtable.rs +++ b/compiler/rustc_trait_selection/src/traits/vtable.rs @@ -320,16 +320,11 @@ fn vtable_entries<'tcx>( } /// Find slot base for trait methods within vtable entries of another trait -// FIXME(@lcnr): This isn't a query, so why does it take a tuple as its argument. pub(super) fn vtable_trait_first_method_offset<'tcx>( tcx: TyCtxt<'tcx>, - key: ( - ty::PolyTraitRef<'tcx>, // trait_to_be_found - ty::PolyTraitRef<'tcx>, // trait_owning_vtable - ), + trait_to_be_found: ty::PolyTraitRef<'tcx>, + trait_owning_vtable: ty::PolyTraitRef<'tcx>, ) -> usize { - let (trait_to_be_found, trait_owning_vtable) = key; - // #90177 let trait_to_be_found_erased = tcx.erase_regions(trait_to_be_found);