Skip to content

Commit

Permalink
Utilize coercion information to guide never type fallback
Browse files Browse the repository at this point in the history
This doesn't yet fully work -- running coercions seems to be prone to causing
errors in the environment, which creates a little noise in some cases. It should
be enough to let us re-run crater, though, I think.
  • Loading branch information
Mark-Simulacrum committed Aug 30, 2021
1 parent 5a8bcf9 commit 40d1e69
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 66 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ use rustc_trait_selection::traits::error_reporting::report_object_safety_error;
#[derive(Debug)]
pub struct CastCheck<'tcx> {
expr: &'tcx hir::Expr<'tcx>,
expr_ty: Ty<'tcx>,
cast_ty: Ty<'tcx>,
pub(crate) expr_ty: Ty<'tcx>,
pub(crate) cast_ty: Ty<'tcx>,
cast_span: Span,
span: Span,
}
Expand Down
53 changes: 35 additions & 18 deletions compiler/rustc_typeck/src/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode};
use smallvec::{smallvec, SmallVec};
use std::ops::Deref;

struct Coerce<'a, 'tcx> {
pub(crate) struct Coerce<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
cause: ObligationCause<'tcx>,
use_lub: bool,
Expand Down Expand Up @@ -116,7 +116,7 @@ fn success<'tcx>(
}

impl<'f, 'tcx> Coerce<'f, 'tcx> {
fn new(
pub(crate) fn new(
fcx: &'f FnCtxt<'f, 'tcx>,
cause: ObligationCause<'tcx>,
allow_two_phase: AllowTwoPhase,
Expand Down Expand Up @@ -146,7 +146,15 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
.and_then(|InferOk { value: ty, obligations }| success(f(ty), ty, obligations))
}

pub(crate) fn coerce_silent(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> {
self.coerce_(false, a, b)
}

fn coerce(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> {
self.coerce_(true, a, b)
}

fn coerce_(&self, report_error: bool, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> {
// First, remove any resolved type variables (at the top level, at least):
let a = self.shallow_resolve(a);
let b = self.shallow_resolve(b);
Expand Down Expand Up @@ -174,7 +182,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
// NOTE: this is wrapped in a `commit_if_ok` because it creates
// a "spurious" type variable, and we don't want to have that
// type variable in memory if the coercion fails.
let unsize = self.commit_if_ok(|_| self.coerce_unsized(a, b));
let unsize = self.commit_if_ok(|_| self.coerce_unsized(report_error, a, b));
match unsize {
Ok(_) => {
debug!("coerce: unsize successful");
Expand Down Expand Up @@ -482,7 +490,12 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
// &[T; n] or &mut [T; n] -> &[T]
// or &mut [T; n] -> &mut [T]
// or &Concrete -> &Trait, etc.
fn coerce_unsized(&self, mut source: Ty<'tcx>, mut target: Ty<'tcx>) -> CoerceResult<'tcx> {
fn coerce_unsized(
&self,
report_error: bool,
mut source: Ty<'tcx>,
mut target: Ty<'tcx>,
) -> CoerceResult<'tcx> {
debug!("coerce_unsized(source={:?}, target={:?})", source, target);

source = self.shallow_resolve(source);
Expand Down Expand Up @@ -690,13 +703,15 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {

// Object safety violations or miscellaneous.
Err(err) => {
self.report_selection_error(
obligation.clone(),
&obligation,
&err,
false,
false,
);
if report_error {
self.report_selection_error(
obligation.clone(),
&obligation,
&err,
false,
false,
);
}
// Treat this like an obligation and follow through
// with the unsizing - the lack of a coercion should
// be silent, as it causes a type mismatch later.
Expand All @@ -707,13 +722,15 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
}

if has_unsized_tuple_coercion && !self.tcx.features().unsized_tuple_coercion {
feature_err(
&self.tcx.sess.parse_sess,
sym::unsized_tuple_coercion,
self.cause.span,
"unsized tuple coercion is not stable enough for use and is subject to change",
)
.emit();
if report_error {
feature_err(
&self.tcx.sess.parse_sess,
sym::unsized_tuple_coercion,
self.cause.span,
"unsized tuple coercion is not stable enough for use and is subject to change",
)
.emit();
}
}

if has_trait_upcasting_coercion && !self.tcx().features().trait_upcasting {
Expand Down
60 changes: 57 additions & 3 deletions compiler/rustc_typeck/src/check/fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use rustc_data_structures::{
graph::{iterate::DepthFirstSearch, vec_graph::VecGraph},
stable_set::FxHashSet,
};
use rustc_middle::ty::{self, Ty};
use rustc_infer::traits::ObligationCauseCode;
use rustc_middle::ty::{self, adjustment::AllowTwoPhase, Ty, TypeFoldable};

impl<'tcx> FnCtxt<'_, 'tcx> {
/// Performs type inference fallback, returning true if any fallback
Expand All @@ -16,6 +17,48 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
self.fulfillment_cx.borrow_mut().pending_obligations()
);

let mut cast_vars = Vec::new();

// FIXME: This seems to cause extra object safety errors. Not clear why; one would expect the probe and such to eat them.
for cast in self.deferred_cast_checks.borrow().iter() {
let source = cast.expr_ty;
let target = cast.cast_ty;
debug!("attempting coerce {:?} -> {:?}", source, target,);
let source = self.resolve_vars_with_obligations(source);

let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable);
// We don't ever need two-phase here since we throw out the result of the coercion
let coerce = crate::check::coercion::Coerce::new(self, cause, AllowTwoPhase::No);
if let Ok(infok) = self.probe(|_| coerce.coerce_silent(source, target)) {
for obligation in infok.obligations {
if let ty::PredicateKind::Projection(predicate) =
obligation.predicate.kind().skip_binder()
{
if !predicate.projection_ty.has_escaping_bound_vars() {
// FIXME: We really *should* do this even with escaping bound
// vars, but there's not much we can do here. In the worst case
// (if this ends up being important) we just don't register a relationship and then end up falling back to !,
// which is not terrible.

if let Some(vid) = self
.fulfillment_cx
.borrow_mut()
.normalize_projection_type(
&self.infcx,
obligation.param_env,
predicate.projection_ty,
obligation.cause.clone(),
)
.ty_vid()
{
cast_vars.push(vid);
}
}
}
}
}
}

// All type checking constraints were added, try to fallback unsolved variables.
self.select_obligations_where_possible(false, |_| {});

Expand All @@ -36,7 +79,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
}

let diverging_fallback =
self.calculate_diverging_fallback(&unsolved_variables, &relationships);
self.calculate_diverging_fallback(&unsolved_variables, &relationships, &cast_vars);

// We do fallback in two passes, to try to generate
// better error messages.
Expand Down Expand Up @@ -263,6 +306,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
&self,
unsolved_variables: &[Ty<'tcx>],
relationships: &FxHashMap<ty::TyVid, ty::FoundRelationships>,
cast_vars: &[ty::TyVid],
) -> FxHashMap<Ty<'tcx>, Ty<'tcx>> {
debug!("calculate_diverging_fallback({:?})", unsolved_variables);

Expand Down Expand Up @@ -368,16 +412,26 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
let mut relationship = ty::FoundRelationships { self_in_trait: false, output: false };

for (vid, rel) in relationships.iter() {
//if self.infcx.shallow_resolve(*ty).ty_vid().map(|t| self.infcx.root_var(t))
if self.infcx.root_var(*vid) == root_vid {
relationship.self_in_trait |= rel.self_in_trait;
relationship.output |= rel.output;
}
}

let mut is_cast = false;
for vid in cast_vars.iter() {
if self.infcx.root_var(*vid) == root_vid {
is_cast = true;
break;
}
}

if relationship.self_in_trait && relationship.output {
debug!("fallback to () - found trait and projection: {:?}", diverging_vid);
diverging_fallback.insert(diverging_ty, self.tcx.types.unit);
} else if is_cast {
debug!("fallback to () - interacted with cast: {:?}", diverging_vid);
diverging_fallback.insert(diverging_ty, self.tcx.types.unit);
} else if can_reach_non_diverging {
debug!("fallback to () - reached non-diverging: {:?}", diverging_vid);
diverging_fallback.insert(diverging_ty, self.tcx.types.unit);
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-22034.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//~ ERROR expected a `FnOnce<()>` closure, found `()`
#![feature(rustc_private)]

extern crate libc;
Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/issues/issue-22034.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
error[E0277]: expected a `FnOnce<()>` closure, found `()`
|
= help: the trait `FnOnce<()>` is not implemented for `()`
= note: wrap the `()` in a closure with no arguments: `|| { /* code */ }`
= note: required for the cast to the object type `dyn Fn()`

error[E0277]: expected a `Fn<()>` closure, found `()`
--> $DIR/issue-22034.rs:8:16
--> $DIR/issue-22034.rs:9:16
|
LL | &mut *(ptr as *mut dyn Fn())
| ^^^ expected an `Fn<()>` closure, found `()`
Expand All @@ -8,6 +14,6 @@ LL | &mut *(ptr as *mut dyn Fn())
= note: wrap the `()` in a closure with no arguments: `|| { /* code */ }`
= note: required for the cast to the object type `dyn Fn()`

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
3 changes: 2 additions & 1 deletion src/test/ui/never_type/fallback-closure-wrap.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// check-pass

use std::marker::PhantomData;

fn main() {
let error = Closure::wrap(Box::new(move || {
//~^ ERROR type mismatch
panic!("Can't connect to server.");
}) as Box<dyn FnMut()>);
}
Expand Down
17 changes: 0 additions & 17 deletions src/test/ui/never_type/fallback-closure-wrap.stderr

This file was deleted.

23 changes: 0 additions & 23 deletions src/test/ui/self/arbitrary-self-types-not-object-safe.stderr

This file was deleted.

0 comments on commit 40d1e69

Please sign in to comment.