From 6a891ec4fe0db4790e9d000b15c5ec79907605b3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jul 2024 11:37:19 -0400 Subject: [PATCH 1/2] Enforce supertrait outlives obligations hold when confirming impl --- .../src/solve/trait_goals.rs | 13 +++++++ .../src/traits/select/mod.rs | 34 +++++++++++++++++-- compiler/rustc_type_ir/src/inherent.rs | 2 ++ ...implied-bounds-unnorm-associated-type-5.rs | 1 + ...ied-bounds-unnorm-associated-type-5.stderr | 17 ++++++++-- tests/ui/static/static-lifetime.rs | 1 + tests/ui/static/static-lifetime.stderr | 30 ++++++++++++++-- .../wf-in-where-clause-static.current.stderr | 12 +++++++ .../wf/wf-in-where-clause-static.next.stderr | 12 +++++++ tests/ui/wf/wf-in-where-clause-static.rs | 10 +++--- 10 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 tests/ui/wf/wf-in-where-clause-static.current.stderr create mode 100644 tests/ui/wf/wf-in-where-clause-static.next.stderr diff --git a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs index 4474bbc235198..77ce81e44a0b0 100644 --- a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs +++ b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs @@ -87,6 +87,19 @@ where .map(|pred| goal.with(cx, pred)); ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds); + // We currently elaborate all supertrait obligations from impls. This + // can be removed when we actually do coinduction correctly and just + // register that the impl header must be WF. + let goal_clause: I::Clause = goal.predicate.upcast(cx); + for clause in elaborate::elaborate(cx, [goal_clause]) { + if matches!( + clause.kind().skip_binder(), + ty::ClauseKind::TypeOutlives(..) | ty::ClauseKind::RegionOutlives(..) + ) { + ecx.add_goal(GoalSource::Misc, goal.with(cx, clause)); + } + } + ecx.evaluate_added_goals_and_make_canonical_response(maximal_certainty) }) } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index cc9174d3aad52..2bc2a8fe693c5 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -7,6 +7,7 @@ use std::fmt::{self, Display}; use std::ops::ControlFlow; use std::{cmp, iter}; +use hir::def::DefKind; use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{Diag, EmissionGuarantee}; @@ -14,8 +15,9 @@ use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::LangItem; use rustc_infer::infer::relate::TypeRelation; -use rustc_infer::infer::BoundRegionConversionTime::HigherRankedType; -use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes}; +use rustc_infer::infer::BoundRegionConversionTime::{self, HigherRankedType}; +use rustc_infer::infer::DefineOpaqueTypes; +use rustc_infer::traits::util::elaborate; use rustc_infer::traits::TraitObligation; use rustc_middle::bug; use rustc_middle::dep_graph::{dep_kinds, DepNodeIndex}; @@ -2798,6 +2800,34 @@ impl<'tcx> SelectionContext<'_, 'tcx> { }); } + if matches!(self.tcx().def_kind(def_id), DefKind::Impl { of_trait: true }) + && let Some(header) = self.tcx().impl_trait_header(def_id) + { + let trait_clause: ty::Clause<'tcx> = + header.trait_ref.instantiate(self.tcx(), args).upcast(self.tcx()); + for clause in elaborate(self.tcx(), [trait_clause]) { + if matches!( + clause.kind().skip_binder(), + ty::ClauseKind::TypeOutlives(..) | ty::ClauseKind::RegionOutlives(..) + ) { + let clause = normalize_with_depth_to( + self, + param_env, + cause.clone(), + recursion_depth, + clause, + &mut obligations, + ); + obligations.push(Obligation { + cause: cause.clone(), + recursion_depth, + param_env, + predicate: clause.as_predicate(), + }); + } + } + } + obligations } } diff --git a/compiler/rustc_type_ir/src/inherent.rs b/compiler/rustc_type_ir/src/inherent.rs index e1a3764fa76b4..263ba676427c5 100644 --- a/compiler/rustc_type_ir/src/inherent.rs +++ b/compiler/rustc_type_ir/src/inherent.rs @@ -451,6 +451,8 @@ pub trait Clause>: + UpcastFrom>> + UpcastFrom> + UpcastFrom>> + + UpcastFrom> + + UpcastFrom>> + UpcastFrom> + UpcastFrom>> + IntoKind>> diff --git a/tests/ui/fn/implied-bounds-unnorm-associated-type-5.rs b/tests/ui/fn/implied-bounds-unnorm-associated-type-5.rs index a7489f6fbaff4..38be3250c7d66 100644 --- a/tests/ui/fn/implied-bounds-unnorm-associated-type-5.rs +++ b/tests/ui/fn/implied-bounds-unnorm-associated-type-5.rs @@ -5,6 +5,7 @@ trait Trait<'a>: 'a { // if the `T: 'a` bound gets implied we would probably get ub here again impl<'a, T> Trait<'a> for T { //~^ ERROR the parameter type `T` may not live long enough + //~| ERROR the parameter type `T` may not live long enough type Type = (); } diff --git a/tests/ui/fn/implied-bounds-unnorm-associated-type-5.stderr b/tests/ui/fn/implied-bounds-unnorm-associated-type-5.stderr index b898df0835c2a..b2aa1abbbdb6e 100644 --- a/tests/ui/fn/implied-bounds-unnorm-associated-type-5.stderr +++ b/tests/ui/fn/implied-bounds-unnorm-associated-type-5.stderr @@ -16,8 +16,21 @@ help: consider adding an explicit lifetime bound LL | impl<'a, T: 'a> Trait<'a> for T { | ++++ +error[E0309]: the parameter type `T` may not live long enough + --> $DIR/implied-bounds-unnorm-associated-type-5.rs:6:27 + | +LL | impl<'a, T> Trait<'a> for T { + | -- ^ ...so that the type `T` will meet its required lifetime bounds + | | + | the parameter type `T` must be valid for the lifetime `'a` as defined here... + | +help: consider adding an explicit lifetime bound + | +LL | impl<'a, T: 'a> Trait<'a> for T { + | ++++ + error[E0505]: cannot move out of `x` because it is borrowed - --> $DIR/implied-bounds-unnorm-associated-type-5.rs:21:10 + --> $DIR/implied-bounds-unnorm-associated-type-5.rs:22:10 | LL | let x = String::from("Hello World!"); | - binding `x` declared here @@ -33,7 +46,7 @@ help: consider cloning the value if the performance cost is acceptable LL | let y = f(&x.clone(), ()); | ++++++++ -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors Some errors have detailed explanations: E0309, E0505. For more information about an error, try `rustc --explain E0309`. diff --git a/tests/ui/static/static-lifetime.rs b/tests/ui/static/static-lifetime.rs index ce1eeb6105f09..a861a2ffedab4 100644 --- a/tests/ui/static/static-lifetime.rs +++ b/tests/ui/static/static-lifetime.rs @@ -1,6 +1,7 @@ pub trait Arbitrary: Sized + 'static {} impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {} //~ ERROR lifetime bound +//~^ ERROR cannot infer an appropriate lifetime for lifetime parameter `'a` fn main() { } diff --git a/tests/ui/static/static-lifetime.stderr b/tests/ui/static/static-lifetime.stderr index 8c9434ce3cb0b..7a956dbfeef6e 100644 --- a/tests/ui/static/static-lifetime.stderr +++ b/tests/ui/static/static-lifetime.stderr @@ -11,6 +11,32 @@ LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {} | ^^ = note: but lifetime parameter must outlive the static lifetime -error: aborting due to 1 previous error +error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` due to conflicting requirements + --> $DIR/static-lifetime.rs:3:34 + | +LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first, the lifetime cannot outlive the lifetime `'a` as defined here... + --> $DIR/static-lifetime.rs:3:6 + | +LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {} + | ^^ +note: ...so that the types are compatible + --> $DIR/static-lifetime.rs:3:34 + | +LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: expected ` as Arbitrary>` + found ` as Arbitrary>` + = note: but, the lifetime must be valid for the static lifetime... +note: ...so that the declared lifetime parameter bounds are satisfied + --> $DIR/static-lifetime.rs:3:34 + | +LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0478`. +Some errors have detailed explanations: E0478, E0495. +For more information about an error, try `rustc --explain E0478`. diff --git a/tests/ui/wf/wf-in-where-clause-static.current.stderr b/tests/ui/wf/wf-in-where-clause-static.current.stderr new file mode 100644 index 0000000000000..d0bb89884c68a --- /dev/null +++ b/tests/ui/wf/wf-in-where-clause-static.current.stderr @@ -0,0 +1,12 @@ +error[E0716]: temporary value dropped while borrowed + --> $DIR/wf-in-where-clause-static.rs:18:18 + | +LL | let s = foo(&String::from("blah blah blah")); + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-- temporary value is freed at the end of this statement + | | | + | | creates a temporary value which is freed while still in use + | argument requires that borrow lasts for `'static` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0716`. diff --git a/tests/ui/wf/wf-in-where-clause-static.next.stderr b/tests/ui/wf/wf-in-where-clause-static.next.stderr new file mode 100644 index 0000000000000..d0bb89884c68a --- /dev/null +++ b/tests/ui/wf/wf-in-where-clause-static.next.stderr @@ -0,0 +1,12 @@ +error[E0716]: temporary value dropped while borrowed + --> $DIR/wf-in-where-clause-static.rs:18:18 + | +LL | let s = foo(&String::from("blah blah blah")); + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-- temporary value is freed at the end of this statement + | | | + | | creates a temporary value which is freed while still in use + | argument requires that borrow lasts for `'static` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0716`. diff --git a/tests/ui/wf/wf-in-where-clause-static.rs b/tests/ui/wf/wf-in-where-clause-static.rs index a3d360e1fb56b..8ee654ef7cff4 100644 --- a/tests/ui/wf/wf-in-where-clause-static.rs +++ b/tests/ui/wf/wf-in-where-clause-static.rs @@ -1,9 +1,6 @@ -//@ check-pass -//@ known-bug: #98117 - -// Should fail. Functions are responsible for checking the well-formedness of -// their own where clauses, so this should fail and require an explicit bound -// `T: 'static`. +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver use std::fmt::Display; @@ -19,5 +16,6 @@ where fn main() { let s = foo(&String::from("blah blah blah")); + //~^ ERROR temporary value dropped while borrowed println!("{}", s); } From fa9ae7b9d3e78a6605199efe358c0f1383d2f166 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jul 2024 12:08:57 -0400 Subject: [PATCH 2/2] Elaborate supertraits in dyn candidates --- .../src/solve/assembly/structural_traits.rs | 14 +++++++++++--- .../src/solve/trait_goals.rs | 6 +++--- .../rustc_trait_selection/src/traits/select/mod.rs | 1 + 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs b/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs index 60beaa0df84ca..e69d8d84d7d89 100644 --- a/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs +++ b/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs @@ -7,7 +7,7 @@ use rustc_type_ir::data_structures::HashMap; use rustc_type_ir::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable}; use rustc_type_ir::inherent::*; use rustc_type_ir::lang_items::TraitSolverLangItem; -use rustc_type_ir::{self as ty, Interner, Upcast as _}; +use rustc_type_ir::{self as ty, elaborate, Interner, Upcast as _}; use rustc_type_ir_macros::{TypeFoldable_Generic, TypeVisitable_Generic}; use tracing::instrument; @@ -671,11 +671,19 @@ where { let cx = ecx.cx(); let mut requirements = vec![]; - requirements.extend( + // Elaborating all supertrait outlives obligations here is not soundness critical, + // since if we just used the unelaborated set, then the transitive supertraits would + // be reachable when proving the former. However, since we elaborate all supertrait + // outlives obligations when confirming impls, we would end up with a different set + // of outlives obligations here if we didn't do the same, leading to ambiguity. + // FIXME(-Znext-solver=coinductive): Adding supertraits here can be removed once we + // make impls coinductive always, since they'll always need to prove their supertraits. + requirements.extend(elaborate::elaborate( + cx, cx.explicit_super_predicates_of(trait_ref.def_id) .iter_instantiated(cx, trait_ref.args) .map(|(pred, _)| pred), - ); + )); // FIXME(associated_const_equality): Also add associated consts to // the requirements here. diff --git a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs index 77ce81e44a0b0..b1dba712f797a 100644 --- a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs +++ b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs @@ -87,9 +87,9 @@ where .map(|pred| goal.with(cx, pred)); ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds); - // We currently elaborate all supertrait obligations from impls. This - // can be removed when we actually do coinduction correctly and just - // register that the impl header must be WF. + // We currently elaborate all supertrait outlives obligations from impls. + // This can be removed when we actually do coinduction correctly, and prove + // all supertrait obligations unconditionally. let goal_clause: I::Clause = goal.predicate.upcast(cx); for clause in elaborate::elaborate(cx, [goal_clause]) { if matches!( diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 2bc2a8fe693c5..1d9a90f0300ab 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -2800,6 +2800,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> { }); } + // Register any outlives obligations from the trait here, cc #124336. if matches!(self.tcx().def_kind(def_id), DefKind::Impl { of_trait: true }) && let Some(header) = self.tcx().impl_trait_header(def_id) {