From 36d7f763f03f9110e0d6b7052799f7ec40cc3c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 13 Feb 2024 04:20:04 +0100 Subject: [PATCH 01/10] Replace clean::InstantiationParam with clean::GenericArg --- src/librustdoc/clean/mod.rs | 52 +++++++++-------------------------- src/librustdoc/clean/types.rs | 39 +++++++------------------- src/librustdoc/core.rs | 13 +++++---- 3 files changed, 31 insertions(+), 73 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index f4527d1e55e85..77a7d33f6e7c5 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -254,16 +254,14 @@ fn clean_poly_trait_ref_with_bindings<'tcx>( } fn clean_lifetime<'tcx>(lifetime: &hir::Lifetime, cx: &mut DocContext<'tcx>) -> Lifetime { - let def = cx.tcx.named_bound_var(lifetime.hir_id); if let Some( - rbv::ResolvedArg::EarlyBound(node_id) - | rbv::ResolvedArg::LateBound(_, _, node_id) - | rbv::ResolvedArg::Free(_, node_id), - ) = def + rbv::ResolvedArg::EarlyBound(did) + | rbv::ResolvedArg::LateBound(_, _, did) + | rbv::ResolvedArg::Free(_, did), + ) = cx.tcx.named_bound_var(lifetime.hir_id) + && let Some(lt) = cx.args.get(&did).and_then(|arg| arg.as_lt()) { - if let Some(lt) = cx.args.get(&node_id).and_then(|p| p.as_lt()).cloned() { - return lt; - } + return lt.clone(); } Lifetime(lifetime.ident.name) } @@ -1791,12 +1789,12 @@ fn maybe_expand_private_type_alias<'tcx>( _ => None, }); if let Some(lt) = lifetime { - let cleaned = if !lt.is_anonymous() { + let lt = if !lt.is_anonymous() { clean_lifetime(lt, cx) } else { Lifetime::elided() }; - args.insert(param.def_id.to_def_id(), InstantiationParam::Lifetime(cleaned)); + args.insert(param.def_id.to_def_id(), GenericArg::Lifetime(lt)); } indices.lifetimes += 1; } @@ -1805,44 +1803,20 @@ fn maybe_expand_private_type_alias<'tcx>( let type_ = generic_args.args.iter().find_map(|arg| match arg { hir::GenericArg::Type(ty) => { if indices.types == j { - return Some(ty); + return Some(*ty); } j += 1; None } _ => None, }); - if let Some(ty) = type_ { - args.insert( - param.def_id.to_def_id(), - InstantiationParam::Type(clean_ty(ty, cx)), - ); - } else if let Some(default) = *default { - args.insert( - param.def_id.to_def_id(), - InstantiationParam::Type(clean_ty(default, cx)), - ); + if let Some(ty) = type_.or(*default) { + args.insert(param.def_id.to_def_id(), GenericArg::Type(clean_ty(ty, cx))); } indices.types += 1; } - hir::GenericParamKind::Const { .. } => { - let mut j = 0; - let const_ = generic_args.args.iter().find_map(|arg| match arg { - hir::GenericArg::Const(ct) => { - if indices.consts == j { - return Some(ct); - } - j += 1; - None - } - _ => None, - }); - if let Some(_) = const_ { - args.insert(param.def_id.to_def_id(), InstantiationParam::Constant); - } - // FIXME(const_generics_defaults) - indices.consts += 1; - } + // FIXME(#82852): Instantiate const parameters. + hir::GenericParamKind::Const { .. } => {} } } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 96b4d1a45f6ea..cee1b25591d33 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -2228,6 +2228,16 @@ pub(crate) enum GenericArg { Infer, } +impl GenericArg { + pub(crate) fn as_lt(&self) -> Option<&Lifetime> { + if let Self::Lifetime(lt) = self { Some(lt) } else { None } + } + + pub(crate) fn as_ty(&self) -> Option<&Type> { + if let Self::Type(ty) = self { Some(ty) } else { None } + } +} + #[derive(Clone, PartialEq, Eq, Debug, Hash)] pub(crate) enum GenericArgs { AngleBracketed { args: Box<[GenericArg]>, bindings: ThinVec }, @@ -2530,35 +2540,6 @@ pub(crate) enum TypeBindingKind { Constraint { bounds: Vec }, } -/// The type, lifetime, or constant that a private type alias's parameter should be -/// replaced with when expanding a use of that type alias. -/// -/// For example: -/// -/// ``` -/// type PrivAlias = Vec; -/// -/// pub fn public_fn() -> PrivAlias { vec![] } -/// ``` -/// -/// `public_fn`'s docs will show it as returning `Vec`, since `PrivAlias` is private. -/// [`InstantiationParam`] is used to record that `T` should be mapped to `i32`. -pub(crate) enum InstantiationParam { - Type(Type), - Lifetime(Lifetime), - Constant, -} - -impl InstantiationParam { - pub(crate) fn as_ty(&self) -> Option<&Type> { - if let Self::Type(ty) = self { Some(ty) } else { None } - } - - pub(crate) fn as_lt(&self) -> Option<&Lifetime> { - if let Self::Lifetime(lt) = self { Some(lt) } else { None } - } -} - // Some nodes are used a lot. Make sure they don't unintentionally get bigger. #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] mod size_asserts { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 28ccda39e4de2..efad5a8d808dd 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -44,10 +44,13 @@ pub(crate) struct DocContext<'tcx> { /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. pub(crate) active_extern_traits: DefIdSet, - // The current set of parameter instantiations, - // for expanding type aliases at the HIR level: - /// Table `DefId` of type, lifetime, or const parameter -> instantiated type, lifetime, or const - pub(crate) args: DefIdMap, + /// The current set of parameter instantiations for expanding type aliases at the HIR level. + /// + /// Maps from the `DefId` of a lifetime or type parameter to the + /// generic argument it's currently instantiated to in this context. + // FIXME(#82852): We don't record const params since we don't visit const exprs at all and + // therefore wouldn't use the corresp. generic arg anyway. Add support for them. + pub(crate) args: DefIdMap, pub(crate) current_type_aliases: DefIdMap, /// Table synthetic type parameter for `impl Trait` in argument position -> bounds pub(crate) impl_trait_bounds: FxHashMap>, @@ -87,7 +90,7 @@ impl<'tcx> DocContext<'tcx> { /// the generic parameters for a type alias' RHS. pub(crate) fn enter_alias( &mut self, - args: DefIdMap, + args: DefIdMap, def_id: DefId, f: F, ) -> R From ae92334c0f5bc35abe4c197818c82690b9268f93 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 13 Feb 2024 05:34:09 +0100 Subject: [PATCH 02/10] remove questionable calls to `commit_if_ok` --- compiler/rustc_infer/src/infer/at.rs | 48 ++++++-------- .../src/infer/canonical/query_response.rs | 66 +++++++++---------- .../src/traits/select/confirmation.rs | 12 ++-- .../expect-infer-var-appearing-twice.stderr | 4 +- 4 files changed, 60 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_infer/src/infer/at.rs b/compiler/rustc_infer/src/infer/at.rs index 859593e119457..05b9479c7b062 100644 --- a/compiler/rustc_infer/src/infer/at.rs +++ b/compiler/rustc_infer/src/infer/at.rs @@ -285,13 +285,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> { T: Relate<'tcx>, { let Trace { at, trace, a_is_expected } = self; - at.infcx.commit_if_ok(|_| { - let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); - fields - .sub(a_is_expected) - .relate(a, b) - .map(move |_| InferOk { value: (), obligations: fields.obligations }) - }) + let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); + fields + .sub(a_is_expected) + .relate(a, b) + .map(move |_| InferOk { value: (), obligations: fields.obligations }) } /// Makes `a == b`; the expectation is set by the call to @@ -302,13 +300,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> { T: Relate<'tcx>, { let Trace { at, trace, a_is_expected } = self; - at.infcx.commit_if_ok(|_| { - let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); - fields - .equate(a_is_expected) - .relate(a, b) - .map(move |_| InferOk { value: (), obligations: fields.obligations }) - }) + let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); + fields + .equate(a_is_expected) + .relate(a, b) + .map(move |_| InferOk { value: (), obligations: fields.obligations }) } #[instrument(skip(self), level = "debug")] @@ -317,13 +313,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> { T: Relate<'tcx>, { let Trace { at, trace, a_is_expected } = self; - at.infcx.commit_if_ok(|_| { - let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); - fields - .lub(a_is_expected) - .relate(a, b) - .map(move |t| InferOk { value: t, obligations: fields.obligations }) - }) + let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); + fields + .lub(a_is_expected) + .relate(a, b) + .map(move |t| InferOk { value: t, obligations: fields.obligations }) } #[instrument(skip(self), level = "debug")] @@ -332,13 +326,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> { T: Relate<'tcx>, { let Trace { at, trace, a_is_expected } = self; - at.infcx.commit_if_ok(|_| { - let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); - fields - .glb(a_is_expected) - .relate(a, b) - .map(move |t| InferOk { value: t, obligations: fields.obligations }) - }) + let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types); + fields + .glb(a_is_expected) + .relate(a, b) + .map(move |t| InferOk { value: t, obligations: fields.obligations }) } } diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index 216b2e70abff6..56a45586c9d2e 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -620,42 +620,40 @@ impl<'tcx> InferCtxt<'tcx> { variables1: &OriginalQueryValues<'tcx>, variables2: impl Fn(BoundVar) -> GenericArg<'tcx>, ) -> InferResult<'tcx, ()> { - self.commit_if_ok(|_| { - let mut obligations = vec![]; - for (index, value1) in variables1.var_values.iter().enumerate() { - let value2 = variables2(BoundVar::new(index)); - - match (value1.unpack(), value2.unpack()) { - (GenericArgKind::Type(v1), GenericArgKind::Type(v2)) => { - obligations.extend( - self.at(cause, param_env) - .eq(DefineOpaqueTypes::Yes, v1, v2)? - .into_obligations(), - ); - } - (GenericArgKind::Lifetime(re1), GenericArgKind::Lifetime(re2)) - if re1.is_erased() && re2.is_erased() => - { - // no action needed - } - (GenericArgKind::Lifetime(v1), GenericArgKind::Lifetime(v2)) => { - obligations.extend( - self.at(cause, param_env) - .eq(DefineOpaqueTypes::Yes, v1, v2)? - .into_obligations(), - ); - } - (GenericArgKind::Const(v1), GenericArgKind::Const(v2)) => { - let ok = self.at(cause, param_env).eq(DefineOpaqueTypes::Yes, v1, v2)?; - obligations.extend(ok.into_obligations()); - } - _ => { - bug!("kind mismatch, cannot unify {:?} and {:?}", value1, value2,); - } + let mut obligations = vec![]; + for (index, value1) in variables1.var_values.iter().enumerate() { + let value2 = variables2(BoundVar::new(index)); + + match (value1.unpack(), value2.unpack()) { + (GenericArgKind::Type(v1), GenericArgKind::Type(v2)) => { + obligations.extend( + self.at(cause, param_env) + .eq(DefineOpaqueTypes::Yes, v1, v2)? + .into_obligations(), + ); + } + (GenericArgKind::Lifetime(re1), GenericArgKind::Lifetime(re2)) + if re1.is_erased() && re2.is_erased() => + { + // no action needed + } + (GenericArgKind::Lifetime(v1), GenericArgKind::Lifetime(v2)) => { + obligations.extend( + self.at(cause, param_env) + .eq(DefineOpaqueTypes::Yes, v1, v2)? + .into_obligations(), + ); + } + (GenericArgKind::Const(v1), GenericArgKind::Const(v2)) => { + let ok = self.at(cause, param_env).eq(DefineOpaqueTypes::Yes, v1, v2)?; + obligations.extend(ok.into_obligations()); + } + _ => { + bug!("kind mismatch, cannot unify {:?} and {:?}", value1, value2,); } } - Ok(InferOk { value: (), obligations }) - }) + } + Ok(InferOk { value: (), obligations }) } } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 2ce33a4d122d9..e4a70c537d201 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -192,13 +192,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { &mut obligations, ); - obligations.extend(self.infcx.commit_if_ok(|_| { + obligations.extend( self.infcx .at(&obligation.cause, obligation.param_env) .sup(DefineOpaqueTypes::No, placeholder_trait_predicate, candidate) .map(|InferOk { obligations, .. }| obligations) - .map_err(|_| Unimplemented) - })?); + .map_err(|_| Unimplemented)?, + ); // FIXME(compiler-errors): I don't think this is needed. if let ty::Alias(ty::Projection, alias_ty) = placeholder_self_ty.kind() { @@ -532,13 +532,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { &mut nested, ); - nested.extend(self.infcx.commit_if_ok(|_| { + nested.extend( self.infcx .at(&obligation.cause, obligation.param_env) .sup(DefineOpaqueTypes::No, obligation_trait_ref, upcast_trait_ref) .map(|InferOk { obligations, .. }| obligations) - .map_err(|_| Unimplemented) - })?); + .map_err(|_| Unimplemented)?, + ); // Check supertraits hold. This is so that their associated type bounds // will be checked in the code below. diff --git a/tests/ui/closure-expected-type/expect-infer-var-appearing-twice.stderr b/tests/ui/closure-expected-type/expect-infer-var-appearing-twice.stderr index e2a2db7c3f098..c0f222b016db7 100644 --- a/tests/ui/closure-expected-type/expect-infer-var-appearing-twice.stderr +++ b/tests/ui/closure-expected-type/expect-infer-var-appearing-twice.stderr @@ -9,8 +9,8 @@ LL | | LL | | }); | |______^ expected due to this | - = note: expected closure signature `fn(_, _) -> _` - found closure signature `fn(u32, i32) -> _` + = note: expected closure signature `fn(_, u32) -> _` + found closure signature `fn(_, i32) -> _` note: required by a bound in `with_closure` --> $DIR/expect-infer-var-appearing-twice.rs:2:14 | From 622b5a5cee22b52a8e96eb43e3d31025f1cdf124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 13 Feb 2024 06:35:19 +0100 Subject: [PATCH 03/10] Remove jsha from the rustdoc review rotation --- triagebot.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index 383b89b18df57..deb947423bcff 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -680,7 +680,6 @@ infra-ci = [ "@Kobzol", ] rustdoc = [ - "@jsha", "@GuillaumeGomez", "@notriddle", "@fmease", From 2b4a2b95dd1a6b85678537332045d122afda682f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 30 Jan 2024 14:22:05 +0000 Subject: [PATCH 04/10] Check normalized call signature for WF in mir typeck --- compiler/rustc_borrowck/src/type_check/mod.rs | 24 ++++++++-- tests/ui/nll/check-normalized-sig-for-wf.rs | 27 +++++++++++ .../ui/nll/check-normalized-sig-for-wf.stderr | 47 +++++++++++++++++++ 3 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 tests/ui/nll/check-normalized-sig-for-wf.rs create mode 100644 tests/ui/nll/check-normalized-sig-for-wf.stderr diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index cfb46f3ac8a96..ae4000f02a7d6 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -1432,7 +1432,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { return; } }; - let (sig, map) = tcx.instantiate_bound_regions(sig, |br| { + let (unnormalized_sig, map) = tcx.instantiate_bound_regions(sig, |br| { use crate::renumber::RegionCtxt; let region_ctxt_fn = || { @@ -1454,7 +1454,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { region_ctxt_fn, ) }); - debug!(?sig); + debug!(?unnormalized_sig); // IMPORTANT: We have to prove well formed for the function signature before // we normalize it, as otherwise types like `<&'a &'b () as Trait>::Assoc` // get normalized away, causing us to ignore the `'b: 'a` bound used by the function. @@ -1464,7 +1464,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // // See #91068 for an example. self.prove_predicates( - sig.inputs_and_output.iter().map(|ty| { + unnormalized_sig.inputs_and_output.iter().map(|ty| { ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed( ty.into(), ))) @@ -1472,7 +1472,23 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { term_location.to_locations(), ConstraintCategory::Boring, ); - let sig = self.normalize(sig, term_location); + + let sig = self.normalize(unnormalized_sig, term_location); + // HACK(#114936): `WF(sig)` does not imply `WF(normalized(sig))` + // with built-in `Fn` implementations, since the impl may not be + // well-formed itself. + if sig != unnormalized_sig { + self.prove_predicates( + sig.inputs_and_output.iter().map(|ty| { + ty::Binder::dummy(ty::PredicateKind::Clause( + ty::ClauseKind::WellFormed(ty.into()), + )) + }), + term_location.to_locations(), + ConstraintCategory::Boring, + ); + } + self.check_call_dest(body, term, &sig, *destination, *target, term_location); // The ordinary liveness rules will ensure that all diff --git a/tests/ui/nll/check-normalized-sig-for-wf.rs b/tests/ui/nll/check-normalized-sig-for-wf.rs new file mode 100644 index 0000000000000..cb0f34ce02f7a --- /dev/null +++ b/tests/ui/nll/check-normalized-sig-for-wf.rs @@ -0,0 +1,27 @@ +// +fn whoops( + s: String, + f: impl for<'s> FnOnce(&'s str) -> (&'static str, [&'static &'s (); 0]), +) -> &'static str +{ + f(&s).0 + //~^ ERROR `s` does not live long enough +} + +// +fn extend(input: &T) -> &'static T { + struct Bounded<'a, 'b: 'static, T>(&'a T, [&'b (); 0]); + let n: Box Bounded<'static, '_, T>> = Box::new(|x| Bounded(x, [])); + n(input).0 + //~^ ERROR borrowed data escapes outside of function +} + +// +fn extend_mut<'a, T>(input: &'a mut T) -> &'static mut T { + struct Bounded<'a, 'b: 'static, T>(&'a mut T, [&'b (); 0]); + let mut n: Box Bounded<'static, '_, T>> = Box::new(|x| Bounded(x, [])); + n(input).0 + //~^ ERROR borrowed data escapes outside of function +} + +fn main() {} diff --git a/tests/ui/nll/check-normalized-sig-for-wf.stderr b/tests/ui/nll/check-normalized-sig-for-wf.stderr new file mode 100644 index 0000000000000..5c96b0c6561a1 --- /dev/null +++ b/tests/ui/nll/check-normalized-sig-for-wf.stderr @@ -0,0 +1,47 @@ +error[E0597]: `s` does not live long enough + --> $DIR/check-normalized-sig-for-wf.rs:7:7 + | +LL | s: String, + | - binding `s` declared here +... +LL | f(&s).0 + | --^^- + | | | + | | borrowed value does not live long enough + | argument requires that `s` is borrowed for `'static` +LL | +LL | } + | - `s` dropped here while still borrowed + +error[E0521]: borrowed data escapes outside of function + --> $DIR/check-normalized-sig-for-wf.rs:15:5 + | +LL | fn extend(input: &T) -> &'static T { + | ----- - let's call the lifetime of this reference `'1` + | | + | `input` is a reference that is only valid in the function body +... +LL | n(input).0 + | ^^^^^^^^ + | | + | `input` escapes the function body here + | argument requires that `'1` must outlive `'static` + +error[E0521]: borrowed data escapes outside of function + --> $DIR/check-normalized-sig-for-wf.rs:23:5 + | +LL | fn extend_mut<'a, T>(input: &'a mut T) -> &'static mut T { + | -- ----- `input` is a reference that is only valid in the function body + | | + | lifetime `'a` defined here +... +LL | n(input).0 + | ^^^^^^^^ + | | + | `input` escapes the function body here + | argument requires that `'a` must outlive `'static` + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0521, E0597. +For more information about an error, try `rustc --explain E0521`. From 57746a3621949af46be86b736a9dec96b772ee66 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 13 Feb 2024 18:51:47 +0100 Subject: [PATCH 05/10] add lcnr to the compiler-team assignment group --- triagebot.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/triagebot.toml b/triagebot.toml index 383b89b18df57..f4ae8cc85d7d5 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -651,6 +651,7 @@ compiler-team = [ "@petrochenkov", "@davidtwco", "@estebank", + "@lcnr", "@oli-obk", "@pnkfelix", "@wesleywiser", From cd3ba4a8852b20ba6aaa55db20bb415d4333c7c7 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Tue, 13 Feb 2024 14:03:59 -0500 Subject: [PATCH 06/10] Fix incorrect use of `compile_fail` `compile_fail` should only be used when the code is meant to show what *not* to do. In other words, there should be a fundamental flaw in the code. However, in this case, the example is just incomplete, so we should use `ignore` to avoid confusing readers. --- library/std/src/process.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 4a7f5d8e0bee6..669affa266a4d 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -171,7 +171,7 @@ pub struct Child { /// The handle for writing to the child's standard input (stdin), if it /// has been captured. You might find it helpful to do /// - /// ```compile_fail,E0425 + /// ```ignore (incomplete) /// let stdin = child.stdin.take().unwrap(); /// ``` /// @@ -183,7 +183,7 @@ pub struct Child { /// The handle for reading from the child's standard output (stdout), if it /// has been captured. You might find it helpful to do /// - /// ```compile_fail,E0425 + /// ```ignore (incomplete) /// let stdout = child.stdout.take().unwrap(); /// ``` /// @@ -195,7 +195,7 @@ pub struct Child { /// The handle for reading from the child's standard error (stderr), if it /// has been captured. You might find it helpful to do /// - /// ```compile_fail,E0425 + /// ```ignore (incomplete) /// let stderr = child.stderr.take().unwrap(); /// ``` /// From b4eee2e8b3bad92465959679901fd5ecb4ada743 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 13 Feb 2024 19:20:13 +0000 Subject: [PATCH 07/10] Do not assemble candidates for default impls --- .../src/solve/assembly/mod.rs | 14 ++++++++++++ .../src/traits/select/candidate_assembly.rs | 8 +++++++ .../specialization-trait-not-implemented.rs | 2 +- ...pecialization-trait-not-implemented.stderr | 18 +++------------ .../specialization/defaultimpl/validation.rs | 1 + .../defaultimpl/validation.stderr | 22 ++++++++++++++----- .../specialization/issue-45814.current.stderr | 16 ++++++-------- .../issue-45814.negative.stderr | 16 ++++++-------- tests/ui/specialization/issue-45814.rs | 2 +- 9 files changed, 59 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs index 6833d2ae330ce..4e78988a0da1e 100644 --- a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs @@ -338,6 +338,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { let mut consider_impls_for_simplified_type = |simp| { if let Some(impls_for_type) = trait_impls.non_blanket_impls().get(&simp) { for &impl_def_id in impls_for_type { + // For every `default impl`, there's always a non-default `impl` + // that will *also* apply. There's no reason to register a candidate + // for this impl, since it is *not* proof that the trait goal holds. + if tcx.defaultness(impl_def_id).is_default() { + return; + } + match G::consider_impl_candidate(self, goal, impl_def_id) { Ok(candidate) => candidates.push(candidate), Err(NoSolution) => (), @@ -441,6 +448,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { let tcx = self.tcx(); let trait_impls = tcx.trait_impls_of(goal.predicate.trait_def_id(tcx)); for &impl_def_id in trait_impls.blanket_impls() { + // For every `default impl`, there's always a non-default `impl` + // that will *also* apply. There's no reason to register a candidate + // for this impl, since it is *not* proof that the trait goal holds. + if tcx.defaultness(impl_def_id).is_default() { + return; + } + match G::consider_impl_candidate(self, goal, impl_def_id) { Ok(candidate) => candidates.push(candidate), Err(NoSolution) => (), diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 31a0d6271fa4a..149dc4c75a7e8 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -566,6 +566,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { { return; } + + // For every `default impl`, there's always a non-default `impl` + // that will *also* apply. There's no reason to register a candidate + // for this impl, since it is *not* proof that the trait goal holds. + if self.tcx().defaultness(impl_def_id).is_default() { + return; + } + if self.reject_fn_ptr_impls( impl_def_id, obligation, diff --git a/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.rs b/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.rs index 6834d57362991..344dd7bb288e7 100644 --- a/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.rs +++ b/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.rs @@ -20,5 +20,5 @@ default impl Foo for T { fn main() { println!("{}", MyStruct.foo_one()); - //~^ ERROR the method + //~^ ERROR no method named `foo_one` found for struct `MyStruct` in the current scope } diff --git a/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.stderr b/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.stderr index e9b0845ccf707..74f81bb023ef4 100644 --- a/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.stderr +++ b/tests/ui/specialization/defaultimpl/specialization-trait-not-implemented.stderr @@ -8,27 +8,15 @@ LL | #![feature(specialization)] = help: consider using `min_specialization` instead, which is more stable and complete = note: `#[warn(incomplete_features)]` on by default -error[E0599]: the method `foo_one` exists for struct `MyStruct`, but its trait bounds were not satisfied +error[E0599]: no method named `foo_one` found for struct `MyStruct` in the current scope --> $DIR/specialization-trait-not-implemented.rs:22:29 | LL | struct MyStruct; - | --------------- method `foo_one` not found for this struct because it doesn't satisfy `MyStruct: Foo` + | --------------- method `foo_one` not found for this struct ... LL | println!("{}", MyStruct.foo_one()); - | ^^^^^^^ method cannot be called on `MyStruct` due to unsatisfied trait bounds + | ^^^^^^^ method not found in `MyStruct` | -note: trait bound `MyStruct: Foo` was not satisfied - --> $DIR/specialization-trait-not-implemented.rs:14:1 - | -LL | default impl Foo for T { - | ^^^^^^^^^^^^^^^^---^^^^^- - | | - | unsatisfied trait bound introduced here -note: the trait `Foo` must be implemented - --> $DIR/specialization-trait-not-implemented.rs:7:1 - | -LL | trait Foo { - | ^^^^^^^^^ = help: items from traits can only be used if the trait is implemented and in scope note: `Foo` defines an item `foo_one`, perhaps you need to implement it --> $DIR/specialization-trait-not-implemented.rs:7:1 diff --git a/tests/ui/specialization/defaultimpl/validation.rs b/tests/ui/specialization/defaultimpl/validation.rs index 8558a1efb82f3..4049c4ea14c5b 100644 --- a/tests/ui/specialization/defaultimpl/validation.rs +++ b/tests/ui/specialization/defaultimpl/validation.rs @@ -7,6 +7,7 @@ struct Z; default impl S {} //~ ERROR inherent impls cannot be `default` default unsafe impl Send for S {} //~ ERROR impls of auto traits cannot be default +//~^ ERROR `S` cannot be sent between threads safely default impl !Send for Z {} //~ ERROR impls of auto traits cannot be default //~^ ERROR negative impls cannot be default impls diff --git a/tests/ui/specialization/defaultimpl/validation.stderr b/tests/ui/specialization/defaultimpl/validation.stderr index eb6dc9355a3a5..5f62e8dce17e4 100644 --- a/tests/ui/specialization/defaultimpl/validation.stderr +++ b/tests/ui/specialization/defaultimpl/validation.stderr @@ -26,8 +26,19 @@ LL | default unsafe impl Send for S {} | | | default because of this +error[E0277]: `S` cannot be sent between threads safely + --> $DIR/validation.rs:9:1 + | +LL | default unsafe impl Send for S {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `S` cannot be sent between threads safely + | + = help: the trait `Send` is not implemented for `S` + = help: the trait `Send` is implemented for `S` + = help: see issue #48214 + = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable + error: impls of auto traits cannot be default - --> $DIR/validation.rs:10:15 + --> $DIR/validation.rs:11:15 | LL | default impl !Send for Z {} | ------- ^^^^ auto trait @@ -35,17 +46,18 @@ LL | default impl !Send for Z {} | default because of this error[E0750]: negative impls cannot be default impls - --> $DIR/validation.rs:10:1 + --> $DIR/validation.rs:11:1 | LL | default impl !Send for Z {} | ^^^^^^^ ^ error[E0750]: negative impls cannot be default impls - --> $DIR/validation.rs:14:1 + --> $DIR/validation.rs:15:1 | LL | default impl !Tr for S {} | ^^^^^^^ ^ -error: aborting due to 5 previous errors; 1 warning emitted +error: aborting due to 6 previous errors; 1 warning emitted -For more information about this error, try `rustc --explain E0750`. +Some errors have detailed explanations: E0277, E0750. +For more information about an error, try `rustc --explain E0277`. diff --git a/tests/ui/specialization/issue-45814.current.stderr b/tests/ui/specialization/issue-45814.current.stderr index da0dff78e2630..b89d3073a8f07 100644 --- a/tests/ui/specialization/issue-45814.current.stderr +++ b/tests/ui/specialization/issue-45814.current.stderr @@ -1,14 +1,12 @@ -error[E0275]: overflow evaluating the requirement `T: Trait<_>` - | - = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_45814`) -note: required for `T` to implement `Trait<_>` - --> $DIR/issue-45814.rs:9:20 +error[E0119]: conflicting implementations of trait `Trait<_>` + --> $DIR/issue-45814.rs:10:1 | LL | default impl Trait for U {} - | ^^^^^^^^ ^ - = note: 128 redundant requirements hidden - = note: required for `T` to implement `Trait<_>` + | --------------------------------- first implementation here +LL | +LL | impl Trait<::Item> for T {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0275`. +For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/specialization/issue-45814.negative.stderr b/tests/ui/specialization/issue-45814.negative.stderr index da0dff78e2630..b89d3073a8f07 100644 --- a/tests/ui/specialization/issue-45814.negative.stderr +++ b/tests/ui/specialization/issue-45814.negative.stderr @@ -1,14 +1,12 @@ -error[E0275]: overflow evaluating the requirement `T: Trait<_>` - | - = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_45814`) -note: required for `T` to implement `Trait<_>` - --> $DIR/issue-45814.rs:9:20 +error[E0119]: conflicting implementations of trait `Trait<_>` + --> $DIR/issue-45814.rs:10:1 | LL | default impl Trait for U {} - | ^^^^^^^^ ^ - = note: 128 redundant requirements hidden - = note: required for `T` to implement `Trait<_>` + | --------------------------------- first implementation here +LL | +LL | impl Trait<::Item> for T {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0275`. +For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/specialization/issue-45814.rs b/tests/ui/specialization/issue-45814.rs index fce236390c2b0..832d734d9450a 100644 --- a/tests/ui/specialization/issue-45814.rs +++ b/tests/ui/specialization/issue-45814.rs @@ -1,4 +1,3 @@ -//~ ERROR overflow evaluating the requirement `T: Trait<_>` // revisions: current negative #![feature(specialization)] #![cfg_attr(negative, feature(with_negative_coherence))] @@ -9,5 +8,6 @@ pub trait Trait {} default impl Trait for U {} impl Trait<::Item> for T {} +//~^ ERROR conflicting implementations of trait `Trait<_>` fn main() {} From bdc6d82f9ac21a9c0b8a5e3f5728a5cbb50a09e2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 09:13:52 +1100 Subject: [PATCH 08/10] Make `struct_span_note` call `struct_note`. So it follows the same pattern as all the other `struct_span_*` methods. --- compiler/rustc_errors/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index e033d66fccf41..7f78ea7aa56b0 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1179,7 +1179,7 @@ impl DiagCtxt { span: impl Into, msg: impl Into, ) -> DiagnosticBuilder<'_, ()> { - DiagnosticBuilder::new(self, Note, msg).with_span(span) + self.struct_note(msg).with_span(span) } #[rustc_lint_diagnostics] From c1ffb0b675c5bb7fb5d91c19fcb9171f873511d0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 08:19:55 +1100 Subject: [PATCH 09/10] Remove `force_print_diagnostic`. There are a couple of places where we call `inner.emitter.emit_diagnostic` directly rather than going through `inner.emit_diagnostic`, to guarantee the diagnostic is printed. This feels dubious to me, particularly the bypassing of `TRACK_DIAGNOSTIC`. This commit removes those. - In `print_error_count`, it uses `ForceWarning` instead of `Warning`. - It removes `DiagCtxtInner::failure_note`, because it only has three uses and direct use of `emit_diagnostic` is consistent with other similar locations. - It removes `force_print_diagnostic`, and adds `struct_failure_note`, and updates `print_query_stack` accordingly, which makes it more normal. That location doesn't seem to need forced printing anyway. --- compiler/rustc_errors/src/lib.rs | 43 +++++++++++--------- compiler/rustc_query_system/src/query/job.rs | 20 ++++----- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 7f78ea7aa56b0..965a62e9a8b8e 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -780,11 +780,12 @@ impl DiagCtxt { match (errors.len(), warnings.len()) { (0, 0) => return, (0, _) => { - // Use `inner.emitter` directly, otherwise the warning might not be emitted, e.g. - // with a configuration like `--cap-lints allow --force-warn bare_trait_objects`. - inner - .emitter - .emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))); + // Use `ForceWarning` rather than `Warning` to guarantee emission, e.g. with a + // configuration like `--cap-lints allow --force-warn bare_trait_objects`. + inner.emit_diagnostic(Diagnostic::new( + ForceWarning(None), + DiagnosticMessage::Str(warnings), + )); } (_, 0) => { inner.emit_diagnostic(Diagnostic::new(Error, errors)); @@ -812,20 +813,23 @@ impl DiagCtxt { error_codes.sort(); if error_codes.len() > 1 { let limit = if error_codes.len() > 9 { 9 } else { error_codes.len() }; - inner.failure_note(format!( + let msg1 = format!( "Some errors have detailed explanations: {}{}", error_codes[..limit].join(", "), if error_codes.len() > 9 { "..." } else { "." } - )); - inner.failure_note(format!( + ); + let msg2 = format!( "For more information about an error, try `rustc --explain {}`.", &error_codes[0] - )); + ); + inner.emit_diagnostic(Diagnostic::new(FailureNote, msg1)); + inner.emit_diagnostic(Diagnostic::new(FailureNote, msg2)); } else { - inner.failure_note(format!( + let msg = format!( "For more information about this error, try `rustc --explain {}`.", &error_codes[0] - )); + ); + inner.emit_diagnostic(Diagnostic::new(FailureNote, msg)); } } } @@ -848,10 +852,6 @@ impl DiagCtxt { self.inner.borrow_mut().taught_diagnostics.insert(code) } - pub fn force_print_diagnostic(&self, db: Diagnostic) { - self.inner.borrow_mut().emitter.emit_diagnostic(db); - } - pub fn emit_diagnostic(&self, diagnostic: Diagnostic) -> Option { self.inner.borrow_mut().emit_diagnostic(diagnostic) } @@ -1207,6 +1207,15 @@ impl DiagCtxt { DiagnosticBuilder::new(self, Help, msg) } + #[rustc_lint_diagnostics] + #[track_caller] + pub fn struct_failure_note( + &self, + msg: impl Into, + ) -> DiagnosticBuilder<'_, ()> { + DiagnosticBuilder::new(self, FailureNote, msg) + } + #[rustc_lint_diagnostics] #[track_caller] pub fn struct_allow(&self, msg: impl Into) -> DiagnosticBuilder<'_, ()> { @@ -1406,10 +1415,6 @@ impl DiagCtxtInner { .or_else(|| self.delayed_bugs.get(0).map(|(_, guar)| guar).copied()) } - fn failure_note(&mut self, msg: impl Into) { - self.emit_diagnostic(Diagnostic::new(FailureNote, msg)); - } - fn flush_delayed(&mut self) { if self.delayed_bugs.is_empty() { return; diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 3ef9de7da74b2..8d7c0ca014490 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -4,7 +4,7 @@ use crate::query::plumbing::CycleError; use crate::query::DepKind; use crate::query::{QueryContext, QueryStackFrame}; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{DiagCtxt, Diagnostic, DiagnosticBuilder, Level}; +use rustc_errors::{DiagCtxt, DiagnosticBuilder}; use rustc_hir::def::DefKind; use rustc_session::Session; use rustc_span::Span; @@ -628,15 +628,15 @@ pub fn print_query_stack( }; if Some(count_printed) < num_frames || num_frames.is_none() { // Only print to stderr as many stack frames as `num_frames` when present. - let mut diag = Diagnostic::new( - Level::FailureNote, - format!( - "#{} [{:?}] {}", - count_printed, query_info.query.dep_kind, query_info.query.description - ), - ); - diag.span = query_info.job.span.into(); - dcx.force_print_diagnostic(diag); + // FIXME: needs translation + #[allow(rustc::diagnostic_outside_of_impl)] + #[allow(rustc::untranslatable_diagnostic)] + dcx.struct_failure_note(format!( + "#{} [{:?}] {}", + count_printed, query_info.query.dep_kind, query_info.query.description + )) + .with_span(query_info.job.span) + .emit(); count_printed += 1; } From 56b451a67ad058b8d28a0d9f566ec63a36dce9d5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 16:19:40 +1100 Subject: [PATCH 10/10] Fix `DiagCtxtInner::reset_err_count`. Several fields were not being reset. Using destructuring makes it much harder to miss a field. --- compiler/rustc_errors/src/lib.rs | 60 ++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 965a62e9a8b8e..fbd812609eee2 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -78,6 +78,7 @@ use std::fmt; use std::hash::Hash; use std::io::Write; use std::num::NonZeroUsize; +use std::ops::DerefMut; use std::panic; use std::path::{Path, PathBuf}; @@ -666,22 +667,51 @@ impl DiagCtxt { /// tools that want to reuse a `Parser` cleaning the previously emitted diagnostics as well as /// the overall count of emitted error diagnostics. pub fn reset_err_count(&self) { + // Use destructuring so that if a field gets added to `DiagCtxtInner`, it's impossible to + // fail to update this method as well. let mut inner = self.inner.borrow_mut(); - inner.stashed_err_count = 0; - inner.deduplicated_err_count = 0; - inner.deduplicated_warn_count = 0; - inner.must_produce_diag = false; - inner.has_printed = false; - inner.suppressed_expected_diag = false; - - // actually free the underlying memory (which `clear` would not do) - inner.err_guars = Default::default(); - inner.lint_err_guars = Default::default(); - inner.delayed_bugs = Default::default(); - inner.taught_diagnostics = Default::default(); - inner.emitted_diagnostic_codes = Default::default(); - inner.emitted_diagnostics = Default::default(); - inner.stashed_diagnostics = Default::default(); + let DiagCtxtInner { + flags: _, + err_guars, + lint_err_guars, + delayed_bugs, + stashed_err_count, + deduplicated_err_count, + deduplicated_warn_count, + emitter: _, + must_produce_diag, + has_printed, + suppressed_expected_diag, + taught_diagnostics, + emitted_diagnostic_codes, + emitted_diagnostics, + stashed_diagnostics, + future_breakage_diagnostics, + check_unstable_expect_diagnostics, + unstable_expect_diagnostics, + fulfilled_expectations, + ice_file: _, + } = inner.deref_mut(); + + // For the `Vec`s and `HashMap`s, we overwrite with an empty container to free the + // underlying memory (which `clear` would not do). + *err_guars = Default::default(); + *lint_err_guars = Default::default(); + *delayed_bugs = Default::default(); + *stashed_err_count = 0; + *deduplicated_err_count = 0; + *deduplicated_warn_count = 0; + *must_produce_diag = false; + *has_printed = false; + *suppressed_expected_diag = false; + *taught_diagnostics = Default::default(); + *emitted_diagnostic_codes = Default::default(); + *emitted_diagnostics = Default::default(); + *stashed_diagnostics = Default::default(); + *future_breakage_diagnostics = Default::default(); + *check_unstable_expect_diagnostics = false; + *unstable_expect_diagnostics = Default::default(); + *fulfilled_expectations = Default::default(); } /// Stash a given diagnostic with the given `Span` and [`StashKey`] as the key.