Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce supertrait outlives obligations hold when confirming impl #124336

Merged
merged 2 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Copy link
Contributor

@lcnr lcnr Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment why we elaborate here:

  • it's not soundness critical
  • however, we currently elaborate the super predicates for user-defined impls which destructures outlives requirements. So for the builtin and user-defined impl of dyn Any: Any to have the same region constraints (to enable us to merge them), we also elaborate to get all outlives constraints here
  • once we prove all super predicates for user-defined impls as part of the new coinductive trait semantics, this elaborate call can be removed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still relevant

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.
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,19 @@ where
.map(|pred| goal.with(cx, pred));
ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds);

// 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!(
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)
})
}
Expand Down
35 changes: 33 additions & 2 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@ 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};
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};
Expand Down Expand Up @@ -2798,6 +2800,35 @@ 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 })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if matches!(self.tcx().def_kind(def_id), DefKind::Impl { of_trait: true })
// 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)
{
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
}
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_type_ir/src/inherent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,8 @@ pub trait Clause<I: Interner<Clause = Self>>:
+ UpcastFrom<I, ty::Binder<I, ty::ClauseKind<I>>>
+ UpcastFrom<I, ty::TraitRef<I>>
+ UpcastFrom<I, ty::Binder<I, ty::TraitRef<I>>>
+ UpcastFrom<I, ty::TraitPredicate<I>>
+ UpcastFrom<I, ty::Binder<I, ty::TraitPredicate<I>>>
+ UpcastFrom<I, ty::ProjectionPredicate<I>>
+ UpcastFrom<I, ty::Binder<I, ty::ProjectionPredicate<I>>>
+ IntoKind<Kind = ty::Binder<I, ty::ClauseKind<I>>>
Expand Down
1 change: 1 addition & 0 deletions tests/ui/fn/implied-bounds-unnorm-associated-type-5.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ();
}

Expand Down
17 changes: 15 additions & 2 deletions tests/ui/fn/implied-bounds-unnorm-associated-type-5.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`.
1 change: 1 addition & 0 deletions tests/ui/static/static-lifetime.rs
Original file line number Diff line number Diff line change
@@ -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() {
}
30 changes: 28 additions & 2 deletions tests/ui/static/static-lifetime.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<Cow<'a, A> as Arbitrary>`
found `<Cow<'_, A> 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`.
12 changes: 12 additions & 0 deletions tests/ui/wf/wf-in-where-clause-static.current.stderr
Original file line number Diff line number Diff line change
@@ -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`.
12 changes: 12 additions & 0 deletions tests/ui/wf/wf-in-where-clause-static.next.stderr
Original file line number Diff line number Diff line change
@@ -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`.
10 changes: 4 additions & 6 deletions tests/ui/wf/wf-in-where-clause-static.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -19,5 +16,6 @@ where

fn main() {
let s = foo(&String::from("blah blah blah"));
//~^ ERROR temporary value dropped while borrowed
println!("{}", s);
}
Loading