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

Suppress fallback and ambiguity errors #32258

Merged
merged 7 commits into from
Apr 25, 2016
Merged
Show file tree
Hide file tree
Changes from 5 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
51 changes: 46 additions & 5 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use ty::fold::{TypeFolder, TypeFoldable};
use ty::relate::{Relate, RelateResult, TypeRelation};
use traits::{self, PredicateObligations, ProjectionMode};
use rustc_data_structures::unify::{self, UnificationTable};
use std::cell::{RefCell, Ref};
use std::cell::{Cell, RefCell, Ref};
use std::fmt;
use syntax::ast;
use syntax::codemap;
Expand Down Expand Up @@ -110,6 +110,25 @@ pub struct InferCtxt<'a, 'tcx: 'a> {
// documentation for `ProjectionMode`.
projection_mode: ProjectionMode,

// When an error occurs, we want to avoid reporting "derived"
// errors that are due to this original failure. Normally, we
// handle this with the `err_count_on_creation` count, which
// basically just tracks how many errors were reported when we
// started type-checking a fn and checks to see if any new errors
// have been reported since then. Not great, but it works.
//
// However, when errors originated in other passes -- notably
// resolve -- this heuristic breaks down. Therefore, we have this
// auxiliary flag that one can set whenever one creates a
// type-error that is due to an error in a prior pass.
//
// Don't read this flag directly, call `is_tainted_by_errors()`
// and `set_tainted_by_errors()`.
tainted_by_errors_flag: Cell<bool>,

// Track how many errors were reported when this infcx is created.
// If the number of errors increases, that's also a sign (line
// `tained_by_errors`) to avoid reporting certain kinds of errors.
err_count_on_creation: usize,
}

Expand Down Expand Up @@ -379,6 +398,7 @@ pub fn new_infer_ctxt<'a, 'tcx>(tcx: &'a TyCtxt<'tcx>,
reported_trait_errors: RefCell::new(FnvHashSet()),
normalize: false,
projection_mode: projection_mode,
tainted_by_errors_flag: Cell::new(false),
err_count_on_creation: tcx.sess.err_count()
}
}
Expand Down Expand Up @@ -1128,15 +1148,36 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
.map(|method| resolve_ty(method.ty)))
}

pub fn errors_since_creation(&self) -> bool {
self.tcx.sess.err_count() - self.err_count_on_creation != 0
/// True if errors have been reported since this infcx was
/// created. This is sometimes used as a heuristic to skip
/// reporting errors that often occur as a result of earlier
/// errors, but where it's hard to be 100% sure (e.g., unresolved
/// inference variables, regionck errors).
pub fn is_tainted_by_errors(&self) -> bool {
debug!("is_tainted_by_errors(err_count={}, err_count_on_creation={}, \
tainted_by_errors_flag={})",
self.tcx.sess.err_count(),
self.err_count_on_creation,
self.tainted_by_errors_flag.get());

if self.tcx.sess.err_count() > self.err_count_on_creation {
return true; // errors reported since this infcx was made
}
self.tainted_by_errors_flag.get()
}

/// Set the "tainted by errors" flag to true. We call this when we
/// observe an error from a prior pass.
pub fn set_tainted_by_errors(&self) {
debug!("set_tainted_by_errors()");
self.tainted_by_errors_flag.set(true)
}

pub fn node_type(&self, id: ast::NodeId) -> Ty<'tcx> {
match self.tables.borrow().node_types.get(&id) {
Some(&t) => t,
// FIXME
None if self.errors_since_creation() =>
None if self.is_tainted_by_errors() =>
self.tcx.types.err,
None => {
bug!("no type for node {}: {} in fcx",
Expand All @@ -1158,7 +1199,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
free_regions: &FreeRegionMap,
subject_node_id: ast::NodeId) {
let errors = self.region_vars.resolve_regions(free_regions, subject_node_id);
if !self.errors_since_creation() {
if !self.is_tainted_by_errors() {
// As a heuristic, just skip reporting region errors
// altogether if other errors have been reported while
// this infcx was in use. This is totally hokey but
Expand Down
1 change: 1 addition & 0 deletions src/librustc/infer/sub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ impl<'a, 'tcx> TypeRelation<'a, 'tcx> for Sub<'a, 'tcx> {
}

(&ty::TyError, _) | (_, &ty::TyError) => {
infcx.set_tainted_by_errors();
Ok(self.tcx().types.err)
}

Expand Down
6 changes: 6 additions & 0 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,12 @@ pub fn maybe_report_ambiguity<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
predicate,
obligation);

// Ambiguity errors are often caused as fallout from earlier
// errors. So just ignore them if this infcx is tainted.
if infcx.is_tainted_by_errors() {
return;
}

match predicate {
ty::Predicate::Trait(ref data) => {
let trait_ref = data.to_poly_trait_ref();
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ pub trait AstConv<'tcx> {
_trait_ref: ty::TraitRef<'tcx>,
_item_name: ast::Name)
-> Ty<'tcx>;

/// Invoked when we encounter an error from some prior pass
/// (e.g. resolve) that is translated into a ty-error. This is
/// used to help suppress derived errors typeck might otherwise
/// report.
fn set_tainted_by_errors(&self);
}

pub fn ast_region_to_region(tcx: &TyCtxt, lifetime: &hir::Lifetime)
Expand Down Expand Up @@ -1532,6 +1538,7 @@ fn base_def_to_ty<'tcx>(this: &AstConv<'tcx>,
prim_ty_to_ty(tcx, base_segments, prim_ty)
}
Def::Err => {
this.set_tainted_by_errors();
return this.tcx().types.err;
}
_ => {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ pub fn check_pat<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
let self_ty = fcx.to_ty(&qself.ty);
let path_res = if let Some(&d) = tcx.def_map.borrow().get(&pat.id) {
if d.base_def == Def::Err {
fcx.infcx().set_tainted_by_errors();
fcx.write_error(pat.id);
return;
}
Expand Down Expand Up @@ -628,6 +629,7 @@ fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
let path_res = match tcx.def_map.borrow().get(&pat.id) {
Some(&path_res) if path_res.base_def != Def::Err => path_res,
_ => {
fcx.infcx().set_tainted_by_errors();
fcx.write_error(pat.id);

if let Some(subpats) = subpats {
Expand Down
106 changes: 98 additions & 8 deletions src/librustc_typeck/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,21 @@ use super::structurally_resolved_type;

use lint;
use hir::def_id::DefId;
use rustc::hir;
use rustc::traits;
use rustc::ty::{self, Ty, TypeFoldable};
use rustc::ty::cast::{CastKind, CastTy};
use syntax::codemap::Span;
use rustc::hir;
use syntax::ast;

use syntax::codemap::Span;
use util::common::ErrorReported;

/// Reifies a cast check to be checked once we have full type information for
/// a function context.
pub struct CastCheck<'tcx> {
expr: &'tcx hir::Expr,
expr_ty: Ty<'tcx>,
cast_ty: Ty<'tcx>,
cast_span: Span,
span: Span,
}

Expand Down Expand Up @@ -111,17 +113,34 @@ enum CastError {
}

impl<'tcx> CastCheck<'tcx> {
pub fn new(expr: &'tcx hir::Expr, expr_ty: Ty<'tcx>, cast_ty: Ty<'tcx>, span: Span)
-> CastCheck<'tcx> {
CastCheck {
pub fn new<'a>(fcx: &FnCtxt<'a, 'tcx>,
expr: &'tcx hir::Expr,
expr_ty: Ty<'tcx>,
cast_ty: Ty<'tcx>,
cast_span: Span,
span: Span)
-> Result<CastCheck<'tcx>, ErrorReported> {
let check = CastCheck {
expr: expr,
expr_ty: expr_ty,
cast_ty: cast_ty,
cast_span: cast_span,
span: span,
};

// For better error messages, we try to check whether the
// target type is known to be sized now (we will also check
// later, once inference is more complete done).
if !fcx.type_is_known_to_be_sized(cast_ty, span) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is constructor doing randomly emitting error messages?

BTW, why do we need the early error message? It causes compilation errors in some cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arielb1

What is constructor doing randomly emitting error messages?

As you well know, a constructor is just another function... seems like as good a time as any. And the return type (in particular, the Result with ErrorReported) clearly indicates that an error is reported.

My thinking was basically that instantiating a cast check does some checks and returns, potentially, further checks that may need to be done later. I'm happy to rename.

BTW, why do we need the early error message? It causes compilation errors in some cases.

Yes, this doesn't seem optimal, I agree. I was just preserving the existing behavior -- but it seems like it could go wrong, if only in unusual situations like x as _ or x as Bar<_>

I originally moved this check to be done at the end, but then you get derived errors later. For example:

let x = foo as SomeTrait;

will give an error because the type of x is SomeTrait, which is not Sized. (Actually, it then later gives a error about "cast to unsized type".)

Probably the best thing would be to rewrite type_is_known_to_be_sized to return "yes, no, maybe" and only error out on no. (Or, for simplicity, just check specifically for casts to traits or slices, and leave the more general function for the end.)

check.report_cast_to_unsized_type(fcx);
return Err(ErrorReported);
}

Ok(check)
}

fn report_cast_error<'a>(&self, fcx: &FnCtxt<'a, 'tcx>,
fn report_cast_error<'a>(&self,
fcx: &FnCtxt<'a, 'tcx>,
e: CastError) {
match e {
CastError::NeedViaPtr |
Expand Down Expand Up @@ -186,6 +205,61 @@ impl<'tcx> CastCheck<'tcx> {
}
}

fn report_cast_to_unsized_type<'a>(&self,
fcx: &FnCtxt<'a, 'tcx>) {
if
self.cast_ty.references_error() ||
self.expr_ty.references_error()
{
return;
}

let tstr = fcx.infcx().ty_to_string(self.cast_ty);
let mut err = fcx.type_error_struct(self.span, |actual| {
format!("cast to unsized type: `{}` as `{}`", actual, tstr)
}, self.expr_ty, None);
match self.expr_ty.sty {
ty::TyRef(_, ty::TypeAndMut { mutbl: mt, .. }) => {
let mtstr = match mt {
hir::MutMutable => "mut ",
hir::MutImmutable => ""
};
if self.cast_ty.is_trait() {
match fcx.tcx().sess.codemap().span_to_snippet(self.cast_span) {
Ok(s) => {
err.span_suggestion(self.cast_span,
"try casting to a reference instead:",
format!("&{}{}", mtstr, s));
},
Err(_) =>
span_help!(err, self.cast_span,
"did you mean `&{}{}`?", mtstr, tstr),
}
} else {
span_help!(err, self.span,
"consider using an implicit coercion to `&{}{}` instead",
mtstr, tstr);
}
}
ty::TyBox(..) => {
match fcx.tcx().sess.codemap().span_to_snippet(self.cast_span) {
Ok(s) => {
err.span_suggestion(self.cast_span,
"try casting to a `Box` instead:",
format!("Box<{}>", s));
},
Err(_) =>
span_help!(err, self.cast_span, "did you mean `Box<{}>`?", tstr),
}
}
_ => {
span_help!(err, self.expr.span,
"consider using a box or reference as appropriate");
}
}
err.emit();
}

fn trivial_cast_lint<'a>(&self, fcx: &FnCtxt<'a, 'tcx>) {
let t_cast = self.cast_ty;
let t_expr = self.expr_ty;
Expand Down Expand Up @@ -218,7 +292,9 @@ impl<'tcx> CastCheck<'tcx> {
debug!("check_cast({}, {:?} as {:?})", self.expr.id, self.expr_ty,
self.cast_ty);

if self.expr_ty.references_error() || self.cast_ty.references_error() {
if !fcx.type_is_known_to_be_sized(self.cast_ty, self.span) {
self.report_cast_to_unsized_type(fcx);
} else if self.expr_ty.references_error() || self.cast_ty.references_error() {
// No sense in giving duplicate error messages
} else if self.try_coercion_cast(fcx) {
self.trivial_cast_lint(fcx);
Expand Down Expand Up @@ -403,3 +479,17 @@ impl<'tcx> CastCheck<'tcx> {
}

}

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn type_is_known_to_be_sized(&self,
ty: Ty<'tcx>,
span: Span)
-> bool
{
traits::type_known_to_meet_builtin_bound(self.infcx(),
ty,
ty::BoundSized,
span)
}
}

Loading