From 44e21503a8f2e789bbd90b5dd52590879ba2fab6 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 20 Jul 2023 09:36:02 +0000 Subject: [PATCH 1/9] Double check that hidden types match the expected hidden type --- .../src/region_infer/opaque_types.rs | 4 +- .../src/util/compare_types.rs | 11 +- .../rustc_hir_analysis/src/check/check.rs | 143 +++++++++++++++++- .../src/solve/eval_ctxt.rs | 1 + src/tools/tidy/src/ui_tests.rs | 2 +- .../hidden_type_mismatch.rs | 57 +++++++ .../hidden_type_mismatch.stderr | 14 ++ .../nested-rpit-with-lifetimes.rs} | 4 +- 8 files changed, 224 insertions(+), 12 deletions(-) create mode 100644 tests/ui/type-alias-impl-trait/hidden_type_mismatch.rs create mode 100644 tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr rename tests/ui/{issues/issue-83190.rs => type-alias-impl-trait/nested-rpit-with-lifetimes.rs} (100%) diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index d8e81827a3be1..17a4f2d76b60f 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -339,8 +339,8 @@ fn check_opaque_type_well_formed<'tcx>( // version. let errors = ocx.select_all_or_error(); - // This is still required for many(half of the tests in ui/type-alias-impl-trait) - // tests to pass + // This is fishy, but we check it again in `check_opaque_meets_bounds`. + // Remove once we can prepopulate with known hidden types. let _ = infcx.take_opaque_types(); if errors.is_empty() { diff --git a/compiler/rustc_const_eval/src/util/compare_types.rs b/compiler/rustc_const_eval/src/util/compare_types.rs index d6a2ffb75111a..88d4f5e715c97 100644 --- a/compiler/rustc_const_eval/src/util/compare_types.rs +++ b/compiler/rustc_const_eval/src/util/compare_types.rs @@ -56,8 +56,13 @@ pub fn is_subtype<'tcx>( // With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing` // we would get unification errors because we're unable to look into opaque types, // even if they're constrained in our current function. - // - // It seems very unlikely that this hides any bugs. - let _ = infcx.take_opaque_types(); + for (key, ty) in infcx.take_opaque_types() { + span_bug!( + ty.hidden_type.span, + "{}, {}", + tcx.type_of(key.def_id).instantiate(tcx, key.args), + ty.hidden_type.ty + ); + } errors.is_empty() } diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index d9e14096954b0..c3e0526054919 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -19,11 +19,13 @@ use rustc_lint_defs::builtin::REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS; use rustc_middle::hir::nested_filter; use rustc_middle::middle::stability::EvalResult; use rustc_middle::traits::DefiningAnchor; +use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES}; use rustc_middle::ty::util::{Discr, IntTypeExt}; use rustc_middle::ty::GenericArgKind; use rustc_middle::ty::{ - self, AdtDef, ParamEnv, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, + self, AdtDef, ParamEnv, RegionKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, + TypeVisitableExt, }; use rustc_session::lint::builtin::{UNINHABITED_STATIC, UNSUPPORTED_CALLING_CONVENTIONS}; use rustc_span::symbol::sym; @@ -34,6 +36,7 @@ use rustc_trait_selection::traits::error_reporting::on_unimplemented::OnUnimplem use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _; use rustc_trait_selection::traits::{self, ObligationCtxt, TraitEngine, TraitEngineExt as _}; +use rustc_type_ir::fold::TypeFoldable; use std::ops::ControlFlow; @@ -437,7 +440,7 @@ fn check_opaque_meets_bounds<'tcx>( // hidden type is well formed even without those bounds. let predicate = ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(hidden_ty.into()))); - ocx.register_obligation(Obligation::new(tcx, misc_cause, param_env, predicate)); + ocx.register_obligation(Obligation::new(tcx, misc_cause.clone(), param_env, predicate)); // Check that all obligations are satisfied by the implementation's // version. @@ -464,11 +467,143 @@ fn check_opaque_meets_bounds<'tcx>( ocx.resolve_regions_and_report_errors(defining_use_anchor, &outlives_env)?; } } - // Clean up after ourselves - let _ = infcx.take_opaque_types(); + // Check that any hidden types found during wf checking match the hidden types that `type_of` sees. + for (key, mut ty) in infcx.take_opaque_types() { + ty.hidden_type.ty = infcx.resolve_vars_if_possible(ty.hidden_type.ty); + sanity_check_found_hidden_type(tcx, key, ty.hidden_type, defining_use_anchor, origin)?; + } Ok(()) } +fn sanity_check_found_hidden_type<'tcx>( + tcx: TyCtxt<'tcx>, + key: ty::OpaqueTypeKey<'tcx>, + mut ty: ty::OpaqueHiddenType<'tcx>, + defining_use_anchor: LocalDefId, + origin: &hir::OpaqueTyOrigin, +) -> Result<(), ErrorGuaranteed> { + if ty.ty.is_ty_var() { + // Nothing was actually constrained. + return Ok(()); + } + if let ty::Alias(ty::Opaque, alias) = ty.ty.kind() { + if alias.def_id == key.def_id.to_def_id() && alias.args == key.args { + // Nothing was actually constrained, this is an opaque usage that was + // only discovered to be opaque after inference vars resolved. + return Ok(()); + } + } + // Closures frequently end up containing erased lifetimes in their final representation. + // These correspond to lifetime variables that never got resolved, so we patch this up here. + ty.ty = ty.ty.fold_with(&mut BottomUpFolder { + tcx, + ty_op: |t| t, + ct_op: |c| c, + lt_op: |l| match l.kind() { + RegionKind::ReVar(_) => tcx.lifetimes.re_erased, + _ => l, + }, + }); + // Get the hidden type, and in case it is in a nested opaque type, find that opaque type's + // usage in the function signature and use the generic arguments from the usage site. + let mut hidden_ty = tcx.type_of(key.def_id).instantiate(tcx, key.args); + if let hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) = origin { + if hidden_ty != ty.ty { + hidden_ty = find_and_apply_rpit_substs( + tcx, + hidden_ty, + defining_use_anchor.to_def_id(), + key.def_id.to_def_id(), + )?; + } + } + // If the hidden types differ, emit a type mismatch diagnostic. + if hidden_ty == ty.ty { + Ok(()) + } else { + let span = tcx.def_span(key.def_id); + let other = ty::OpaqueHiddenType { ty: hidden_ty, span }; + Err(ty.report_mismatch(&other, key.def_id, tcx).emit()) + } +} + +fn find_and_apply_rpit_substs<'tcx>( + tcx: TyCtxt<'tcx>, + mut hidden_ty: Ty<'tcx>, + function: DefId, + opaque: DefId, +) -> Result, ErrorGuaranteed> { + // Find use of the RPIT in the function signature and thus find the right substs to + // convert it into the parameter space of the function signature. This is needed, + // because that's what `type_of` returns, against which we compare later. + let ret = tcx.fn_sig(function).instantiate_identity().output(); + struct Visitor<'tcx> { + tcx: TyCtxt<'tcx>, + opaque: DefId, + function: DefId, + seen: FxHashSet, + } + impl<'tcx> ty::TypeVisitor> for Visitor<'tcx> { + type BreakTy = GenericArgsRef<'tcx>; + + #[instrument(level = "trace", skip(self), ret)] + fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { + trace!("{:#?}", t.kind()); + match t.kind() { + ty::Alias(ty::Opaque, alias) => { + trace!(?alias.def_id); + if alias.def_id == self.opaque { + return ControlFlow::Break(alias.args); + } else if self.seen.insert(alias.def_id) { + for clause in self + .tcx + .explicit_item_bounds(alias.def_id) + .iter_instantiated_copied(self.tcx, alias.args) + { + trace!(?clause); + clause.visit_with(self)?; + } + } + } + ty::Alias(ty::Projection, alias) => { + if self.tcx.is_impl_trait_in_trait(alias.def_id) + && self.tcx.impl_trait_in_trait_parent_fn(alias.def_id) == self.function + { + // If we're lowering to associated item, install the opaque type which is just + // the `type_of` of the trait's associated item. If we're using the old lowering + // strategy, then just reinterpret the associated type like an opaque :^) + self.tcx + .type_of(alias.def_id) + .instantiate(self.tcx, alias.args) + .visit_with(self)?; + } + } + ty::Alias(ty::Weak, alias) => { + self.tcx + .type_of(alias.def_id) + .instantiate(self.tcx, alias.args) + .visit_with(self)?; + } + _ => (), + } + + t.super_visit_with(self) + } + } + if let ControlFlow::Break(args) = + ret.visit_with(&mut Visitor { tcx, function, opaque, seen: Default::default() }) + { + trace!(?args); + trace!("expected: {hidden_ty:#?}"); + hidden_ty = ty::EarlyBinder::bind(hidden_ty).instantiate(tcx, args); + trace!("expected: {hidden_ty:#?}"); + } else { + tcx.sess + .delay_span_bug(tcx.def_span(function), format!("{ret:?} does not contain {opaque:?}")); + } + Ok(hidden_ty) +} + fn is_enum_of_nonnullable_ptr<'tcx>( tcx: TyCtxt<'tcx>, adt_def: AdtDef<'tcx>, diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index 6e0aa08c307bd..0bc2cdf06532e 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs @@ -271,6 +271,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { // assertions against dropping an `InferCtxt` without taking opaques. // FIXME: Once we remove support for the old impl we can remove this. if input.anchor != DefiningAnchor::Error { + // This seems ok, but fragile. let _ = infcx.take_opaque_types(); } diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index c3a639528413b..360a82aa24fcb 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -10,7 +10,7 @@ use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. -const ISSUES_ENTRY_LIMIT: usize = 1894; +const ISSUES_ENTRY_LIMIT: usize = 1893; const ROOT_ENTRY_LIMIT: usize = 870; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ diff --git a/tests/ui/type-alias-impl-trait/hidden_type_mismatch.rs b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.rs new file mode 100644 index 0000000000000..12ce6b14e31da --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.rs @@ -0,0 +1,57 @@ +//! This test checks that we don't lose hidden types +//! for *other* opaque types that we register and use +//! to prove bounds while checking that a hidden type +//! satisfies its opaque type's bounds. + +#![feature(trivial_bounds, type_alias_impl_trait)] +#![allow(trivial_bounds)] + +mod sus { + use super::*; + pub type Sep = impl Sized + std::fmt::Display; + //~^ ERROR: concrete type differs from previous defining opaque type use + pub fn mk_sep() -> Sep { + String::from("hello") + } + + pub trait Proj { + type Assoc; + } + impl Proj for () { + type Assoc = sus::Sep; + } + + pub struct Bar { + pub inner: ::Assoc, + pub _marker: T, + } + impl Clone for Bar { + fn clone(&self) -> Self { + todo!() + } + } + impl + Copy> Copy for Bar {} + // This allows producing `Tait`s via `From`, even though + // `define_tait` is not actually callable, and thus assumed + // `Bar<()>: Copy` even though it isn't. + pub type Tait = impl Copy + From> + Into>; + pub fn define_tait() -> Tait + where + // this proves `Bar<()>: Copy`, but `define_tait` is + // now uncallable + (): Proj, + { + Bar { inner: 1i32, _marker: () } + } +} + +fn copy_tait(x: sus::Tait) -> (sus::Tait, sus::Tait) { + (x, x) +} + +fn main() { + let bar = sus::Bar { inner: sus::mk_sep(), _marker: () }; + let (y, z) = copy_tait(bar.into()); // copy a string + drop(y.into()); // drop one instance + println!("{}", z.into().inner); // print the other +} diff --git a/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr new file mode 100644 index 0000000000000..85e8a600ce37f --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr @@ -0,0 +1,14 @@ +error: concrete type differs from previous defining opaque type use + --> $DIR/hidden_type_mismatch.rs:11:20 + | +LL | pub type Sep = impl Sized + std::fmt::Display; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, got `String` + | +note: previous use here + --> $DIR/hidden_type_mismatch.rs:37:21 + | +LL | pub type Tait = impl Copy + From> + Into>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/tests/ui/issues/issue-83190.rs b/tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs similarity index 100% rename from tests/ui/issues/issue-83190.rs rename to tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs index da931c3edaf6f..11b659eec9732 100644 --- a/tests/ui/issues/issue-83190.rs +++ b/tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs @@ -1,7 +1,7 @@ -// check-pass - // Regression test for issue #83190, triggering an ICE in borrowck. +// check-pass + pub trait Any {} impl Any for T {} From c0c2d39668be6d0633dbcb7fa19bd54ed7122587 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 11 Jul 2023 15:37:09 +0000 Subject: [PATCH 2/9] Don't call a type uncallable if its signature has errors in it --- compiler/rustc_hir_typeck/src/callee.rs | 8 ++++++++ tests/ui/typeck/apit-with-error-type-in-sig.rs | 8 ++++++++ tests/ui/typeck/apit-with-error-type-in-sig.stderr | 9 +++++++++ 3 files changed, 25 insertions(+) create mode 100644 tests/ui/typeck/apit-with-error-type-in-sig.rs create mode 100644 tests/ui/typeck/apit-with-error-type-in-sig.stderr diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index a24d1ff077f38..a1fd09d4050f8 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -581,6 +581,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { callee_ty: Ty<'tcx>, arg_exprs: &'tcx [hir::Expr<'tcx>], ) -> ErrorGuaranteed { + // Callee probe fails when APIT references errors, so suppress those + // errors here. + if let Some((_, _, args)) = self.extract_callable_info(callee_ty) + && let Err(err) = args.error_reported() + { + return err; + } + let mut unit_variant = None; if let hir::ExprKind::Path(qpath) = &callee_expr.kind && let Res::Def(def::DefKind::Ctor(kind, CtorKind::Const), _) diff --git a/tests/ui/typeck/apit-with-error-type-in-sig.rs b/tests/ui/typeck/apit-with-error-type-in-sig.rs new file mode 100644 index 0000000000000..35990fc16c89a --- /dev/null +++ b/tests/ui/typeck/apit-with-error-type-in-sig.rs @@ -0,0 +1,8 @@ +type Foo = Bar; +//~^ ERROR cannot find type `Bar` in this scope + +fn check(f: impl FnOnce(Foo), val: Foo) { + f(val); +} + +fn main() {} diff --git a/tests/ui/typeck/apit-with-error-type-in-sig.stderr b/tests/ui/typeck/apit-with-error-type-in-sig.stderr new file mode 100644 index 0000000000000..49b2eac1b65e5 --- /dev/null +++ b/tests/ui/typeck/apit-with-error-type-in-sig.stderr @@ -0,0 +1,9 @@ +error[E0412]: cannot find type `Bar` in this scope + --> $DIR/apit-with-error-type-in-sig.rs:1:12 + | +LL | type Foo = Bar; + | ^^^ not found in this scope + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0412`. From 10d0ff975cead8a533a729734ad4918d943b69e7 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 21 Jul 2023 14:28:22 +0000 Subject: [PATCH 3/9] Explain what the heck is going on with this lifetime remapping business --- compiler/rustc_hir_analysis/src/check/check.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index c3e0526054919..727eeafc49135 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -504,12 +504,18 @@ fn sanity_check_found_hidden_type<'tcx>( _ => l, }, }); - // Get the hidden type, and in case it is in a nested opaque type, find that opaque type's - // usage in the function signature and use the generic arguments from the usage site. + // Get the hidden type. let mut hidden_ty = tcx.type_of(key.def_id).instantiate(tcx, key.args); + // In case it is in a nested opaque type, find that opaque type's + // usage in the function signature and use the generic arguments from the usage site. + // We need to do because RPITs ignore the lifetimes of the function, + // as they have their own copies of all the lifetimes they capture. + // So the only way to get the lifetimes represented in terms of the function, + // is to look how they are used in the function signature (or do some other fancy + // recording of this mapping at ast -> hir lowering time). if let hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) = origin { if hidden_ty != ty.ty { - hidden_ty = find_and_apply_rpit_substs( + hidden_ty = find_and_apply_rpit_args( tcx, hidden_ty, defining_use_anchor.to_def_id(), @@ -517,6 +523,7 @@ fn sanity_check_found_hidden_type<'tcx>( )?; } } + // If the hidden types differ, emit a type mismatch diagnostic. if hidden_ty == ty.ty { Ok(()) @@ -527,13 +534,13 @@ fn sanity_check_found_hidden_type<'tcx>( } } -fn find_and_apply_rpit_substs<'tcx>( +fn find_and_apply_rpit_args<'tcx>( tcx: TyCtxt<'tcx>, mut hidden_ty: Ty<'tcx>, function: DefId, opaque: DefId, ) -> Result, ErrorGuaranteed> { - // Find use of the RPIT in the function signature and thus find the right substs to + // Find use of the RPIT in the function signature and thus find the right args to // convert it into the parameter space of the function signature. This is needed, // because that's what `type_of` returns, against which we compare later. let ret = tcx.fn_sig(function).instantiate_identity().output(); From 5b4549dd13778c6546ccb9d89c32a8fe9eb3b7b7 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 24 Jul 2023 14:01:19 +0000 Subject: [PATCH 4/9] Some documentation nits --- .../rustc_hir_analysis/src/check/check.rs | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 727eeafc49135..4379d7b77b11e 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -506,13 +506,6 @@ fn sanity_check_found_hidden_type<'tcx>( }); // Get the hidden type. let mut hidden_ty = tcx.type_of(key.def_id).instantiate(tcx, key.args); - // In case it is in a nested opaque type, find that opaque type's - // usage in the function signature and use the generic arguments from the usage site. - // We need to do because RPITs ignore the lifetimes of the function, - // as they have their own copies of all the lifetimes they capture. - // So the only way to get the lifetimes represented in terms of the function, - // is to look how they are used in the function signature (or do some other fancy - // recording of this mapping at ast -> hir lowering time). if let hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) = origin { if hidden_ty != ty.ty { hidden_ty = find_and_apply_rpit_args( @@ -534,6 +527,36 @@ fn sanity_check_found_hidden_type<'tcx>( } } +/// In case it is in a nested opaque type, find that opaque type's +/// usage in the function signature and use the generic arguments from the usage site. +/// We need to do because RPITs ignore the lifetimes of the function, +/// as they have their own copies of all the lifetimes they capture. +/// So the only way to get the lifetimes represented in terms of the function, +/// is to look how they are used in the function signature (or do some other fancy +/// recording of this mapping at ast -> hir lowering time). +/// +/// As an example: +/// ```text +/// trait Id { +/// type Assoc; +/// } +/// impl<'a> Id for &'a () { +/// type Assoc = &'a (); +/// } +/// fn func<'a>(x: &'a ()) -> impl Id { x } +/// // desugared to +/// +/// // hidden type is `&'bDup ()` +/// // During wfcheck the hidden type of `Inner` is `&'a ()`, but +/// // `typeof(Inner<'b, 'bDup>) = &'bDup ()`. +/// // So we walk the signature of `func` to find the use of `Inner<'static, 'a>` +/// // and then use that to replace the lifetimes in the hidden type, obtaining +/// // `&'a ()`. +/// type Outer<'b, 'bDup> = impl Id>; +/// // hidden type is `&'cDup ()` +/// type Inner<'c, 'cDup> = impl Sized + 'cDup; +/// fn func<'a>(x: &'a () -> Outer<'static, 'a> { x } +/// ``` fn find_and_apply_rpit_args<'tcx>( tcx: TyCtxt<'tcx>, mut hidden_ty: Ty<'tcx>, From 30f787800a62077ebb211391320de4aad432b4a6 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 24 Jul 2023 15:34:36 +0000 Subject: [PATCH 5/9] Explain RPITs in the way they actually work --- .../rustc_hir_analysis/src/check/check.rs | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 4379d7b77b11e..96eb83f37682f 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -545,17 +545,23 @@ fn sanity_check_found_hidden_type<'tcx>( /// } /// fn func<'a>(x: &'a ()) -> impl Id { x } /// // desugared to +/// fn func<'a>(x: &'a () -> Outer<'a, Assoc = Inner<'a>> { +/// // Note that in contrast to other nested items, RPIT type aliases can +/// // access their parents' generics. /// -/// // hidden type is `&'bDup ()` -/// // During wfcheck the hidden type of `Inner` is `&'a ()`, but -/// // `typeof(Inner<'b, 'bDup>) = &'bDup ()`. -/// // So we walk the signature of `func` to find the use of `Inner<'static, 'a>` -/// // and then use that to replace the lifetimes in the hidden type, obtaining -/// // `&'a ()`. -/// type Outer<'b, 'bDup> = impl Id>; -/// // hidden type is `&'cDup ()` -/// type Inner<'c, 'cDup> = impl Sized + 'cDup; -/// fn func<'a>(x: &'a () -> Outer<'static, 'a> { x } +/// // hidden type is `&'aDupOuter ()` +/// // During wfcheck the hidden type of `Inner<'aDupOuter>` is `&'a ()`, but +/// // `typeof(Inner<'aDupOuter>) = &'aDupOuter ()`. +/// // So we walk the signature of `func` to find the use of `Inner<'a>` +/// // and then use that to replace the lifetimes in the hidden type, obtaining +/// // `&'a ()`. +/// type Outer<'aDupOuter> = impl Id>; +/// +/// // hidden type is `&'aDupInner ()` +/// type Inner<'aDupInner> = impl Sized + 'aDupInner; +/// +/// x +/// } /// ``` fn find_and_apply_rpit_args<'tcx>( tcx: TyCtxt<'tcx>, From 0a0ce4905d9a557f9731a5c502016c0baf7e49c8 Mon Sep 17 00:00:00 2001 From: Eric Mark Martin Date: Tue, 25 Jul 2023 00:49:49 -0400 Subject: [PATCH 6/9] factor out more stable impls --- compiler/rustc_smir/src/rustc_smir/mod.rs | 165 ++++++++++++++-------- 1 file changed, 110 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 0805f4f4d5b51..044e2f8f32569 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -575,19 +575,22 @@ impl<'tcx> Stable<'tcx> for mir::Terminator<'tcx> { impl<'tcx> Stable<'tcx> for ty::GenericArgs<'tcx> { type T = stable_mir::ty::GenericArgs; fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { - use stable_mir::ty::{GenericArgKind, GenericArgs}; - - GenericArgs( - self.iter() - .map(|arg| match arg.unpack() { - ty::GenericArgKind::Lifetime(region) => { - GenericArgKind::Lifetime(opaque(®ion)) - } - ty::GenericArgKind::Type(ty) => GenericArgKind::Type(tables.intern_ty(ty)), - ty::GenericArgKind::Const(const_) => GenericArgKind::Const(opaque(&const_)), - }) - .collect(), - ) + use stable_mir::ty::GenericArgs; + + GenericArgs(self.iter().map(|arg| arg.unpack().stable(tables)).collect()) + } +} + +impl<'tcx> Stable<'tcx> for ty::GenericArgKind<'tcx> { + type T = stable_mir::ty::GenericArgKind; + + fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { + use stable_mir::ty::GenericArgKind; + match self { + ty::GenericArgKind::Lifetime(region) => GenericArgKind::Lifetime(opaque(region)), + ty::GenericArgKind::Type(ty) => GenericArgKind::Type(tables.intern_ty(*ty)), + ty::GenericArgKind::Const(const_) => GenericArgKind::Const(opaque(&const_)), + } } } @@ -659,63 +662,118 @@ impl<'tcx> Stable<'tcx> for ty::FnSig<'tcx> { } } +impl<'tcx> Stable<'tcx> for ty::BoundTyKind { + type T = stable_mir::ty::BoundTyKind; + + fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { + use stable_mir::ty::BoundTyKind; + + match self { + ty::BoundTyKind::Anon => BoundTyKind::Anon, + ty::BoundTyKind::Param(def_id, symbol) => { + BoundTyKind::Param(rustc_internal::param_def(*def_id), symbol.to_string()) + } + } + } +} + +impl<'tcx> Stable<'tcx> for ty::BoundRegionKind { + type T = stable_mir::ty::BoundRegionKind; + + fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { + use stable_mir::ty::BoundRegionKind; + + match self { + ty::BoundRegionKind::BrAnon(option_span) => { + BoundRegionKind::BrAnon(option_span.map(|span| opaque(&span))) + } + ty::BoundRegionKind::BrNamed(def_id, symbol) => { + BoundRegionKind::BrNamed(rustc_internal::br_named_def(*def_id), symbol.to_string()) + } + ty::BoundRegionKind::BrEnv => BoundRegionKind::BrEnv, + } + } +} + impl<'tcx> Stable<'tcx> for ty::BoundVariableKind { type T = stable_mir::ty::BoundVariableKind; - fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { - use stable_mir::ty::{BoundRegionKind, BoundTyKind, BoundVariableKind}; + + fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { + use stable_mir::ty::BoundVariableKind; match self { ty::BoundVariableKind::Ty(bound_ty_kind) => { - BoundVariableKind::Ty(match bound_ty_kind { - ty::BoundTyKind::Anon => BoundTyKind::Anon, - ty::BoundTyKind::Param(def_id, symbol) => { - BoundTyKind::Param(rustc_internal::param_def(*def_id), symbol.to_string()) - } - }) + BoundVariableKind::Ty(bound_ty_kind.stable(tables)) } ty::BoundVariableKind::Region(bound_region_kind) => { - BoundVariableKind::Region(match bound_region_kind { - ty::BoundRegionKind::BrAnon(option_span) => { - BoundRegionKind::BrAnon(option_span.map(|span| opaque(&span))) - } - ty::BoundRegionKind::BrNamed(def_id, symbol) => BoundRegionKind::BrNamed( - rustc_internal::br_named_def(*def_id), - symbol.to_string(), - ), - ty::BoundRegionKind::BrEnv => BoundRegionKind::BrEnv, - }) + BoundVariableKind::Region(bound_region_kind.stable(tables)) } ty::BoundVariableKind::Const => BoundVariableKind::Const, } } } +impl<'tcx> Stable<'tcx> for ty::IntTy { + type T = IntTy; + + fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { + match self { + ty::IntTy::Isize => IntTy::Isize, + ty::IntTy::I8 => IntTy::I8, + ty::IntTy::I16 => IntTy::I16, + ty::IntTy::I32 => IntTy::I32, + ty::IntTy::I64 => IntTy::I64, + ty::IntTy::I128 => IntTy::I128, + } + } +} + +impl<'tcx> Stable<'tcx> for ty::UintTy { + type T = UintTy; + + fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { + match self { + ty::UintTy::Usize => UintTy::Usize, + ty::UintTy::U8 => UintTy::U8, + ty::UintTy::U16 => UintTy::U16, + ty::UintTy::U32 => UintTy::U32, + ty::UintTy::U64 => UintTy::U64, + ty::UintTy::U128 => UintTy::U128, + } + } +} + +impl<'tcx> Stable<'tcx> for ty::FloatTy { + type T = FloatTy; + + fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { + match self { + ty::FloatTy::F32 => FloatTy::F32, + ty::FloatTy::F64 => FloatTy::F64, + } + } +} + +impl<'tcx> Stable<'tcx> for hir::Movability { + type T = Movability; + + fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { + match self { + hir::Movability::Static => Movability::Static, + hir::Movability::Movable => Movability::Movable, + } + } +} + impl<'tcx> Stable<'tcx> for Ty<'tcx> { type T = stable_mir::ty::TyKind; fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { match self.kind() { ty::Bool => TyKind::RigidTy(RigidTy::Bool), ty::Char => TyKind::RigidTy(RigidTy::Char), - ty::Int(int_ty) => match int_ty { - ty::IntTy::Isize => TyKind::RigidTy(RigidTy::Int(IntTy::Isize)), - ty::IntTy::I8 => TyKind::RigidTy(RigidTy::Int(IntTy::I8)), - ty::IntTy::I16 => TyKind::RigidTy(RigidTy::Int(IntTy::I16)), - ty::IntTy::I32 => TyKind::RigidTy(RigidTy::Int(IntTy::I32)), - ty::IntTy::I64 => TyKind::RigidTy(RigidTy::Int(IntTy::I64)), - ty::IntTy::I128 => TyKind::RigidTy(RigidTy::Int(IntTy::I128)), - }, - ty::Uint(uint_ty) => match uint_ty { - ty::UintTy::Usize => TyKind::RigidTy(RigidTy::Uint(UintTy::Usize)), - ty::UintTy::U8 => TyKind::RigidTy(RigidTy::Uint(UintTy::U8)), - ty::UintTy::U16 => TyKind::RigidTy(RigidTy::Uint(UintTy::U16)), - ty::UintTy::U32 => TyKind::RigidTy(RigidTy::Uint(UintTy::U32)), - ty::UintTy::U64 => TyKind::RigidTy(RigidTy::Uint(UintTy::U64)), - ty::UintTy::U128 => TyKind::RigidTy(RigidTy::Uint(UintTy::U128)), - }, - ty::Float(float_ty) => match float_ty { - ty::FloatTy::F32 => TyKind::RigidTy(RigidTy::Float(FloatTy::F32)), - ty::FloatTy::F64 => TyKind::RigidTy(RigidTy::Float(FloatTy::F64)), - }, + ty::Int(int_ty) => TyKind::RigidTy(RigidTy::Int(int_ty.stable(tables))), + ty::Uint(uint_ty) => TyKind::RigidTy(RigidTy::Uint(uint_ty.stable(tables))), + ty::Float(float_ty) => TyKind::RigidTy(RigidTy::Float(float_ty.stable(tables))), ty::Adt(adt_def, generic_args) => TyKind::RigidTy(RigidTy::Adt( rustc_internal::adt_def(adt_def.did()), generic_args.stable(tables), @@ -758,10 +816,7 @@ impl<'tcx> Stable<'tcx> for Ty<'tcx> { ty::Generator(def_id, generic_args, movability) => TyKind::RigidTy(RigidTy::Generator( rustc_internal::generator_def(*def_id), generic_args.stable(tables), - match movability { - hir::Movability::Static => Movability::Static, - hir::Movability::Movable => Movability::Movable, - }, + movability.stable(tables), )), ty::Never => TyKind::RigidTy(RigidTy::Never), ty::Tuple(fields) => TyKind::RigidTy(RigidTy::Tuple( From 25db1fac81103832519bd219877e7f756e690e77 Mon Sep 17 00:00:00 2001 From: Steven Trotter Date: Fri, 14 Jul 2023 21:33:45 +0100 Subject: [PATCH 7/9] Added recursive checking back up to see if a `Clone` suggestion would be helpful. --- .../src/fn_ctxt/suggestions.rs | 84 +++++++ tests/ui/typeck/explain_clone_autoref.rs | 116 ++++++++++ tests/ui/typeck/explain_clone_autoref.stderr | 215 +++++++++++++++++- 3 files changed, 414 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index ec19d017c259b..77914a1c16cd3 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1512,9 +1512,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { found_ty: Ty<'tcx>, expr: &hir::Expr<'_>, ) { + // When `expr` is `x` in something like `let x = foo.clone(); x`, need to recurse up to get + // `foo` and `clone`. + let expr = self.note_type_is_not_clone_inner_expr(expr); + + // If we've recursed to an `expr` of `foo.clone()`, get `foo` and `clone`. let hir::ExprKind::MethodCall(segment, callee_expr, &[], _) = expr.kind else { return; }; + let Some(clone_trait_did) = self.tcx.lang_items().clone_trait() else { return; }; @@ -1567,6 +1573,84 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Given a type mismatch error caused by `&T` being cloned instead of `T`, and + /// the `expr` as the source of this type mismatch, try to find the method call + /// as the source of this error and return that instead. Otherwise, return the + /// original expression. + fn note_type_is_not_clone_inner_expr<'b>( + &'b self, + expr: &'b hir::Expr<'b>, + ) -> &'b hir::Expr<'b> { + match expr.peel_blocks().kind { + hir::ExprKind::Path(hir::QPath::Resolved( + None, + hir::Path { segments: [_], res: crate::Res::Local(binding), .. }, + )) => { + let Some(hir::Node::Pat(hir::Pat { hir_id, .. })) = self.tcx.hir().find(*binding) + else { + return expr; + }; + let Some(parent) = self.tcx.hir().find(self.tcx.hir().parent_id(*hir_id)) else { + return expr; + }; + + match parent { + // foo.clone() + hir::Node::Local(hir::Local { init: Some(init), .. }) => { + self.note_type_is_not_clone_inner_expr(init) + } + // When `expr` is more complex like a tuple + hir::Node::Pat(hir::Pat { + hir_id: pat_hir_id, + kind: hir::PatKind::Tuple(pats, ..), + .. + }) => { + let Some(hir::Node::Local(hir::Local { init: Some(init), .. })) = + self.tcx.hir().find(self.tcx.hir().parent_id(*pat_hir_id)) else { + return expr; + }; + + match init.peel_blocks().kind { + ExprKind::Tup(init_tup) => { + if let Some(init) = pats + .iter() + .enumerate() + .filter(|x| x.1.hir_id == *hir_id) + .map(|(i, _)| init_tup.get(i).unwrap()) + .next() + { + self.note_type_is_not_clone_inner_expr(init) + } else { + expr + } + } + _ => expr, + } + } + _ => expr, + } + } + // If we're calling into a closure that may not be typed recurse into that call. no need + // to worry if it's a call to a typed function or closure as this would ne handled + // previously. + hir::ExprKind::Call(Expr { kind: call_expr_kind, .. }, _) => { + if let hir::ExprKind::Path(hir::QPath::Resolved(None, call_expr_path)) = call_expr_kind + && let hir::Path { segments: [_], res: crate::Res::Local(binding), .. } = call_expr_path + && let Some(hir::Node::Pat(hir::Pat { hir_id, .. })) = self.tcx.hir().find(*binding) + && let Some(closure) = self.tcx.hir().find(self.tcx.hir().parent_id(*hir_id)) + && let hir::Node::Local(hir::Local { init: Some(init), .. }) = closure + && let Expr { kind: hir::ExprKind::Closure(hir::Closure { body: body_id, .. }), ..} = init + { + let hir::Body { value: body_expr, .. } = self.tcx.hir().body(*body_id); + self.note_type_is_not_clone_inner_expr(body_expr) + } else { + expr + } + } + _ => expr, + } + } + /// A common error is to add an extra semicolon: /// /// ```compile_fail,E0308 diff --git a/tests/ui/typeck/explain_clone_autoref.rs b/tests/ui/typeck/explain_clone_autoref.rs index 4d21574700ad9..88aaac469b21d 100644 --- a/tests/ui/typeck/explain_clone_autoref.rs +++ b/tests/ui/typeck/explain_clone_autoref.rs @@ -11,3 +11,119 @@ fn clone_thing(nc: &NotClone) -> NotClone { //~| NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead //~| NOTE expected `NotClone`, found `&NotClone` } + +fn clone_thing2(nc: &NotClone) -> NotClone { + let nc: NotClone = nc.clone(); + //~^ ERROR mismatched type + //~| NOTE expected due to this + //~| NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + //~| NOTE expected `NotClone`, found `&NotClone` + nc +} + +fn clone_thing3(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let nc = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + nc + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing4(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let nc = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let nc2 = nc; + nc2 + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +impl NotClone { + fn other_fn(&self) {} + fn get_ref_notclone(&self) -> &Self { + self + } +} + +fn clone_thing5(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let nc = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let nc2 = nc; + nc2.other_fn(); + let nc3 = nc2; + nc3 + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing6(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let (ret, _) = (nc.clone(), 1); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let _ = nc.clone(); + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing7(nc: Vec<&NotClone>) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let ret = nc[0].clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing8(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let ret = { + let a = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + a + }; + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing9(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let cl = || nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let ret = cl(); + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing10(nc: &NotClone) -> (NotClone, NotClone) { + let (a, b) = { + let a = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + (a, nc.clone()) + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + }; + (a, b) + //~^ ERROR mismatched type + //~| ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing11(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let a = { + let nothing = nc.clone(); + let a = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let nothing = nc.clone(); + a + }; + a + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} diff --git a/tests/ui/typeck/explain_clone_autoref.stderr b/tests/ui/typeck/explain_clone_autoref.stderr index 38cb7fe551841..40d3df3011901 100644 --- a/tests/ui/typeck/explain_clone_autoref.stderr +++ b/tests/ui/typeck/explain_clone_autoref.stderr @@ -18,6 +18,219 @@ LL + #[derive(Clone)] LL | struct NotClone; | -error: aborting due to previous error +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:16:24 + | +LL | let nc: NotClone = nc.clone(); + | -------- ^^^^^^^^^^ expected `NotClone`, found `&NotClone` + | | + | expected due to this + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:16:24 + | +LL | let nc: NotClone = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:28:5 + | +LL | fn clone_thing3(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | nc + | ^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:26:14 + | +LL | let nc = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:38:5 + | +LL | fn clone_thing4(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | nc2 + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:35:14 + | +LL | let nc = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:57:5 + | +LL | fn clone_thing5(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | nc3 + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:52:14 + | +LL | let nc = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:67:5 + | +LL | fn clone_thing6(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:64:21 + | +LL | let (ret, _) = (nc.clone(), 1); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:76:5 + | +LL | fn clone_thing7(nc: Vec<&NotClone>) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:74:15 + | +LL | let ret = nc[0].clone(); + | ^^^^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:88:5 + | +LL | fn clone_thing8(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:84:17 + | +LL | let a = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:98:5 + | +LL | fn clone_thing9(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:95:17 + | +LL | let cl = || nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:110:6 + | +LL | (a, b) + | ^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:105:17 + | +LL | let a = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:110:9 + | +LL | (a, b) + | ^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:107:13 + | +LL | (a, nc.clone()) + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:126:5 + | +LL | fn clone_thing11(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | a + | ^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:121:17 + | +LL | let a = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0308`. From df4bfd9e9759a9f9d5b1407db23d5881a13612ca Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 25 Jul 2023 13:40:04 +0000 Subject: [PATCH 8/9] Try explaining where `Inner` is in the signature better --- compiler/rustc_hir_analysis/src/check/check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 96eb83f37682f..58dd0c1b8abaa 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -545,7 +545,7 @@ fn sanity_check_found_hidden_type<'tcx>( /// } /// fn func<'a>(x: &'a ()) -> impl Id { x } /// // desugared to -/// fn func<'a>(x: &'a () -> Outer<'a, Assoc = Inner<'a>> { +/// fn func<'a>(x: &'a () -> Outer<'a> where as Id>::Assoc = Inner<'a> { /// // Note that in contrast to other nested items, RPIT type aliases can /// // access their parents' generics. /// From 7d46885dc192d9e0eaebe2f55e90326c9ec6216d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 25 Jul 2023 17:56:18 +0200 Subject: [PATCH 9/9] Split nested GHA groups instead of panicking --- src/tools/build_helper/src/ci.rs | 62 ++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/src/tools/build_helper/src/ci.rs b/src/tools/build_helper/src/ci.rs index b70ea5ec11390..a8505ec95967d 100644 --- a/src/tools/build_helper/src/ci.rs +++ b/src/tools/build_helper/src/ci.rs @@ -36,25 +36,26 @@ impl CiEnv { } pub mod gha { - use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Mutex; - static GROUP_ACTIVE: AtomicBool = AtomicBool::new(false); + static ACTIVE_GROUPS: Mutex> = Mutex::new(Vec::new()); /// All github actions log messages from this call to the Drop of the return value - /// will be grouped and hidden by default in logs. Note that nesting these does - /// not really work. + /// will be grouped and hidden by default in logs. Note that since github actions doesn't + /// support group nesting, any active group will be first finished when a subgroup is started, + /// and then re-started when the subgroup finishes. #[track_caller] pub fn group(name: impl std::fmt::Display) -> Group { - if std::env::var_os("GITHUB_ACTIONS").is_some() { - eprintln!("::group::{name}"); - } else { - eprintln!("{name}") + let mut groups = ACTIVE_GROUPS.lock().unwrap(); + + // A group is currently active. End it first to avoid nesting. + if !groups.is_empty() { + end_group(); } - // https://github.com/actions/toolkit/issues/1001 - assert!( - !GROUP_ACTIVE.swap(true, Ordering::Relaxed), - "nested groups are not supported by GHA!" - ); + + let name = name.to_string(); + start_group(&name); + groups.push(name); Group(()) } @@ -64,13 +65,36 @@ pub mod gha { impl Drop for Group { fn drop(&mut self) { - if std::env::var_os("GITHUB_ACTIONS").is_some() { - eprintln!("::endgroup::"); + end_group(); + + let mut groups = ACTIVE_GROUPS.lock().unwrap(); + // Remove the current group + groups.pop(); + + // If there was some previous group, restart it + if is_in_gha() { + if let Some(name) = groups.last() { + start_group(format!("{name} (continued)")); + } } - assert!( - GROUP_ACTIVE.swap(false, Ordering::Relaxed), - "group dropped but no group active!" - ); } } + + fn start_group(name: impl std::fmt::Display) { + if is_in_gha() { + eprintln!("::group::{name}"); + } else { + eprintln!("{name}") + } + } + + fn end_group() { + if is_in_gha() { + eprintln!("::endgroup::"); + } + } + + fn is_in_gha() -> bool { + std::env::var_os("GITHUB_ACTIONS").is_some() + } }