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

unify query canonicalization mode #118968

Merged
merged 1 commit into from
Jan 9, 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
83 changes: 27 additions & 56 deletions compiler/rustc_infer/src/infer/canonical/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,33 @@ impl<'tcx> InferCtxt<'tcx> {
where
V: TypeFoldable<TyCtxt<'tcx>>,
{
self.canonicalize_query_with_mode(value, query_state, &CanonicalizeAllFreeRegions)
let (param_env, value) = value.into_parts();
let param_env = self.tcx.canonical_param_env_cache.get_or_insert(
self.tcx,
param_env,
query_state,
|tcx, param_env, query_state| {
// FIXME(#118965): We don't canonicalize the static lifetimes that appear in the
// `param_env` beacause they are treated differently by trait selection.
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
// `param_env` beacause they are treated differently by trait selection.
// `param_env` because they are treated differently by trait selection.

Canonicalizer::canonicalize(
param_env,
None,
tcx,
&CanonicalizeFreeRegionsOtherThanStatic,
query_state,
)
},
);

Canonicalizer::canonicalize_with_base(
param_env,
value,
Some(self),
self.tcx,
&CanonicalizeAllFreeRegions,
query_state,
)
.unchecked_map(|(param_env, value)| param_env.and(value))
}

/// Canonicalizes a query *response* `V`. When we canonicalize a
Expand Down Expand Up @@ -96,61 +122,6 @@ impl<'tcx> InferCtxt<'tcx> {
&mut query_state,
)
}

/// A variant of `canonicalize_query` that does not
/// canonicalize `'static`. This is useful when
/// the query implementation can perform more efficient
/// handling of `'static` regions (e.g. trait evaluation).
pub fn canonicalize_query_keep_static<V>(
&self,
value: ty::ParamEnvAnd<'tcx, V>,
query_state: &mut OriginalQueryValues<'tcx>,
) -> Canonical<'tcx, ty::ParamEnvAnd<'tcx, V>>
where
V: TypeFoldable<TyCtxt<'tcx>>,
{
self.canonicalize_query_with_mode(
value,
query_state,
&CanonicalizeFreeRegionsOtherThanStatic,
)
}

fn canonicalize_query_with_mode<V>(
&self,
value: ty::ParamEnvAnd<'tcx, V>,
query_state: &mut OriginalQueryValues<'tcx>,
canonicalize_region_mode: &dyn CanonicalizeMode,
) -> Canonical<'tcx, ty::ParamEnvAnd<'tcx, V>>
where
V: TypeFoldable<TyCtxt<'tcx>>,
{
let (param_env, value) = value.into_parts();
let base = self.tcx.canonical_param_env_cache.get_or_insert(
self.tcx,
param_env,
query_state,
|tcx, param_env, query_state| {
Canonicalizer::canonicalize(
param_env,
None,
tcx,
&CanonicalizeFreeRegionsOtherThanStatic,
query_state,
)
},
);

Canonicalizer::canonicalize_with_base(
base,
value,
Some(self),
self.tcx,
canonicalize_region_mode,
query_state,
)
.unchecked_map(|(param_env, value)| param_env.and(value))
}
}

/// Controls how we canonicalize "free regions" that are not inference
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_trait_selection/src/traits/outlives_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
assert!(!ty.has_non_region_infer());

let mut canonical_var_values = OriginalQueryValues::default();
let canonical_ty =
self.canonicalize_query_keep_static(param_env.and(ty), &mut canonical_var_values);
let canonical_ty = self.canonicalize_query(param_env.and(ty), &mut canonical_var_values);
let Ok(canonical_result) = self.tcx.implied_outlives_bounds(canonical_ty) else {
return vec![];
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,8 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
})
} else {
assert!(!self.intercrate);
let c_pred = self.canonicalize_query_keep_static(
param_env.and(obligation.predicate),
&mut _orig_values,
);
let c_pred =
self.canonicalize_query(param_env.and(obligation.predicate), &mut _orig_values);
self.tcx.at(obligation.cause.span()).evaluate_obligation(c_pred)
}
}
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_trait_selection/src/traits/query/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,7 @@ impl<'cx, 'tcx> FallibleTypeFolder<TyCtxt<'tcx>> for QueryNormalizer<'cx, 'tcx>
let data = data.try_fold_with(self)?;

let mut orig_values = OriginalQueryValues::default();
// HACK(matthewjasper) `'static` is special-cased in selection,
// so we cannot canonicalize it.
let c_data = infcx
.canonicalize_query_keep_static(self.param_env.and(data), &mut orig_values);
let c_data = infcx.canonicalize_query(self.param_env.and(data), &mut orig_values);
debug!("QueryNormalizer: c_data = {:#?}", c_data);
debug!("QueryNormalizer: orig_values = {:#?}", orig_values);
let result = match kind {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,9 @@ pub trait QueryTypeOp<'tcx>: fmt::Debug + Copy + TypeFoldable<TyCtxt<'tcx>> + 't
return Ok((result, None, vec![], Certainty::Proven));
}

// FIXME(#33684) -- We need to use
// `canonicalize_query_keep_static` here because of things
// like the subtype query, which go awry around
// `'static` otherwise.
let mut canonical_var_values = OriginalQueryValues::default();
let old_param_env = query_key.param_env;
let canonical_self =
infcx.canonicalize_query_keep_static(query_key, &mut canonical_var_values);
let canonical_self = infcx.canonicalize_query(query_key, &mut canonical_var_values);
let canonical_result = Self::perform_query(infcx.tcx, canonical_self)?;

let InferOk { value, obligations } = infcx
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: implementation of `Deserialize` is not general enough
LL | assert_deserialize_owned::<&'static str>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Deserialize` is not general enough
|
= note: `&'static str` must implement `Deserialize<'0>`, for any lifetime `'0`...
= note: `&str` must implement `Deserialize<'0>`, for any lifetime `'0`...
= note: ...but `&str` actually implements `Deserialize<'1>`, for some specific lifetime `'1`

error: aborting due to 1 previous error
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/higher-ranked/trait-bounds/issue-59311.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ error: higher-ranked lifetime error
LL | v.t(|| {});
| ^^^^^
|
= note: could not prove `for<'a> &'a V: 'static`
= note: could not prove `for<'a> &'a V: 'b`

error: aborting due to 2 previous errors

2 changes: 1 addition & 1 deletion tests/ui/nll/issue-54302.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: implementation of `Deserialize` is not general enough
LL | assert_deserialize_owned::<&'static str>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Deserialize` is not general enough
|
= note: `&'static str` must implement `Deserialize<'0>`, for any lifetime `'0`...
= note: `&str` must implement `Deserialize<'0>`, for any lifetime `'0`...
= note: ...but `&str` actually implements `Deserialize<'1>`, for some specific lifetime `'1`

error: aborting due to 1 previous error
Expand Down
Loading