Skip to content

Commit

Permalink
Auto merge of #86866 - nikomatsakis:issue-84841, r=oli-obk
Browse files Browse the repository at this point in the history
Hack: Ignore inference variables in certain queries

Fixes #84841
Fixes #86753

Some queries are not built to accept types with inference variables, which can lead to ICEs. These queries probably ought to be converted to canonical form, but as a quick workaround, we can return conservative results in the case that inference variables are found.

We should file a follow-up issue (and update the FIXMEs...) to do the proper refactoring.

cc `@arora-aman`

r? `@oli-obk`
  • Loading branch information
bors committed Jul 4, 2021
2 parents 9044245 + 492ba34 commit 23c652d
Show file tree
Hide file tree
Showing 13 changed files with 168 additions and 58 deletions.
29 changes: 11 additions & 18 deletions compiler/rustc_infer/src/infer/canonical/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
{
self.tcx.sess.perf_stats.queries_canonicalized.fetch_add(1, Ordering::Relaxed);

Canonicalizer::canonicalize(
value,
Some(self),
self.tcx,
&CanonicalizeAllFreeRegions,
query_state,
)
Canonicalizer::canonicalize(value, self, self.tcx, &CanonicalizeAllFreeRegions, query_state)
}

/// Canonicalizes a query *response* `V`. When we canonicalize a
Expand Down Expand Up @@ -87,7 +81,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
let mut query_state = OriginalQueryValues::default();
Canonicalizer::canonicalize(
value,
Some(self),
self,
self.tcx,
&CanonicalizeQueryResponse,
&mut query_state,
Expand All @@ -101,7 +95,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
let mut query_state = OriginalQueryValues::default();
Canonicalizer::canonicalize(
value,
Some(self),
self,
self.tcx,
&CanonicalizeUserTypeAnnotation,
&mut query_state,
Expand Down Expand Up @@ -133,7 +127,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {

Canonicalizer::canonicalize(
value,
Some(self),
self,
self.tcx,
&CanonicalizeFreeRegionsOtherThanStatic,
query_state,
Expand Down Expand Up @@ -275,7 +269,7 @@ impl CanonicalizeRegionMode for CanonicalizeFreeRegionsOtherThanStatic {
}

struct Canonicalizer<'cx, 'tcx> {
infcx: Option<&'cx InferCtxt<'cx, 'tcx>>,
infcx: &'cx InferCtxt<'cx, 'tcx>,
tcx: TyCtxt<'tcx>,
variables: SmallVec<[CanonicalVarInfo<'tcx>; 8]>,
query_state: &'cx mut OriginalQueryValues<'tcx>,
Expand Down Expand Up @@ -316,7 +310,6 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Canonicalizer<'cx, 'tcx> {
ty::ReVar(vid) => {
let resolved_vid = self
.infcx
.unwrap()
.inner
.borrow_mut()
.unwrap_region_constraints()
Expand All @@ -343,7 +336,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Canonicalizer<'cx, 'tcx> {
match *t.kind() {
ty::Infer(ty::TyVar(vid)) => {
debug!("canonical: type var found with vid {:?}", vid);
match self.infcx.unwrap().probe_ty_var(vid) {
match self.infcx.probe_ty_var(vid) {
// `t` could be a float / int variable; canonicalize that instead.
Ok(t) => {
debug!("(resolved to {:?})", t);
Expand Down Expand Up @@ -429,7 +422,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Canonicalizer<'cx, 'tcx> {
match ct.val {
ty::ConstKind::Infer(InferConst::Var(vid)) => {
debug!("canonical: const var found with vid {:?}", vid);
match self.infcx.unwrap().probe_const_var(vid) {
match self.infcx.probe_const_var(vid) {
Ok(c) => {
debug!("(resolved to {:?})", c);
return self.fold_const(c);
Expand Down Expand Up @@ -476,7 +469,7 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {
/// `canonicalize_query` and `canonicalize_response`.
fn canonicalize<V>(
value: V,
infcx: Option<&InferCtxt<'_, 'tcx>>,
infcx: &InferCtxt<'_, 'tcx>,
tcx: TyCtxt<'tcx>,
canonicalize_region_mode: &dyn CanonicalizeRegionMode,
query_state: &mut OriginalQueryValues<'tcx>,
Expand Down Expand Up @@ -610,7 +603,7 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {

/// Returns the universe in which `vid` is defined.
fn region_var_universe(&self, vid: ty::RegionVid) -> ty::UniverseIndex {
self.infcx.unwrap().inner.borrow_mut().unwrap_region_constraints().var_universe(vid)
self.infcx.inner.borrow_mut().unwrap_region_constraints().var_universe(vid)
}

/// Creates a canonical variable (with the given `info`)
Expand All @@ -631,7 +624,7 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {
/// *that*. Otherwise, create a new canonical variable for
/// `ty_var`.
fn canonicalize_ty_var(&mut self, info: CanonicalVarInfo<'tcx>, ty_var: Ty<'tcx>) -> Ty<'tcx> {
let infcx = self.infcx.expect("encountered ty-var without infcx");
let infcx = self.infcx;
let bound_to = infcx.shallow_resolve(ty_var);
if bound_to != ty_var {
self.fold_ty(bound_to)
Expand All @@ -650,7 +643,7 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {
info: CanonicalVarInfo<'tcx>,
const_var: &'tcx ty::Const<'tcx>,
) -> &'tcx ty::Const<'tcx> {
let infcx = self.infcx.expect("encountered const-var without infcx");
let infcx = self.infcx;
let bound_to = infcx.shallow_resolve(const_var);
if bound_to != const_var {
self.fold_const(bound_to)
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1559,9 +1559,22 @@ rustc_queries! {
desc { "evaluating trait selection obligation `{}`", goal.value }
}

/// Evaluates whether the given type implements the given trait
/// in the given environment.
///
/// The inputs are:
///
/// - the def-id of the trait
/// - the self type
/// - the *other* type parameters of the trait, excluding the self-type
/// - the parameter environment
///
/// FIXME. If the type, trait, or environment has inference variables,
/// this yields `EvaluatedToUnknown`. It should be refactored
/// to use canonicalization, really.
query type_implements_trait(
key: (DefId, Ty<'tcx>, SubstsRef<'tcx>, ty::ParamEnv<'tcx>, )
) -> bool {
) -> traits::EvaluationResult {
desc { "evaluating `type_implements_trait` `{:?}`", key }
}

Expand Down
17 changes: 13 additions & 4 deletions compiler/rustc_middle/src/ty/normalize_erasing_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,32 @@ struct NormalizeAfterErasingRegionsFolder<'tcx> {
param_env: ty::ParamEnv<'tcx>,
}

impl<'tcx> NormalizeAfterErasingRegionsFolder<'tcx> {
fn normalize_generic_arg_after_erasing_regions(
&self,
arg: ty::GenericArg<'tcx>,
) -> ty::GenericArg<'tcx> {
let arg = self.param_env.and(arg);
self.tcx.normalize_generic_arg_after_erasing_regions(arg)
}
}

impl TypeFolder<'tcx> for NormalizeAfterErasingRegionsFolder<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
let arg = self.param_env.and(ty.into());
self.tcx.normalize_generic_arg_after_erasing_regions(arg).expect_ty()
self.normalize_generic_arg_after_erasing_regions(ty.into()).expect_ty()
}

fn fold_const(&mut self, c: &'tcx ty::Const<'tcx>) -> &'tcx ty::Const<'tcx> {
let arg = self.param_env.and(c.into());
self.tcx.normalize_generic_arg_after_erasing_regions(arg).expect_const()
self.normalize_generic_arg_after_erasing_regions(c.into()).expect_const()
}

#[inline]
fn fold_mir_const(&mut self, c: mir::ConstantKind<'tcx>) -> mir::ConstantKind<'tcx> {
// FIXME: This *probably* needs canonicalization too!
let arg = self.param_env.and(c);
self.tcx.normalize_mir_const_after_erasing_regions(arg)
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,15 @@ impl<'tcx> ty::TyS<'tcx> {
[component_ty] => component_ty,
_ => self,
};

// FIXME(#86868): We should be canonicalizing, or else moving this to a method of inference
// context, or *something* like that, but for now just avoid passing inference
// variables to queries that can't cope with them. Instead, conservatively
// return "true" (may change drop order).
if query_ty.needs_infer() {
return true;
}

// This doesn't depend on regions, so try to minimize distinct
// query keys used.
let erased = tcx.normalize_erasing_regions(param_env, query_ty);
Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_middle::mir::{
FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef,
ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, VarBindingForm,
};
use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TypeFoldable};
use rustc_middle::ty::{self, suggest_constraining_type_param, Ty};
use rustc_span::source_map::DesugaringKind;
use rustc_span::symbol::sym;
use rustc_span::{Span, DUMMY_SP};
Expand Down Expand Up @@ -1329,18 +1329,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let return_ty = tcx.erase_regions(return_ty);

// to avoid panics
if !return_ty.has_infer_types() {
if let Some(iter_trait) = tcx.get_diagnostic_item(sym::Iterator) {
if tcx.type_implements_trait((iter_trait, return_ty, ty_params, self.param_env))
{
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(return_span) {
err.span_suggestion_hidden(
return_span,
"use `.collect()` to allocate the iterator",
format!("{}{}", snippet, ".collect::<Vec<_>>()"),
Applicability::MaybeIncorrect,
);
}
if let Some(iter_trait) = tcx.get_diagnostic_item(sym::Iterator) {
if tcx
.type_implements_trait((iter_trait, return_ty, ty_params, self.param_env))
.must_apply_modulo_regions()
{
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(return_span) {
err.span_suggestion_hidden(
return_span,
"use `.collect()` to allocate the iterator",
format!("{}{}", snippet, ".collect::<Vec<_>>()"),
Applicability::MaybeIncorrect,
);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2396,7 +2396,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
normalized_ty,
);
debug!("suggest_await_before_try: try_trait_obligation {:?}", try_obligation);
if self.predicate_may_hold(&try_obligation) && impls_future {
if self.predicate_may_hold(&try_obligation)
&& impls_future.must_apply_modulo_regions()
{
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
if snippet.ends_with('?') {
err.span_suggestion_verbose(
Expand Down
16 changes: 12 additions & 4 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,7 @@ fn vtable_trait_first_method_offset<'tcx>(
}

/// Check whether a `ty` implements given trait(trait_def_id).
///
/// NOTE: Always return `false` for a type which needs inference.
/// See query definition for details.
fn type_implements_trait<'tcx>(
tcx: TyCtxt<'tcx>,
key: (
Expand All @@ -552,7 +551,7 @@ fn type_implements_trait<'tcx>(
SubstsRef<'tcx>,
ParamEnv<'tcx>,
),
) -> bool {
) -> EvaluationResult {
let (trait_def_id, ty, params, param_env) = key;

debug!(
Expand All @@ -562,13 +561,22 @@ fn type_implements_trait<'tcx>(

let trait_ref = ty::TraitRef { def_id: trait_def_id, substs: tcx.mk_substs_trait(ty, params) };

// FIXME(#86868): If there are inference variables anywhere, just give up and assume
// we don't know the answer. This works around the ICEs that would result from
// using those inference variables within the `infer_ctxt` we create below.
// Really we should be using canonicalized variables, or perhaps removing
// this query altogether.
if (trait_ref, param_env).needs_infer() {
return EvaluationResult::EvaluatedToUnknown;
}

let obligation = Obligation {
cause: ObligationCause::dummy(),
param_env,
recursion_depth: 0,
predicate: trait_ref.without_const().to_predicate(tcx),
};
tcx.infer_ctxt().enter(|infcx| infcx.predicate_must_hold_modulo_regions(&obligation))
tcx.infer_ctxt().enter(|infcx| infcx.evaluate_obligation_no_overflow(&obligation))
}

pub fn provide(providers: &mut ty::query::Providers) {
Expand Down
14 changes: 4 additions & 10 deletions compiler/rustc_typeck/src/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,16 +440,10 @@ impl<'a, 'tcx> CastCheck<'tcx> {
let expr_ty = fcx.resolve_vars_if_possible(self.expr_ty);
let expr_ty = fcx.tcx.erase_regions(expr_ty);
let ty_params = fcx.tcx.mk_substs_trait(expr_ty, &[]);
// Check for infer types because cases like `Option<{integer}>` would
// panic otherwise.
if !expr_ty.has_infer_types()
&& !ty.has_infer_types()
&& fcx.tcx.type_implements_trait((
from_trait,
ty,
ty_params,
fcx.param_env,
))
if fcx
.tcx
.type_implements_trait((from_trait, ty, ty_params, fcx.param_env))
.must_apply_modulo_regions()
{
label = false;
err.span_suggestion(
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_typeck/src/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -961,12 +961,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let is_drop_defined_for_ty = |ty: Ty<'tcx>| {
let drop_trait = self.tcx.require_lang_item(hir::LangItem::Drop, Some(closure_span));
let ty_params = self.tcx.mk_substs_trait(base_path_ty, &[]);
self.tcx.type_implements_trait((
drop_trait,
ty,
ty_params,
self.tcx.param_env(closure_def_id.expect_local()),
))
self.tcx
.type_implements_trait((
drop_trait,
ty,
ty_params,
self.tcx.param_env(closure_def_id.expect_local()),
))
.must_apply_modulo_regions()
};

let is_drop_defined_for_ty = is_drop_defined_for_ty(base_path_ty);
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/async-await/issue-84841.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// edition:2018

fn main() {

}

async fn foo() {
// Adding an .await here avoids the ICE
test()?;
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| ERROR the `?` operator can only be used in an async function that returns
}

// Removing the const generic parameter here avoids the ICE
async fn test<const N: usize>() {
}
28 changes: 28 additions & 0 deletions src/test/ui/async-await/issue-84841.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/issue-84841.rs:9:5
|
LL | test()?;
| ^^^^^^^ the `?` operator cannot be applied to type `impl Future`
|
= help: the trait `Try` is not implemented for `impl Future`
= note: required by `branch`

error[E0277]: the `?` operator can only be used in an async function that returns `Result` or `Option` (or another type that implements `FromResidual`)
--> $DIR/issue-84841.rs:9:11
|
LL | async fn foo() {
| ________________-
LL | | // Adding an .await here avoids the ICE
LL | | test()?;
| | ^ cannot use the `?` operator in an async function that returns `()`
LL | |
LL | |
LL | | }
| |_- this function should return `Result` or `Option` to accept `?`
|
= help: the trait `FromResidual<_>` is not implemented for `()`
= note: required by `from_residual`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
Loading

0 comments on commit 23c652d

Please sign in to comment.