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

ignore implied bounds with placeholders #112422

Merged
merged 1 commit into from
Nov 17, 2023
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_infer::infer::region_constraints::GenericKind;
use rustc_infer::infer::InferCtxt;
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::traits::query::OutlivesBound;
use rustc_middle::ty::{self, RegionVid, Ty};
use rustc_middle::ty::{self, RegionVid, Ty, TypeVisitableExt};
use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP};
use rustc_trait_selection::traits::query::type_op::{self, TypeOp};
use std::rc::Rc;
Expand Down Expand Up @@ -321,6 +321,9 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
.map_err(|_: ErrorGuaranteed| debug!("failed to compute implied bounds {:?}", ty))
.ok()?;
debug!(?bounds, ?constraints);
// Because of #109628, we may have unexpected placeholders. Ignore them!
// FIXME(#109628): panic in this case once the issue is fixed.
let bounds = bounds.into_iter().filter(|bound| !bound.has_placeholders());
Comment on lines +324 to +326
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move that into the TypeOp?

Copy link
Member Author

Choose a reason for hiding this comment

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

Moving this into the TypeOp has no benefit as we would have to duplicate the logic in QueryTypeOp::{perform_query,perform_locally_in_new_solver} as well. It is also more difficult to handle canonicalized query responses - there is no easy way to detect placeholders for example.

Doing it in the query itself is difficult as we need to do OpportunisticRegionResolution before checking for placeholders. Using TypeVisitableExt::has_placeholders to check for erroneus placeholders there does not play well with #109388 as it would yield many false positives.

self.add_outlives_bounds(bounds);
constraints
}
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_trait_selection/src/traits/outlives_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
};

let mut constraints = QueryRegionConstraints::default();
let Ok(InferOk { value, obligations }) = self
let Ok(InferOk { value: mut bounds, obligations }) = self
.instantiate_nll_query_response_and_region_obligations(
&ObligationCause::dummy(),
param_env,
Expand All @@ -85,6 +85,10 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
};
assert_eq!(&obligations, &[]);

// Because of #109628, we may have unexpected placeholders. Ignore them!
// FIXME(#109628): panic in this case once the issue is fixed.
bounds.retain(|bound| !bound.has_placeholders());

if !constraints.is_empty() {
let span = self.tcx.def_span(body_id);

Expand Down Expand Up @@ -114,7 +118,7 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
}
};

value
bounds
}

fn implied_bounds_tys(
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/implied-bounds/normalization-placeholder-leak.fail.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0477]: the type `&'lt u8` does not fulfill the required lifetime
--> $DIR/normalization-placeholder-leak.rs:31:40
|
LL | fn test_lifetime<'lt, T: Trait>(_: Foo<&'lt u8>) {}
| ^^^^^^^^^^^^

error[E0477]: the type `<T as AnotherTrait>::Ty2<'lt>` does not fulfill the required lifetime
--> $DIR/normalization-placeholder-leak.rs:36:44
|
LL | fn test_alias<'lt, T: AnotherTrait>(_: Foo<T::Ty2::<'lt>>) {}
| ^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0477`.
56 changes: 56 additions & 0 deletions tests/ui/implied-bounds/normalization-placeholder-leak.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Because of #109628, when we compute the implied bounds from `Foo<X>`,
// we incorrectly get `X: placeholder('x)`.
// Make sure we ignore these bogus bounds and not use them for anything useful.
//
// revisions: fail pass
// [fail] check-fail
// [pass] check-pass

trait Trait {
type Ty<'a> where Self: 'a;
}

impl<T> Trait for T {
type Ty<'a> = () where Self: 'a;
}

struct Foo<T: Trait>(T)
where
for<'x> T::Ty<'x>: Sized;

trait AnotherTrait {
type Ty2<'a>: 'a;
}

#[cfg(fail)]
mod fail {
use super::*;

// implied_bound: `'lt: placeholder('x)`.
// don't use the bound to prove `'lt: 'static`.
fn test_lifetime<'lt, T: Trait>(_: Foo<&'lt u8>) {}
//[fail]~^ ERROR `&'lt u8` does not fulfill the required lifetime

// implied bound: `T::Ty2<'lt>: placeholder('x)`.
// don't use the bound to prove `T::Ty2<'lt>: 'static`.
fn test_alias<'lt, T: AnotherTrait>(_: Foo<T::Ty2::<'lt>>) {}
//[fail]~^ ERROR `<T as AnotherTrait>::Ty2<'lt>` does not fulfill the required lifetime
}


mod pass {
use super::*;

// implied_bound: 'static: placeholder('x).
// don't ice.
fn test_lifetime<T: Trait>(_: Foo<&'static u8>) {}

// implied bound: T::Ty2<'static>: placeholder('x).
// don't add the bound to the environment,
// otherwise we would fail to infer a value for `'_`.
fn test_alias<T: AnotherTrait>(_: Foo<T::Ty2::<'static>>) {
None::<&'static T::Ty2<'_>>;
}
}

fn main() {}
Loading