From 86a8a3beb46f7b64a5bdd393b86b2916fe082f88 Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 30 Sep 2022 17:47:39 +0100 Subject: [PATCH 01/13] make `compare_const_impl` a query and use it in `instance.rs` --- .../rustc_hir_analysis/src/check/check.rs | 8 +- .../src/check/compare_method.rs | 98 +++++++++---------- compiler/rustc_hir_analysis/src/check/mod.rs | 1 + compiler/rustc_middle/src/query/mod.rs | 6 ++ compiler/rustc_query_impl/src/keys.rs | 11 +++ compiler/rustc_ty_utils/src/instance.rs | 38 ++----- .../associated-consts/mismatched_impl_ty_1.rs | 18 ++++ .../associated-consts/mismatched_impl_ty_2.rs | 11 +++ .../associated-consts/mismatched_impl_ty_3.rs | 11 +++ 9 files changed, 118 insertions(+), 84 deletions(-) create mode 100644 src/test/ui/associated-consts/mismatched_impl_ty_1.rs create mode 100644 src/test/ui/associated-consts/mismatched_impl_ty_2.rs create mode 100644 src/test/ui/associated-consts/mismatched_impl_ty_3.rs diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 4bbe9abaf98df..5e65446e10001 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -2,7 +2,7 @@ use crate::check::intrinsicck::InlineAsmCtxt; use super::coercion::CoerceMany; use super::compare_method::check_type_bounds; -use super::compare_method::{compare_const_impl, compare_impl_method, compare_ty_impl}; +use super::compare_method::{compare_impl_method, compare_ty_impl}; use super::*; use rustc_attr as attr; use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan}; @@ -1045,13 +1045,11 @@ fn check_impl_items_against_trait<'tcx>( match impl_item_full.kind { hir::ImplItemKind::Const(..) => { // Find associated const definition. - compare_const_impl( - tcx, + let _ = tcx.compare_assoc_const_impl_item_with_trait_item(( &ty_impl_item, - impl_item.span, &ty_trait_item, impl_trait_ref, - ); + )); } hir::ImplItemKind::Fn(..) => { let opt_trait_span = tcx.hir().span_if_local(ty_trait_item.def_id); diff --git a/compiler/rustc_hir_analysis/src/check/compare_method.rs b/compiler/rustc_hir_analysis/src/check/compare_method.rs index ae98a8f6209de..c859796d388aa 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_method.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_method.rs @@ -1300,15 +1300,15 @@ fn compare_generic_param_kinds<'tcx>( Ok(()) } -pub(crate) fn compare_const_impl<'tcx>( +/// Use `tcx.compare_assoc_const_impl_item_with_trait_item` instead +pub(crate) fn raw_compare_const_impl<'tcx>( tcx: TyCtxt<'tcx>, - impl_c: &ty::AssocItem, - impl_c_span: Span, - trait_c: &ty::AssocItem, - impl_trait_ref: ty::TraitRef<'tcx>, -) { + (impl_c, trait_c, impl_trait_ref): (&ty::AssocItem, &ty::AssocItem, ty::TraitRef<'tcx>), +) -> Result<(), ErrorGuaranteed> { debug!("compare_const_impl(impl_trait_ref={:?})", impl_trait_ref); + let impl_c_span = tcx.def_span(impl_c.def_id); + tcx.infer_ctxt().enter(|infcx| { let param_env = tcx.param_env(impl_c.def_id); let ocx = ObligationCtxt::new(&infcx); @@ -1346,68 +1346,68 @@ pub(crate) fn compare_const_impl<'tcx>( debug!("compare_const_impl: trait_ty={:?}", trait_ty); - let err = infcx + let maybe_error_reported = infcx .at(&cause, param_env) .sup(trait_ty, impl_ty) - .map(|ok| ocx.register_infer_ok_obligations(ok)); + .map(|ok| ocx.register_infer_ok_obligations(ok)) + .map_err(|terr| { + debug!( + "checking associated const for compatibility: impl ty {:?}, trait ty {:?}", + impl_ty, trait_ty + ); - if let Err(terr) = err { - debug!( - "checking associated const for compatibility: impl ty {:?}, trait ty {:?}", - impl_ty, trait_ty - ); + // Locate the Span containing just the type of the offending impl + match tcx.hir().expect_impl_item(impl_c.def_id.expect_local()).kind { + ImplItemKind::Const(ref ty, _) => cause.span = ty.span, + _ => bug!("{:?} is not a impl const", impl_c), + } - // Locate the Span containing just the type of the offending impl - match tcx.hir().expect_impl_item(impl_c.def_id.expect_local()).kind { - ImplItemKind::Const(ref ty, _) => cause.span = ty.span, - _ => bug!("{:?} is not a impl const", impl_c), - } + let mut diag = struct_span_err!( + tcx.sess, + cause.span, + E0326, + "implemented const `{}` has an incompatible type for trait", + trait_c.name + ); - let mut diag = struct_span_err!( - tcx.sess, - cause.span, - E0326, - "implemented const `{}` has an incompatible type for trait", - trait_c.name - ); + let trait_c_span = trait_c.def_id.as_local().map(|trait_c_def_id| { + // Add a label to the Span containing just the type of the const + match tcx.hir().expect_trait_item(trait_c_def_id).kind { + TraitItemKind::Const(ref ty, _) => ty.span, + _ => bug!("{:?} is not a trait const", trait_c), + } + }); - let trait_c_span = trait_c.def_id.as_local().map(|trait_c_def_id| { - // Add a label to the Span containing just the type of the const - match tcx.hir().expect_trait_item(trait_c_def_id).kind { - TraitItemKind::Const(ref ty, _) => ty.span, - _ => bug!("{:?} is not a trait const", trait_c), - } + infcx.note_type_err( + &mut diag, + &cause, + trait_c_span.map(|span| (span, "type in trait".to_owned())), + Some(infer::ValuePairs::Terms(ExpectedFound { + expected: trait_ty.into(), + found: impl_ty.into(), + })), + terr, + false, + false, + ); + diag.emit() }); - infcx.note_type_err( - &mut diag, - &cause, - trait_c_span.map(|span| (span, "type in trait".to_owned())), - Some(infer::ValuePairs::Terms(ExpectedFound { - expected: trait_ty.into(), - found: impl_ty.into(), - })), - terr, - false, - false, - ); - diag.emit(); - } - // Check that all obligations are satisfied by the implementation's // version. let errors = ocx.select_all_or_error(); if !errors.is_empty() { - infcx.report_fulfillment_errors(&errors, None, false); - return; + return Err(infcx.report_fulfillment_errors(&errors, None, false)); } + // FIXME return `ErrorReported` if region obligations error? let outlives_environment = OutlivesEnvironment::new(param_env); infcx.check_region_obligations_and_report_errors( impl_c.def_id.expect_local(), &outlives_environment, ); - }); + maybe_error_reported + }) } pub(crate) fn compare_ty_impl<'tcx>( diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs index 593a9776bde30..04e8c9c22d159 100644 --- a/compiler/rustc_hir_analysis/src/check/mod.rs +++ b/compiler/rustc_hir_analysis/src/check/mod.rs @@ -251,6 +251,7 @@ pub fn provide(providers: &mut Providers) { check_mod_item_types, region_scope_tree, collect_trait_impl_trait_tys, + compare_assoc_const_impl_item_with_trait_item: compare_method::raw_compare_const_impl, ..*providers }; } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index fb867f8b46b10..3fdeb3dd8ba00 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2100,4 +2100,10 @@ rustc_queries! { query permits_zero_init(key: TyAndLayout<'tcx>) -> bool { desc { "checking to see if {:?} permits being left zeroed", key.ty } } + + query compare_assoc_const_impl_item_with_trait_item( + key: (&'tcx ty::AssocItem, &'tcx ty::AssocItem, ty::TraitRef<'tcx>) + ) -> Result<(), ErrorGuaranteed> { + desc { |tcx| "checking assoc const `{}` has the same type as trait item", tcx.def_path_str(key.0.def_id) } + } } diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs index 47762440e290b..68debcffc8570 100644 --- a/compiler/rustc_query_impl/src/keys.rs +++ b/compiler/rustc_query_impl/src/keys.rs @@ -557,3 +557,14 @@ impl<'tcx> Key for (Ty<'tcx>, ty::ValTree<'tcx>) { DUMMY_SP } } + +impl<'tcx> Key for (&'tcx ty::AssocItem, &'tcx ty::AssocItem, ty::TraitRef<'tcx>) { + #[inline(always)] + fn query_crate_is_local(&self) -> bool { + self.0.def_id.krate == LOCAL_CRATE + } + + fn default_span(&self, tcx: TyCtxt<'_>) -> Span { + tcx.def_span(self.0.def_id) + } +} diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index fa1dc90e4a24b..97e2a146d2860 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -182,40 +182,18 @@ fn resolve_associated_item<'tcx>( // a `trait` to an associated `const` definition in an `impl`, where // the definition in the `impl` has the wrong type (for which an // error has already been/will be emitted elsewhere). - // - // NB: this may be expensive, we try to skip it in all the cases where - // we know the error would've been caught (e.g. in an upstream crate). - // - // A better approach might be to just introduce a query (returning - // `Result<(), ErrorGuaranteed>`) for the check that `rustc_hir_analysis` - // performs (i.e. that the definition's type in the `impl` matches - // the declaration in the `trait`), so that we can cheaply check - // here if it failed, instead of approximating it. if leaf_def.item.kind == ty::AssocKind::Const && trait_item_id != leaf_def.item.def_id && leaf_def.item.def_id.is_local() { - let normalized_type_of = |def_id, substs| { - tcx.subst_and_normalize_erasing_regions(substs, param_env, tcx.type_of(def_id)) - }; - - let original_ty = normalized_type_of(trait_item_id, rcvr_substs); - let resolved_ty = normalized_type_of(leaf_def.item.def_id, substs); - - if original_ty != resolved_ty { - let msg = format!( - "Instance::resolve: inconsistent associated `const` type: \ - was `{}: {}` but resolved to `{}: {}`", - tcx.def_path_str_with_substs(trait_item_id, rcvr_substs), - original_ty, - tcx.def_path_str_with_substs(leaf_def.item.def_id, substs), - resolved_ty, - ); - let span = tcx.def_span(leaf_def.item.def_id); - let reported = tcx.sess.delay_span_bug(span, &msg); - - return Err(reported); - } + let impl_item = tcx.associated_item(leaf_def.item.def_id); + let trait_item = tcx.associated_item(trait_item_id); + let impl_trait_ref = tcx.impl_trait_ref(impl_item.container_id(tcx)).unwrap(); + tcx.compare_assoc_const_impl_item_with_trait_item(( + impl_item, + trait_item, + impl_trait_ref, + ))?; } Some(ty::Instance::new(leaf_def.item.def_id, substs)) diff --git a/src/test/ui/associated-consts/mismatched_impl_ty_1.rs b/src/test/ui/associated-consts/mismatched_impl_ty_1.rs new file mode 100644 index 0000000000000..4dc6c2e47a9ea --- /dev/null +++ b/src/test/ui/associated-consts/mismatched_impl_ty_1.rs @@ -0,0 +1,18 @@ +// run-pass +#![feature(generic_const_exprs)] +#![allow(incomplete_features)] + +trait MyTrait { + type ArrayType; + const SIZE: usize; + const ARRAY: Self::ArrayType; +} +impl MyTrait for () { + type ArrayType = [u8; Self::SIZE]; + const SIZE: usize = 4; + const ARRAY: [u8; Self::SIZE] = [1, 2, 3, 4]; +} + +fn main() { + let _ = <() as MyTrait>::ARRAY; +} diff --git a/src/test/ui/associated-consts/mismatched_impl_ty_2.rs b/src/test/ui/associated-consts/mismatched_impl_ty_2.rs new file mode 100644 index 0000000000000..539becfdc7c82 --- /dev/null +++ b/src/test/ui/associated-consts/mismatched_impl_ty_2.rs @@ -0,0 +1,11 @@ +// run-pass +trait Trait { + const ASSOC: fn(&'static u32); +} +impl Trait for () { + const ASSOC: for<'a> fn(&'a u32) = |_| (); +} + +fn main() { + let _ = <() as Trait>::ASSOC; +} diff --git a/src/test/ui/associated-consts/mismatched_impl_ty_3.rs b/src/test/ui/associated-consts/mismatched_impl_ty_3.rs new file mode 100644 index 0000000000000..17bcc8fe5768c --- /dev/null +++ b/src/test/ui/associated-consts/mismatched_impl_ty_3.rs @@ -0,0 +1,11 @@ +// run-pass +trait Trait { + const ASSOC: for<'a, 'b> fn(&'a u32, &'b u32); +} +impl Trait for () { + const ASSOC: for<'a> fn(&'a u32, &'a u32) = |_, _| (); +} + +fn main() { + let _ = <() as Trait>::ASSOC; +} From 831f4402aa8531544d3589158e56d5f5f5d51373 Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 30 Sep 2022 17:56:07 +0100 Subject: [PATCH 02/13] bless --- .../associated-const-impl-wrong-lifetime.stderr | 2 +- src/test/ui/nll/trait-associated-constant.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr b/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr index de1d9589e9961..742b81535dfdf 100644 --- a/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr +++ b/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr @@ -2,7 +2,7 @@ error[E0308]: const not compatible with trait --> $DIR/associated-const-impl-wrong-lifetime.rs:7:5 | LL | const NAME: &'a str = "unit"; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch + | ^^^^^^^^^^^^^^^^^^^ lifetime mismatch | = note: expected reference `&'static str` found reference `&'a str` diff --git a/src/test/ui/nll/trait-associated-constant.stderr b/src/test/ui/nll/trait-associated-constant.stderr index ae0ffd904e799..cf1c52ba7cc43 100644 --- a/src/test/ui/nll/trait-associated-constant.stderr +++ b/src/test/ui/nll/trait-associated-constant.stderr @@ -2,7 +2,7 @@ error[E0308]: const not compatible with trait --> $DIR/trait-associated-constant.rs:21:5 | LL | const AC: Option<&'c str> = None; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch + | ^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch | = note: expected enum `Option<&'b str>` found enum `Option<&'c str>` From c1a9cf42b400bab235d2d1c11d35270879d8fd60 Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 30 Sep 2022 18:53:32 +0100 Subject: [PATCH 03/13] make query take `(LocalDefId, DefId)` --- .../rustc_hir_analysis/src/check/check.rs | 5 +-- .../src/check/compare_method.rs | 39 ++++++++++--------- compiler/rustc_middle/src/query/mod.rs | 4 +- compiler/rustc_ty_utils/src/instance.rs | 10 ++--- compiler/rustc_ty_utils/src/lib.rs | 1 + 5 files changed, 28 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 5e65446e10001..c47fdaefd60e9 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -1046,9 +1046,8 @@ fn check_impl_items_against_trait<'tcx>( hir::ImplItemKind::Const(..) => { // Find associated const definition. let _ = tcx.compare_assoc_const_impl_item_with_trait_item(( - &ty_impl_item, - &ty_trait_item, - impl_trait_ref, + impl_item.id.def_id.def_id, + ty_impl_item.trait_item_def_id.unwrap(), )); } hir::ImplItemKind::Fn(..) => { diff --git a/compiler/rustc_hir_analysis/src/check/compare_method.rs b/compiler/rustc_hir_analysis/src/check/compare_method.rs index c859796d388aa..75e56cd4be78e 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_method.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_method.rs @@ -1,6 +1,6 @@ use super::potentially_plural_count; use crate::errors::LifetimesOrBoundsMismatchOnTrait; -use hir::def_id::DefId; +use hir::def_id::{DefId, LocalDefId}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed}; use rustc_hir as hir; @@ -1303,14 +1303,17 @@ fn compare_generic_param_kinds<'tcx>( /// Use `tcx.compare_assoc_const_impl_item_with_trait_item` instead pub(crate) fn raw_compare_const_impl<'tcx>( tcx: TyCtxt<'tcx>, - (impl_c, trait_c, impl_trait_ref): (&ty::AssocItem, &ty::AssocItem, ty::TraitRef<'tcx>), + (impl_const_item_def, trait_const_item_def): (LocalDefId, DefId), ) -> Result<(), ErrorGuaranteed> { + let impl_const_item = tcx.associated_item(impl_const_item_def); + let trait_const_item = tcx.associated_item(trait_const_item_def); + let impl_trait_ref = tcx.impl_trait_ref(impl_const_item.container_id(tcx)).unwrap(); debug!("compare_const_impl(impl_trait_ref={:?})", impl_trait_ref); - let impl_c_span = tcx.def_span(impl_c.def_id); + let impl_c_span = tcx.def_span(impl_const_item_def.to_def_id()); tcx.infer_ctxt().enter(|infcx| { - let param_env = tcx.param_env(impl_c.def_id); + let param_env = tcx.param_env(impl_const_item_def.to_def_id()); let ocx = ObligationCtxt::new(&infcx); // The below is for the most part highly similar to the procedure @@ -1322,18 +1325,18 @@ pub(crate) fn raw_compare_const_impl<'tcx>( // Create a parameter environment that represents the implementation's // method. - let impl_c_hir_id = tcx.hir().local_def_id_to_hir_id(impl_c.def_id.expect_local()); + let impl_c_hir_id = tcx.hir().local_def_id_to_hir_id(impl_const_item_def); // Compute placeholder form of impl and trait const tys. - let impl_ty = tcx.type_of(impl_c.def_id); - let trait_ty = tcx.bound_type_of(trait_c.def_id).subst(tcx, trait_to_impl_substs); + let impl_ty = tcx.type_of(impl_const_item_def.to_def_id()); + let trait_ty = tcx.bound_type_of(trait_const_item_def).subst(tcx, trait_to_impl_substs); let mut cause = ObligationCause::new( impl_c_span, impl_c_hir_id, ObligationCauseCode::CompareImplItemObligation { - impl_item_def_id: impl_c.def_id.expect_local(), - trait_item_def_id: trait_c.def_id, - kind: impl_c.kind, + impl_item_def_id: impl_const_item_def, + trait_item_def_id: trait_const_item_def, + kind: impl_const_item.kind, }, ); @@ -1357,9 +1360,9 @@ pub(crate) fn raw_compare_const_impl<'tcx>( ); // Locate the Span containing just the type of the offending impl - match tcx.hir().expect_impl_item(impl_c.def_id.expect_local()).kind { + match tcx.hir().expect_impl_item(impl_const_item_def).kind { ImplItemKind::Const(ref ty, _) => cause.span = ty.span, - _ => bug!("{:?} is not a impl const", impl_c), + _ => bug!("{:?} is not a impl const", impl_const_item), } let mut diag = struct_span_err!( @@ -1367,14 +1370,14 @@ pub(crate) fn raw_compare_const_impl<'tcx>( cause.span, E0326, "implemented const `{}` has an incompatible type for trait", - trait_c.name + trait_const_item.name ); - let trait_c_span = trait_c.def_id.as_local().map(|trait_c_def_id| { + let trait_c_span = trait_const_item_def.as_local().map(|trait_c_def_id| { // Add a label to the Span containing just the type of the const match tcx.hir().expect_trait_item(trait_c_def_id).kind { TraitItemKind::Const(ref ty, _) => ty.span, - _ => bug!("{:?} is not a trait const", trait_c), + _ => bug!("{:?} is not a trait const", trait_const_item), } }); @@ -1402,10 +1405,8 @@ pub(crate) fn raw_compare_const_impl<'tcx>( // FIXME return `ErrorReported` if region obligations error? let outlives_environment = OutlivesEnvironment::new(param_env); - infcx.check_region_obligations_and_report_errors( - impl_c.def_id.expect_local(), - &outlives_environment, - ); + infcx + .check_region_obligations_and_report_errors(impl_const_item_def, &outlives_environment); maybe_error_reported }) } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 3fdeb3dd8ba00..10dc441b187a3 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2102,8 +2102,8 @@ rustc_queries! { } query compare_assoc_const_impl_item_with_trait_item( - key: (&'tcx ty::AssocItem, &'tcx ty::AssocItem, ty::TraitRef<'tcx>) + key: (LocalDefId, DefId) ) -> Result<(), ErrorGuaranteed> { - desc { |tcx| "checking assoc const `{}` has the same type as trait item", tcx.def_path_str(key.0.def_id) } + desc { |tcx| "checking assoc const `{}` has the same type as trait item", tcx.def_path_str(key.0.to_def_id()) } } } diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 97e2a146d2860..ff3cdde75a418 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -184,15 +184,11 @@ fn resolve_associated_item<'tcx>( // error has already been/will be emitted elsewhere). if leaf_def.item.kind == ty::AssocKind::Const && trait_item_id != leaf_def.item.def_id - && leaf_def.item.def_id.is_local() + && let Some(leaf_def_item) = leaf_def.item.def_id.as_local() { - let impl_item = tcx.associated_item(leaf_def.item.def_id); - let trait_item = tcx.associated_item(trait_item_id); - let impl_trait_ref = tcx.impl_trait_ref(impl_item.container_id(tcx)).unwrap(); tcx.compare_assoc_const_impl_item_with_trait_item(( - impl_item, - trait_item, - impl_trait_ref, + leaf_def_item, + trait_item_id, ))?; } diff --git a/compiler/rustc_ty_utils/src/lib.rs b/compiler/rustc_ty_utils/src/lib.rs index 10c18789f7476..9fd51e75b0e4c 100644 --- a/compiler/rustc_ty_utils/src/lib.rs +++ b/compiler/rustc_ty_utils/src/lib.rs @@ -5,6 +5,7 @@ //! This API is completely unstable and subject to change. #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] +#![feature(let_chains)] #![feature(control_flow_enum)] #![feature(never_type)] #![feature(box_patterns)] From 611db1d3f397005199c9bd9ea70535da9d16bfcd Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 30 Sep 2022 22:52:02 +0100 Subject: [PATCH 04/13] remove unnecessary `Key` impl --- compiler/rustc_query_impl/src/keys.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs index 68debcffc8570..47762440e290b 100644 --- a/compiler/rustc_query_impl/src/keys.rs +++ b/compiler/rustc_query_impl/src/keys.rs @@ -557,14 +557,3 @@ impl<'tcx> Key for (Ty<'tcx>, ty::ValTree<'tcx>) { DUMMY_SP } } - -impl<'tcx> Key for (&'tcx ty::AssocItem, &'tcx ty::AssocItem, ty::TraitRef<'tcx>) { - #[inline(always)] - fn query_crate_is_local(&self) -> bool { - self.0.def_id.krate == LOCAL_CRATE - } - - fn default_span(&self, tcx: TyCtxt<'_>) -> Span { - tcx.def_span(self.0.def_id) - } -} From fa863414fe1f225f795c6167b12663031ef2a83a Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 5 Oct 2022 11:23:14 -0700 Subject: [PATCH 05/13] Add regression test for lifetimes in alloc internals autotraits Currently pretty much all of the btree_map and btree_set ones fail, as well as linked_list::DrainFilter. error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:38:5 | 38 | / require_send_sync(async { 39 | | let _v = None::>; 40 | | async {}.await; 41 | | }); | |______^ | = note: could not prove `impl Future: Send` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:56:5 | 56 | / require_send_sync(async { 57 | | let _v = None::< 58 | | alloc::collections::btree_map::DrainFilter< 59 | | '_, ... | 65 | | async {}.await; 66 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:68:5 | 68 | / require_send_sync(async { 69 | | let _v = None::>; 70 | | async {}.await; 71 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:88:5 | 88 | / require_send_sync(async { 89 | | let _v = None::>; 90 | | async {}.await; 91 | | }); | |______^ | = note: could not prove `impl Future: Send` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:93:5 | 93 | / require_send_sync(async { 94 | | let _v = None::>; 95 | | async {}.await; 96 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:98:5 | 98 | / require_send_sync(async { 99 | | let _v = None::>; 100 | | async {}.await; 101 | | }); | |______^ | = note: could not prove `impl Future: Send` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:103:5 | 103 | / require_send_sync(async { 104 | | let _v = None::>; 105 | | async {}.await; 106 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:108:5 | 108 | / require_send_sync(async { 109 | | let _v = None::>; 110 | | async {}.await; 111 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:113:5 | 113 | / require_send_sync(async { 114 | | let _v = None::>; 115 | | async {}.await; 116 | | }); | |______^ | = note: could not prove `impl Future: Send` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:118:5 | 118 | / require_send_sync(async { 119 | | let _v = None::>; 120 | | async {}.await; 121 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:123:5 | 123 | / require_send_sync(async { 124 | | let _v = None::>; 125 | | async {}.await; 126 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:128:5 | 128 | / require_send_sync(async { 129 | | let _v = None::>; 130 | | async {}.await; 131 | | }); | |______^ | = note: could not prove `impl Future: Send` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:133:5 | 133 | / require_send_sync(async { 134 | | let _v = None::>; 135 | | async {}.await; 136 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:146:5 | 146 | / require_send_sync(async { 147 | | let _v = None::>; 148 | | async {}.await; 149 | | }); | |______^ | = note: could not prove `impl Future: Send` error: implementation of `Send` is not general enough --> library/alloc/tests/autotraits.rs:151:5 | 151 | / require_send_sync(async { 152 | | let _v = None:: bool>>; 153 | | async {}.await; 154 | | }); | |______^ implementation of `Send` is not general enough | = note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:156:5 | 156 | / require_send_sync(async { 157 | | let _v = None::>; 158 | | async {}.await; 159 | | }); | |______^ | = note: could not prove `impl Future: Send` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:166:5 | 166 | / require_send_sync(async { 167 | | let _v = None::>; 168 | | async {}.await; 169 | | }); | |______^ | = note: could not prove `impl Future: Send` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:171:5 | 171 | / require_send_sync(async { 172 | | let _v = None::>; 173 | | async {}.await; 174 | | }); | |______^ | = note: could not prove `impl Future: Send` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:176:5 | 176 | / require_send_sync(async { 177 | | let _v = None::>; 178 | | async {}.await; 179 | | }); | |______^ | = note: could not prove `impl Future: Send` error: higher-ranked lifetime error --> library/alloc/tests/autotraits.rs:181:5 | 181 | / require_send_sync(async { 182 | | let _v = None::>; 183 | | async {}.await; 184 | | }); | |______^ | = note: could not prove `impl Future: Send` error: future cannot be sent between threads safely --> library/alloc/tests/autotraits.rs:243:23 | 243 | require_send_sync(async { | _______________________^ 244 | | let _v = 245 | | None:: bool>>; 246 | | async {}.await; 247 | | }); | |_____^ future created by async block is not `Send` | = help: within `impl Future`, the trait `Send` is not implemented for `NonNull>` note: future is not `Send` as this value is used across an await --> library/alloc/tests/autotraits.rs:246:17 | 244 | let _v = | -- has type `Option fn(&'a mut &'b u32) -> bool>>` which is not `Send` 245 | None:: bool>>; 246 | async {}.await; | ^^^^^^ await occurs here, with `_v` maybe used later 247 | }); | - `_v` is later dropped here note: required by a bound in `require_send_sync` --> library/alloc/tests/autotraits.rs:3:25 | 3 | fn require_send_sync(_: T) {} | ^^^^ required by this bound in `require_send_sync` error: future cannot be shared between threads safely --> library/alloc/tests/autotraits.rs:243:23 | 243 | require_send_sync(async { | _______________________^ 244 | | let _v = 245 | | None:: bool>>; 246 | | async {}.await; 247 | | }); | |_____^ future created by async block is not `Sync` | = help: within `impl Future`, the trait `Sync` is not implemented for `NonNull>` note: future is not `Sync` as this value is used across an await --> library/alloc/tests/autotraits.rs:246:17 | 244 | let _v = | -- has type `Option fn(&'a mut &'b u32) -> bool>>` which is not `Sync` 245 | None:: bool>>; 246 | async {}.await; | ^^^^^^ await occurs here, with `_v` maybe used later 247 | }); | - `_v` is later dropped here note: required by a bound in `require_send_sync` --> library/alloc/tests/autotraits.rs:3:32 | 3 | fn require_send_sync(_: T) {} | ^^^^ required by this bound in `require_send_sync` --- library/alloc/tests/autotraits.rs | 293 ++++++++++++++++++++++++++++++ library/alloc/tests/lib.rs | 4 + 2 files changed, 297 insertions(+) create mode 100644 library/alloc/tests/autotraits.rs diff --git a/library/alloc/tests/autotraits.rs b/library/alloc/tests/autotraits.rs new file mode 100644 index 0000000000000..8ff5f0abe73dd --- /dev/null +++ b/library/alloc/tests/autotraits.rs @@ -0,0 +1,293 @@ +fn require_sync(_: T) {} +fn require_send_sync(_: T) {} + +struct NotSend(*const ()); +unsafe impl Sync for NotSend {} + +#[test] +fn test_btree_map() { + // Tests of this form are prone to https://github.com/rust-lang/rust/issues/64552. + // + // In theory the async block's future would be Send if the value we hold + // across the await point is Send, and Sync if the value we hold across the + // await point is Sync. + // + // We test autotraits in this convoluted way, instead of a straightforward + // `require_send_sync::()`, because the interaction with + // generators exposes some current limitations in rustc's ability to prove a + // lifetime bound on the erased generator witness types. See the above link. + // + // A typical way this would surface in real code is: + // + // fn spawn(_: T) {} + // + // async fn f() { + // let map = BTreeMap::>::new(); + // for _ in &map { + // async {}.await; + // } + // } + // + // fn main() { + // spawn(f()); + // } + // + // where with some unintentionally overconstrained Send impls in liballoc's + // internals, the future might incorrectly not be Send even though every + // single type involved in the program is Send and Sync. + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + // Testing like this would not catch all issues that the above form catches. + require_send_sync(None::>); + + require_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::< + alloc::collections::btree_map::DrainFilter< + '_, + &u32, + &u32, + fn(&&u32, &mut &u32) -> bool, + >, + >; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); +} + +#[test] +fn test_btree_set() { + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None:: bool>>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); +} + +#[test] +fn test_binary_heap() { + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); +} + +#[test] +fn test_linked_list() { + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + // FIXME + /* + require_send_sync(async { + let _v = + None:: bool>>; + async {}.await; + }); + */ + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); +} + +#[test] +fn test_vec_deque() { + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); + + require_send_sync(async { + let _v = None::>; + async {}.await; + }); +} diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index f30ebd77e2466..ffc5ca7a5c6cc 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -2,6 +2,7 @@ #![feature(alloc_layout_extra)] #![feature(assert_matches)] #![feature(box_syntax)] +#![feature(btree_drain_filter)] #![feature(cow_is_borrowed)] #![feature(const_box)] #![feature(const_convert)] @@ -14,6 +15,8 @@ #![feature(core_intrinsics)] #![feature(drain_filter)] #![feature(exact_size_is_empty)] +#![feature(linked_list_cursors)] +#![feature(map_try_insert)] #![feature(new_uninit)] #![feature(pattern)] #![feature(trusted_len)] @@ -49,6 +52,7 @@ use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; mod arc; +mod autotraits; mod borrow; mod boxed; mod btree_set_hash; From 4fdd0d96757d1ba4bbba0edeff3c8665f324505b Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 5 Oct 2022 11:46:12 -0700 Subject: [PATCH 06/13] Fix overconstrained Send impls in btree internals --- library/alloc/src/collections/btree/node.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index f1d2d3b30d9e2..da766b67a328f 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -206,9 +206,9 @@ impl<'a, K: 'a, V: 'a, Type> Clone for NodeRef, K, V, Type> { unsafe impl Sync for NodeRef {} -unsafe impl<'a, K: Sync + 'a, V: Sync + 'a, Type> Send for NodeRef, K, V, Type> {} -unsafe impl<'a, K: Send + 'a, V: Send + 'a, Type> Send for NodeRef, K, V, Type> {} -unsafe impl<'a, K: Send + 'a, V: Send + 'a, Type> Send for NodeRef, K, V, Type> {} +unsafe impl Send for NodeRef, K, V, Type> {} +unsafe impl Send for NodeRef, K, V, Type> {} +unsafe impl Send for NodeRef, K, V, Type> {} unsafe impl Send for NodeRef {} unsafe impl Send for NodeRef {} From 8e7783bd131e7e5a326985765b7f4399c497f7bb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 5 Oct 2022 19:46:40 +0000 Subject: [PATCH 07/13] Fix opaque_hidden_inferred_bound lint ICE --- .../locales/en-US/lint.ftl | 3 +- .../src/opaque_hidden_inferred_bound.rs | 55 +++++++++++++------ src/test/ui/lint/issue-102705.rs | 22 ++++++++ 3 files changed, 61 insertions(+), 19 deletions(-) create mode 100644 src/test/ui/lint/issue-102705.rs diff --git a/compiler/rustc_error_messages/locales/en-US/lint.ftl b/compiler/rustc_error_messages/locales/en-US/lint.ftl index 0fd9b0ead167c..7e28f22c0ba8b 100644 --- a/compiler/rustc_error_messages/locales/en-US/lint.ftl +++ b/compiler/rustc_error_messages/locales/en-US/lint.ftl @@ -436,4 +436,5 @@ lint_check_name_deprecated = lint name `{$lint_name}` is deprecated and does not lint_opaque_hidden_inferred_bound = opaque type `{$ty}` does not satisfy its associated type bounds .specifically = this associated type bound is unsatisfied for `{$proj_ty}` - .suggestion = add this bound + +lint_opaque_hidden_inferred_bound_sugg = add this bound diff --git a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs index d8ce20db37ce9..4ff3b2e2c4a76 100644 --- a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs +++ b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs @@ -1,7 +1,10 @@ +use rustc_errors::DecorateLint; use rustc_hir as hir; use rustc_infer::infer::TyCtxtInferExt; -use rustc_macros::LintDiagnostic; -use rustc_middle::ty::{self, fold::BottomUpFolder, Ty, TypeFoldable}; +use rustc_macros::{LintDiagnostic, Subdiagnostic}; +use rustc_middle::ty::{ + self, fold::BottomUpFolder, print::TraitPredPrintModifiersAndPath, Ty, TypeFoldable, +}; use rustc_span::Span; use rustc_trait_selection::traits; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; @@ -117,23 +120,29 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound { )) { // If it's a trait bound and an opaque that doesn't satisfy it, // then we can emit a suggestion to add the bound. - let (suggestion, suggest_span) = + let sugg = match (proj_term.kind(), assoc_pred.kind().skip_binder()) { - (ty::Opaque(def_id, _), ty::PredicateKind::Trait(trait_pred)) => ( - format!(" + {}", trait_pred.print_modifiers_and_trait_path()), - Some(cx.tcx.def_span(def_id).shrink_to_hi()), - ), - _ => (String::new(), None), + (ty::Opaque(def_id, _), ty::PredicateKind::Trait(trait_pred)) => Some(AddBound { + suggest_span: cx.tcx.def_span(*def_id).shrink_to_hi(), + trait_ref: trait_pred.print_modifiers_and_trait_path(), + }), + _ => None, }; - cx.emit_spanned_lint( + let lint = OpaqueHiddenInferredBoundLint { + ty: cx.tcx.mk_opaque(def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id)), + proj_ty: proj_term, + assoc_pred_span, + }; + cx.struct_span_lint( OPAQUE_HIDDEN_INFERRED_BOUND, pred_span, - OpaqueHiddenInferredBoundLint { - ty: cx.tcx.mk_opaque(def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id)), - proj_ty: proj_term, - assoc_pred_span, - suggestion, - suggest_span, + lint.msg(), + |diag| { + lint.decorate_lint(diag); + if let Some(sugg) = sugg { + diag.subdiagnostic(sugg); + } + diag }, ); } @@ -150,7 +159,17 @@ struct OpaqueHiddenInferredBoundLint<'tcx> { proj_ty: Ty<'tcx>, #[label(lint::specifically)] assoc_pred_span: Span, - #[suggestion_verbose(applicability = "machine-applicable", code = "{suggestion}")] - suggest_span: Option, - suggestion: String, +} + +#[derive(Subdiagnostic)] +#[suggestion_verbose( + lint::opaque_hidden_inferred_bound_sugg, + applicability = "machine-applicable", + code = " + {trait_ref}" +)] +struct AddBound<'tcx> { + #[primary_span] + suggest_span: Span, + #[skip_arg] + trait_ref: TraitPredPrintModifiersAndPath<'tcx>, } diff --git a/src/test/ui/lint/issue-102705.rs b/src/test/ui/lint/issue-102705.rs new file mode 100644 index 0000000000000..5bcc8950adafb --- /dev/null +++ b/src/test/ui/lint/issue-102705.rs @@ -0,0 +1,22 @@ +// check-pass + +#![allow(opaque_hidden_inferred_bound)] +#![allow(dead_code)] + +trait Duh {} + +impl Duh for i32 {} + +trait Trait { + type Assoc: Duh; +} + +impl R> Trait for F { + type Assoc = R; +} + +fn foo() -> impl Trait { + || 42 +} + +fn main() {} From fe0533638c058c0c159804ce91887304ca16a640 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 5 Oct 2022 19:55:19 +0000 Subject: [PATCH 08/13] Use proper subdiagnostic --- .../src/opaque_hidden_inferred_bound.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs index 4ff3b2e2c4a76..31705624a7fef 100644 --- a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs +++ b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs @@ -1,4 +1,3 @@ -use rustc_errors::DecorateLint; use rustc_hir as hir; use rustc_infer::infer::TyCtxtInferExt; use rustc_macros::{LintDiagnostic, Subdiagnostic}; @@ -120,7 +119,7 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound { )) { // If it's a trait bound and an opaque that doesn't satisfy it, // then we can emit a suggestion to add the bound. - let sugg = + let add_bound = match (proj_term.kind(), assoc_pred.kind().skip_binder()) { (ty::Opaque(def_id, _), ty::PredicateKind::Trait(trait_pred)) => Some(AddBound { suggest_span: cx.tcx.def_span(*def_id).shrink_to_hi(), @@ -128,21 +127,14 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound { }), _ => None, }; - let lint = OpaqueHiddenInferredBoundLint { - ty: cx.tcx.mk_opaque(def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id)), - proj_ty: proj_term, - assoc_pred_span, - }; - cx.struct_span_lint( + cx.emit_spanned_lint( OPAQUE_HIDDEN_INFERRED_BOUND, pred_span, - lint.msg(), - |diag| { - lint.decorate_lint(diag); - if let Some(sugg) = sugg { - diag.subdiagnostic(sugg); - } - diag + OpaqueHiddenInferredBoundLint { + ty: cx.tcx.mk_opaque(def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id)), + proj_ty: proj_term, + assoc_pred_span, + add_bound, }, ); } @@ -159,6 +151,8 @@ struct OpaqueHiddenInferredBoundLint<'tcx> { proj_ty: Ty<'tcx>, #[label(lint::specifically)] assoc_pred_span: Span, + #[subdiagnostic] + add_bound: Option>, } #[derive(Subdiagnostic)] From eea06de0c894921deab541ad03c5ab728d4c449b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 6 Oct 2022 14:13:12 +1100 Subject: [PATCH 09/13] Fix some comments. - It's `--print`, not `--prints`. - `-Ztime` and `-Ztime-passes` print to stderr, not stdout. --- compiler/rustc_data_structures/src/profiling.rs | 8 ++++---- compiler/rustc_driver/src/lib.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index d8b26f9840b63..3e74032d8516d 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -158,10 +158,10 @@ pub struct SelfProfilerRef { // actually enabled. event_filter_mask: EventFilter, - // Print verbose generic activities to stdout + // Print verbose generic activities to stderr. print_verbose_generic_activities: bool, - // Print extra verbose generic activities to stdout + // Print extra verbose generic activities to stderr. print_extra_verbose_generic_activities: bool, } @@ -214,7 +214,7 @@ impl SelfProfilerRef { /// Start profiling a verbose generic activity. Profiling continues until the /// VerboseTimingGuard returned from this call is dropped. In addition to recording /// a measureme event, "verbose" generic activities also print a timing entry to - /// stdout if the compiler is invoked with -Ztime or -Ztime-passes. + /// stderr if the compiler is invoked with -Ztime or -Ztime-passes. pub fn verbose_generic_activity<'a>( &'a self, event_label: &'static str, @@ -228,7 +228,7 @@ impl SelfProfilerRef { /// Start profiling an extra verbose generic activity. Profiling continues until the /// VerboseTimingGuard returned from this call is dropped. In addition to recording /// a measureme event, "extra verbose" generic activities also print a timing entry to - /// stdout if the compiler is invoked with -Ztime-passes. + /// stderr if the compiler is invoked with -Ztime-passes. pub fn extra_verbose_generic_activity<'a, A>( &'a self, event_label: &'static str, diff --git a/compiler/rustc_driver/src/lib.rs b/compiler/rustc_driver/src/lib.rs index fcd49f5d01567..97657767f5618 100644 --- a/compiler/rustc_driver/src/lib.rs +++ b/compiler/rustc_driver/src/lib.rs @@ -128,8 +128,8 @@ pub struct TimePassesCallbacks { impl Callbacks for TimePassesCallbacks { fn config(&mut self, config: &mut interface::Config) { - // If a --prints=... option has been given, we don't print the "total" - // time because it will mess up the --prints output. See #64339. + // If a --print=... option has been given, we don't print the "total" + // time because it will mess up the --print output. See #64339. self.time_passes = config.opts.prints.is_empty() && config.opts.time_passes(); config.opts.trimmed_def_paths = TrimmedDefPaths::GoodPath; } From 9110d925d0b3e4842376e830f4404a28e1aa2658 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 6 Oct 2022 14:51:45 +1100 Subject: [PATCH 10/13] Remove `-Ztime` option. The compiler currently has `-Ztime` and `-Ztime-passes`. I've used `-Ztime-passes` for years but only recently learned about `-Ztime`. What's the difference? Let's look at the `-Zhelp` output: ``` -Z time=val -- measure time of rustc processes (default: no) -Z time-passes=val -- measure time of each rustc pass (default: no) ``` The `-Ztime-passes` description is clear, but the `-Ztime` one is less so. Sounds like it measures the time for the entire process? No. The real difference is that `-Ztime-passes` prints out info about passes, and `-Ztime` does the same, but only for a subset of those passes. More specifically, there is a distinction in the profiling code between a "verbose generic activity" and an "extra verbose generic activity". `-Ztime-passes` prints both kinds, while `-Ztime` only prints the first one. (It took me a close reading of the source code to determine this difference.) In practice this distinction has low value. Perhaps in the past the "extra verbose" output was more voluminous, but now that we only print stats for a pass if it exceeds 5ms or alters the RSS, `-Ztime-passes` is less spammy. Also, a lot of the "extra verbose" cases are for individual lint passes, and you need to also use `-Zno-interleave-lints` to see those anyway. Therefore, this commit removes `-Ztime` and the associated machinery. One thing to note is that the existing "extra verbose" activities all have an extra string argument, so the commit adds the ability to accept an extra argument to the "verbose" activities. --- compiler/rustc_codegen_llvm/src/back/lto.rs | 2 +- compiler/rustc_codegen_ssa/src/back/write.rs | 2 +- .../rustc_data_structures/src/profiling.rs | 24 +++++-------------- compiler/rustc_driver/src/lib.rs | 5 +++- compiler/rustc_interface/src/tests.rs | 1 - compiler/rustc_lint/src/early.rs | 2 +- compiler/rustc_lint/src/late.rs | 17 +++++++------ .../rustc_query_impl/src/on_disk_cache.rs | 2 +- compiler/rustc_session/src/options.rs | 11 --------- compiler/rustc_session/src/session.rs | 11 ++++----- src/bootstrap/bin/rustc.rs | 2 +- src/doc/rustc/src/command-line-arguments.md | 2 +- src/librustdoc/formats/renderer.rs | 4 ++-- src/test/rustdoc-ui/z-help.stdout | 1 - 14 files changed, 33 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index 2049422b79a30..cef7bf1e8034d 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -573,7 +573,7 @@ pub(crate) fn run_pass_manager( module: &mut ModuleCodegen, thin: bool, ) -> Result<(), FatalError> { - let _timer = cgcx.prof.extra_verbose_generic_activity("LLVM_lto_optimize", &*module.name); + let _timer = cgcx.prof.verbose_generic_activity_with_arg("LLVM_lto_optimize", &*module.name); let config = cgcx.config(module.kind); // Now we have one massive module inside of llmod. Time to run the diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 680b9b642d9b2..6188094bbbdd4 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1637,7 +1637,7 @@ fn start_executing_work( llvm_start_time: &mut Option>, ) { if config.time_module && llvm_start_time.is_none() { - *llvm_start_time = Some(prof.extra_verbose_generic_activity("LLVM_passes", "crate")); + *llvm_start_time = Some(prof.verbose_generic_activity("LLVM_passes")); } } } diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index 3e74032d8516d..0980d36c36e86 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -158,30 +158,21 @@ pub struct SelfProfilerRef { // actually enabled. event_filter_mask: EventFilter, - // Print verbose generic activities to stderr. + // Print verbose generic activities to stderr? print_verbose_generic_activities: bool, - - // Print extra verbose generic activities to stderr. - print_extra_verbose_generic_activities: bool, } impl SelfProfilerRef { pub fn new( profiler: Option>, print_verbose_generic_activities: bool, - print_extra_verbose_generic_activities: bool, ) -> SelfProfilerRef { // If there is no SelfProfiler then the filter mask is set to NONE, // ensuring that nothing ever tries to actually access it. let event_filter_mask = profiler.as_ref().map_or(EventFilter::empty(), |p| p.event_filter_mask); - SelfProfilerRef { - profiler, - event_filter_mask, - print_verbose_generic_activities, - print_extra_verbose_generic_activities, - } + SelfProfilerRef { profiler, event_filter_mask, print_verbose_generic_activities } } /// This shim makes sure that calls only get executed if the filter mask @@ -214,7 +205,7 @@ impl SelfProfilerRef { /// Start profiling a verbose generic activity. Profiling continues until the /// VerboseTimingGuard returned from this call is dropped. In addition to recording /// a measureme event, "verbose" generic activities also print a timing entry to - /// stderr if the compiler is invoked with -Ztime or -Ztime-passes. + /// stderr if the compiler is invoked with -Ztime-passes. pub fn verbose_generic_activity<'a>( &'a self, event_label: &'static str, @@ -225,11 +216,8 @@ impl SelfProfilerRef { VerboseTimingGuard::start(message, self.generic_activity(event_label)) } - /// Start profiling an extra verbose generic activity. Profiling continues until the - /// VerboseTimingGuard returned from this call is dropped. In addition to recording - /// a measureme event, "extra verbose" generic activities also print a timing entry to - /// stderr if the compiler is invoked with -Ztime-passes. - pub fn extra_verbose_generic_activity<'a, A>( + /// Like `verbose_generic_activity`, but with an extra arg. + pub fn verbose_generic_activity_with_arg<'a, A>( &'a self, event_label: &'static str, event_arg: A, @@ -237,7 +225,7 @@ impl SelfProfilerRef { where A: Borrow + Into, { - let message = if self.print_extra_verbose_generic_activities { + let message = if self.print_verbose_generic_activities { Some(format!("{}({})", event_label, event_arg.borrow())) } else { None diff --git a/compiler/rustc_driver/src/lib.rs b/compiler/rustc_driver/src/lib.rs index 97657767f5618..7d5604fcabcc9 100644 --- a/compiler/rustc_driver/src/lib.rs +++ b/compiler/rustc_driver/src/lib.rs @@ -127,10 +127,13 @@ pub struct TimePassesCallbacks { } impl Callbacks for TimePassesCallbacks { + // JUSTIFICATION: the session doesn't exist at this point. + #[allow(rustc::bad_opt_access)] fn config(&mut self, config: &mut interface::Config) { // If a --print=... option has been given, we don't print the "total" // time because it will mess up the --print output. See #64339. - self.time_passes = config.opts.prints.is_empty() && config.opts.time_passes(); + // + self.time_passes = config.opts.prints.is_empty() && config.opts.unstable_opts.time_passes; config.opts.trimmed_def_paths = TrimmedDefPaths::GoodPath; } } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 2cd959689e6cf..98eeaad976fe1 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -692,7 +692,6 @@ fn test_unstable_options_tracking_hash() { untracked!(span_free_formats, true); untracked!(temps_dir, Some(String::from("abc"))); untracked!(threads, 99); - untracked!(time, true); untracked!(time_llvm_passes, true); untracked!(time_passes, true); untracked!(trace_macros, true); diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index f7759bec908b4..aee870dd29d65 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -409,7 +409,7 @@ pub fn check_ast_node<'a>( if sess.opts.unstable_opts.no_interleave_lints { for (i, pass) in passes.iter_mut().enumerate() { buffered = - sess.prof.extra_verbose_generic_activity("run_lint", pass.name()).run(|| { + sess.prof.verbose_generic_activity_with_arg("run_lint", pass.name()).run(|| { early_lint_node( sess, !pre_expansion && i == 0, diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index da6f1c5eeccfd..d4e19ef6b223f 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -425,20 +425,23 @@ fn late_lint_crate<'tcx, T: LateLintPass<'tcx>>(tcx: TyCtxt<'tcx>, builtin_lints late_lint_pass_crate(tcx, builtin_lints); } else { for pass in &mut passes { - tcx.sess.prof.extra_verbose_generic_activity("run_late_lint", pass.name()).run(|| { - late_lint_pass_crate(tcx, LateLintPassObjects { lints: slice::from_mut(pass) }); - }); + tcx.sess.prof.verbose_generic_activity_with_arg("run_late_lint", pass.name()).run( + || { + late_lint_pass_crate(tcx, LateLintPassObjects { lints: slice::from_mut(pass) }); + }, + ); } let mut passes: Vec<_> = unerased_lint_store(tcx).late_module_passes.iter().map(|pass| (pass)(tcx)).collect(); for pass in &mut passes { - tcx.sess.prof.extra_verbose_generic_activity("run_late_module_lint", pass.name()).run( - || { + tcx.sess + .prof + .verbose_generic_activity_with_arg("run_late_module_lint", pass.name()) + .run(|| { late_lint_pass_crate(tcx, LateLintPassObjects { lints: slice::from_mut(pass) }); - }, - ); + }); } } } diff --git a/compiler/rustc_query_impl/src/on_disk_cache.rs b/compiler/rustc_query_impl/src/on_disk_cache.rs index 0e93f3ce1d646..e96ea682caece 100644 --- a/compiler/rustc_query_impl/src/on_disk_cache.rs +++ b/compiler/rustc_query_impl/src/on_disk_cache.rs @@ -1067,7 +1067,7 @@ pub fn encode_query_results<'a, 'tcx, CTX, Q>( let _timer = tcx .dep_context() .profiler() - .extra_verbose_generic_activity("encode_query_results_for", std::any::type_name::()); + .verbose_generic_activity_with_arg("encode_query_results_for", std::any::type_name::()); assert!(Q::query_state(tcx).all_inactive()); let cache = Q::query_cache(tcx); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 486c514a4f5c2..8d527c05122d1 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -280,14 +280,6 @@ macro_rules! options { ) } -impl Options { - // JUSTIFICATION: defn of the suggested wrapper fn - #[allow(rustc::bad_opt_access)] - pub fn time_passes(&self) -> bool { - self.unstable_opts.time_passes || self.unstable_opts.time - } -} - impl CodegenOptions { // JUSTIFICATION: defn of the suggested wrapper fn #[allow(rustc::bad_opt_access)] @@ -1596,9 +1588,6 @@ options! { #[rustc_lint_opt_deny_field_access("use `Session::threads` instead of this field")] threads: usize = (1, parse_threads, [UNTRACKED], "use a thread pool with N threads"), - #[rustc_lint_opt_deny_field_access("use `Session::time_passes` instead of this field")] - time: bool = (false, parse_bool, [UNTRACKED], - "measure time of rustc processes (default: no)"), #[rustc_lint_opt_deny_field_access("use `Session::time_llvm_passes` instead of this field")] time_llvm_passes: bool = (false, parse_bool, [UNTRACKED], "measure time of each LLVM pass (default: no)"), diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 59b544ce9eb83..5926cdc9dad9a 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -606,10 +606,6 @@ impl Session { self.parse_sess.source_map() } - pub fn time_passes(&self) -> bool { - self.opts.time_passes() - } - /// Returns `true` if internal lints should be added to the lint store - i.e. if /// `-Zunstable-options` is provided and this isn't rustdoc (internal lints can trigger errors /// to be emitted under rustdoc). @@ -927,6 +923,10 @@ impl Session { self.opts.unstable_opts.instrument_mcount } + pub fn time_passes(&self) -> bool { + self.opts.unstable_opts.time_passes + } + pub fn time_llvm_passes(&self) -> bool { self.opts.unstable_opts.time_llvm_passes } @@ -1403,8 +1403,7 @@ pub fn build_session( CguReuseTracker::new_disabled() }; - let prof = - SelfProfilerRef::new(self_profiler, sopts.time_passes(), sopts.unstable_opts.time_passes); + let prof = SelfProfilerRef::new(self_profiler, sopts.unstable_opts.time_passes); let ctfe_backtrace = Lock::new(match env::var("RUSTC_CTFE_BACKTRACE") { Ok(ref val) if val == "immediate" => CtfeBacktrace::Immediate, diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index e96f8b0d3125f..776d73b98c4f1 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -67,7 +67,7 @@ fn main() { if target == "all" || target.into_string().unwrap().split(',').any(|c| c.trim() == crate_name) { - cmd.arg("-Ztime"); + cmd.arg("-Ztime-passes"); } } } diff --git a/src/doc/rustc/src/command-line-arguments.md b/src/doc/rustc/src/command-line-arguments.md index 79cdfb82e417e..2d12cf382b160 100644 --- a/src/doc/rustc/src/command-line-arguments.md +++ b/src/doc/rustc/src/command-line-arguments.md @@ -300,7 +300,7 @@ _Note:_ The order of these lint level arguments is taken into account, see [lint ## `-Z`: set unstable options This flag will allow you to set unstable options of rustc. In order to set multiple options, -the -Z flag can be used multiple times. For example: `rustc -Z verbose -Z time`. +the -Z flag can be used multiple times. For example: `rustc -Z verbose -Z time-passes`. Specifying options with -Z is only available on nightly. To view all available options run: `rustc -Z help`. diff --git a/src/librustdoc/formats/renderer.rs b/src/librustdoc/formats/renderer.rs index 62ba984acc961..6f9cc026675b6 100644 --- a/src/librustdoc/formats/renderer.rs +++ b/src/librustdoc/formats/renderer.rs @@ -58,7 +58,7 @@ pub(crate) fn run_format<'tcx, T: FormatRenderer<'tcx>>( let emit_crate = options.should_emit_crate(); let (mut format_renderer, krate) = prof - .extra_verbose_generic_activity("create_renderer", T::descr()) + .verbose_generic_activity_with_arg("create_renderer", T::descr()) .run(|| T::init(krate, options, cache, tcx))?; if !emit_crate { @@ -92,6 +92,6 @@ pub(crate) fn run_format<'tcx, T: FormatRenderer<'tcx>>( .run(|| cx.item(item))?; } } - prof.extra_verbose_generic_activity("renderer_after_krate", T::descr()) + prof.verbose_generic_activity_with_arg("renderer_after_krate", T::descr()) .run(|| format_renderer.after_krate()) } diff --git a/src/test/rustdoc-ui/z-help.stdout b/src/test/rustdoc-ui/z-help.stdout index 25d5c6e4ad2b9..65536cb3aa135 100644 --- a/src/test/rustdoc-ui/z-help.stdout +++ b/src/test/rustdoc-ui/z-help.stdout @@ -170,7 +170,6 @@ -Z thinlto=val -- enable ThinLTO when possible -Z thir-unsafeck=val -- use the THIR unsafety checker (default: no) -Z threads=val -- use a thread pool with N threads - -Z time=val -- measure time of rustc processes (default: no) -Z time-llvm-passes=val -- measure time of each LLVM pass (default: no) -Z time-passes=val -- measure time of each rustc pass (default: no) -Z tls-model=val -- choose the TLS model to use (`rustc --print tls-models` for details) From 4e8faff3a15968970a6810e0af0eafea14f9f7f1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 6 Oct 2022 15:39:27 +1100 Subject: [PATCH 11/13] Be consistent about deciding whether to print pass data. `print_time_passes_entry` unconditionally prints data about a pass. The most commonly used call site, in `VerboseTimingGuard::drop`, guards it with a `should_print_passes` test. But there are a couple of other call sites that don't do that test. This commit moves the `should_print_passes` test within `print_time_passes_entry` so that all passes are treated equally. --- .../rustc_data_structures/src/profiling.rs | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index 0980d36c36e86..ba1960805d84b 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -733,27 +733,9 @@ impl Drop for VerboseTimingGuard<'_> { if let Some((start_time, start_rss, ref message)) = self.start_and_message { let end_rss = get_resident_set_size(); let dur = start_time.elapsed(); - - if should_print_passes(dur, start_rss, end_rss) { - print_time_passes_entry(&message, dur, start_rss, end_rss); - } - } - } -} - -fn should_print_passes(dur: Duration, start_rss: Option, end_rss: Option) -> bool { - if dur.as_millis() > 5 { - return true; - } - - if let (Some(start_rss), Some(end_rss)) = (start_rss, end_rss) { - let change_rss = end_rss.abs_diff(start_rss); - if change_rss > 0 { - return true; + print_time_passes_entry(&message, dur, start_rss, end_rss); } } - - false } pub fn print_time_passes_entry( @@ -762,6 +744,26 @@ pub fn print_time_passes_entry( start_rss: Option, end_rss: Option, ) { + // Print the pass if its duration is greater than 5 ms, or it changed the + // measured RSS. + let is_notable = || { + if dur.as_millis() > 5 { + return true; + } + + if let (Some(start_rss), Some(end_rss)) = (start_rss, end_rss) { + let change_rss = end_rss.abs_diff(start_rss); + if change_rss > 0 { + return true; + } + } + + false + }; + if !is_notable() { + return; + } + let rss_to_mb = |rss| (rss as f64 / 1_000_000.0).round() as usize; let rss_change_to_mb = |rss| (rss as f64 / 1_000_000.0).round() as i128; From 25ed5d5db295e1fbf27e511c9159e67c22b08f43 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 6 Oct 2022 07:00:38 +0100 Subject: [PATCH 12/13] reviews --- .../rustc_hir_analysis/src/check/check.rs | 1 - .../src/check/compare_method.rs | 81 ++++++++++--------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index c47fdaefd60e9..f331354a044e0 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -1044,7 +1044,6 @@ fn check_impl_items_against_trait<'tcx>( let impl_item_full = tcx.hir().impl_item(impl_item.id); match impl_item_full.kind { hir::ImplItemKind::Const(..) => { - // Find associated const definition. let _ = tcx.compare_assoc_const_impl_item_with_trait_item(( impl_item.id.def_id.def_id, ty_impl_item.trait_item_def_id.unwrap(), diff --git a/compiler/rustc_hir_analysis/src/check/compare_method.rs b/compiler/rustc_hir_analysis/src/check/compare_method.rs index 75e56cd4be78e..d006948c58793 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_method.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_method.rs @@ -1349,53 +1349,54 @@ pub(crate) fn raw_compare_const_impl<'tcx>( debug!("compare_const_impl: trait_ty={:?}", trait_ty); - let maybe_error_reported = infcx + let err = infcx .at(&cause, param_env) .sup(trait_ty, impl_ty) - .map(|ok| ocx.register_infer_ok_obligations(ok)) - .map_err(|terr| { - debug!( - "checking associated const for compatibility: impl ty {:?}, trait ty {:?}", - impl_ty, trait_ty - ); + .map(|ok| ocx.register_infer_ok_obligations(ok)); - // Locate the Span containing just the type of the offending impl - match tcx.hir().expect_impl_item(impl_const_item_def).kind { - ImplItemKind::Const(ref ty, _) => cause.span = ty.span, - _ => bug!("{:?} is not a impl const", impl_const_item), - } + if let Err(terr) = err { + debug!( + "checking associated const for compatibility: impl ty {:?}, trait ty {:?}", + impl_ty, trait_ty + ); - let mut diag = struct_span_err!( - tcx.sess, - cause.span, - E0326, - "implemented const `{}` has an incompatible type for trait", - trait_const_item.name - ); + // Locate the Span containing just the type of the offending impl + match tcx.hir().expect_impl_item(impl_const_item_def).kind { + ImplItemKind::Const(ref ty, _) => cause.span = ty.span, + _ => bug!("{:?} is not a impl const", impl_const_item), + } - let trait_c_span = trait_const_item_def.as_local().map(|trait_c_def_id| { - // Add a label to the Span containing just the type of the const - match tcx.hir().expect_trait_item(trait_c_def_id).kind { - TraitItemKind::Const(ref ty, _) => ty.span, - _ => bug!("{:?} is not a trait const", trait_const_item), - } - }); + let mut diag = struct_span_err!( + tcx.sess, + cause.span, + E0326, + "implemented const `{}` has an incompatible type for trait", + trait_const_item.name + ); - infcx.note_type_err( - &mut diag, - &cause, - trait_c_span.map(|span| (span, "type in trait".to_owned())), - Some(infer::ValuePairs::Terms(ExpectedFound { - expected: trait_ty.into(), - found: impl_ty.into(), - })), - terr, - false, - false, - ); - diag.emit() + let trait_c_span = trait_const_item_def.as_local().map(|trait_c_def_id| { + // Add a label to the Span containing just the type of the const + match tcx.hir().expect_trait_item(trait_c_def_id).kind { + TraitItemKind::Const(ref ty, _) => ty.span, + _ => bug!("{:?} is not a trait const", trait_const_item), + } }); + infcx.note_type_err( + &mut diag, + &cause, + trait_c_span.map(|span| (span, "type in trait".to_owned())), + Some(infer::ValuePairs::Terms(ExpectedFound { + expected: trait_ty.into(), + found: impl_ty.into(), + })), + terr, + false, + false, + ); + return Err(diag.emit()); + }; + // Check that all obligations are satisfied by the implementation's // version. let errors = ocx.select_all_or_error(); @@ -1407,7 +1408,7 @@ pub(crate) fn raw_compare_const_impl<'tcx>( let outlives_environment = OutlivesEnvironment::new(param_env); infcx .check_region_obligations_and_report_errors(impl_const_item_def, &outlives_environment); - maybe_error_reported + Ok(()) }) } From 694f0a1e352fabc3a6ec54a07dedd2c6281783a7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 6 Oct 2022 13:01:07 +0200 Subject: [PATCH 13/13] Migrate search input color to CSS variable --- src/librustdoc/html/static/css/rustdoc.css | 1 + src/librustdoc/html/static/css/themes/ayu.css | 5 +---- src/librustdoc/html/static/css/themes/dark.css | 5 +---- src/librustdoc/html/static/css/themes/light.css | 1 + 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index b86b8f9dbf864..28e968c9ff5f9 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -914,6 +914,7 @@ so that we can apply CSS-filters to change the arrow color in themes */ font-size: 1rem; width: 100%; background-color: var(--button-background-color); + color: var(--search-color); } .search-input:focus { border-color: var(--search-input-focused-border-color); diff --git a/src/librustdoc/html/static/css/themes/ayu.css b/src/librustdoc/html/static/css/themes/ayu.css index 7245dce6ed6e3..0975d497cb23c 100644 --- a/src/librustdoc/html/static/css/themes/ayu.css +++ b/src/librustdoc/html/static/css/themes/ayu.css @@ -40,6 +40,7 @@ Original by Dempfi (https://github.com/dempfi/ayu) --search-result-link-focus-background-color: #3c3c3c; --stab-background-color: #314559; --stab-code-color: #e6e1cf; + --search-color: #fff; } .slider { @@ -149,10 +150,6 @@ details.rustdoc-toggle > summary::before { filter: invert(98%) sepia(12%) saturate(81%) hue-rotate(343deg) brightness(113%) contrast(76%); } -.search-input { - color: #fff; -} - .module-item .stab, .import-item .stab { color: #000; diff --git a/src/librustdoc/html/static/css/themes/dark.css b/src/librustdoc/html/static/css/themes/dark.css index 9d5406e65a8b9..ee2a9ec8a0bee 100644 --- a/src/librustdoc/html/static/css/themes/dark.css +++ b/src/librustdoc/html/static/css/themes/dark.css @@ -35,6 +35,7 @@ --search-result-link-focus-background-color: #616161; --stab-background-color: #314559; --stab-code-color: #e6e1cf; + --search-color: #111; } .slider { @@ -72,10 +73,6 @@ details.rustdoc-toggle > summary::before { filter: invert(100%); } -.search-input { - color: #111; -} - #crate-search-div::after { /* match border-color; uses https://codepen.io/sosuke/pen/Pjoqqp */ filter: invert(94%) sepia(0%) saturate(721%) hue-rotate(255deg) brightness(90%) contrast(90%); diff --git a/src/librustdoc/html/static/css/themes/light.css b/src/librustdoc/html/static/css/themes/light.css index b63895e56c687..7287d5b62123b 100644 --- a/src/librustdoc/html/static/css/themes/light.css +++ b/src/librustdoc/html/static/css/themes/light.css @@ -35,6 +35,7 @@ --search-result-link-focus-background-color: #ccc; --stab-background-color: #fff5d6; --stab-code-color: #000; + --search-color: #000; } .slider {