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

Remove blanket silencing of "type annotation needed" errors #64746

Merged
merged 3 commits into from
Sep 25, 2019
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
4 changes: 1 addition & 3 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2184,9 +2184,7 @@ impl<'a> LoweringContext<'a> {
match decl.output {
FunctionRetTy::Ty(ref ty) => match in_band_ty_params {
Some((def_id, _)) if impl_trait_return_allow => {
hir::Return(self.lower_ty(ty,
ImplTraitContext::OpaqueTy(Some(def_id))
))
hir::Return(self.lower_ty(ty, ImplTraitContext::OpaqueTy(Some(def_id))))
}
_ => {
hir::Return(self.lower_ty(ty, ImplTraitContext::disallowed()))
Expand Down
13 changes: 12 additions & 1 deletion src/librustc/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,9 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
value.fold_with(&mut BottomUpFolder {
tcx,
ty_op: |ty| {
if let ty::Opaque(def_id, substs) = ty.sty {
if ty.references_error() {
return tcx.types.err;
} else if let ty::Opaque(def_id, substs) = ty.sty {
// Check that this is `impl Trait` type is
// declared by `parent_def_id` -- i.e., one whose
// value we are inferring. At present, this is
Expand Down Expand Up @@ -1155,6 +1157,15 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
);
debug!("instantiate_opaque_types: ty_var={:?}", ty_var);

for predicate in &bounds.predicates {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could make a method returning -> bool here and use if stuff.obligation_references_error() { return ty_var; } here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to hide the O(n) behavior of the added code so that I could maybe nerd-snipe someone to point me a place where we could do the same with no added time ^_^

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you just leave a doc comment about the O(n) complexity? This is in diagnostics code anyways so what does O(n) matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it it needs to go over the predicates vec twice instead of once for every case. This is to add the obligations, which may or may not be met.

if let ty::Predicate::Projection(projection) = &predicate {
if projection.skip_binder().ty.references_error() {
// No point on adding these obligations since there's a type error involved.
return ty_var;
}
}
}

self.obligations.reserve(bounds.predicates.len());
for predicate in bounds.predicates {
// Change the predicate to refer to the type variable,
Expand Down
71 changes: 42 additions & 29 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1432,8 +1432,11 @@ impl<'tcx> TyCtxt<'tcx> {
}

impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>,
body_id: Option<hir::BodyId>) {
fn maybe_report_ambiguity(
&self,
obligation: &PredicateObligation<'tcx>,
body_id: Option<hir::BodyId>,
) {
// Unable to successfully determine, probably means
// insufficient type information, but could mean
// ambiguous impls. The latter *ought* to be a
Expand All @@ -1442,9 +1445,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let predicate = self.resolve_vars_if_possible(&obligation.predicate);
let span = obligation.cause.span;

debug!("maybe_report_ambiguity(predicate={:?}, obligation={:?})",
predicate,
obligation);
debug!(
"maybe_report_ambiguity(predicate={:?}, obligation={:?} body_id={:?}, code={:?})",
predicate,
obligation,
body_id,
obligation.cause.code,
);

// Ambiguity errors are often caused as fallout from earlier
// errors. So just ignore them if this infcx is tainted.
Expand All @@ -1456,6 +1463,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
ty::Predicate::Trait(ref data) => {
let trait_ref = data.to_poly_trait_ref();
let self_ty = trait_ref.self_ty();
debug!("self_ty {:?} {:?} trait_ref {:?}", self_ty, self_ty.sty, trait_ref);

if predicate.references_error() {
return;
}
Expand All @@ -1480,24 +1489,25 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// be ignoring the fact that we don't KNOW the type works
// out. Though even that would probably be harmless, given that
// we're only talking about builtin traits, which are known to be
// inhabited. But in any case I just threw in this check for
// has_errors() to be sure that compilation isn't happening
// anyway. In that case, why inundate the user.
if !self.tcx.sess.has_errors() {
if
self.tcx.lang_items().sized_trait()
.map_or(false, |sized_id| sized_id == trait_ref.def_id())
{
self.need_type_info_err(body_id, span, self_ty).emit();
} else {
let mut err = struct_span_err!(self.tcx.sess,
span, E0283,
"type annotations required: \
cannot resolve `{}`",
predicate);
self.note_obligation_cause(&mut err, obligation);
err.emit();
}
// inhabited. We used to check for `self.tcx.sess.has_errors()` to
// avoid inundating the user with unnecessary errors, but we now
// check upstream for type errors and dont add the obligations to
// begin with in those cases.
if
self.tcx.lang_items().sized_trait()
.map_or(false, |sized_id| sized_id == trait_ref.def_id())
{
self.need_type_info_err(body_id, span, self_ty).emit();
} else {
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0283,
"type annotations needed: cannot resolve `{}`",
predicate,
);
self.note_obligation_cause(&mut err, obligation);
err.emit();
}
}

Expand All @@ -1524,11 +1534,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {

_ => {
if !self.tcx.sess.has_errors() {
let mut err = struct_span_err!(self.tcx.sess,
obligation.cause.span, E0284,
"type annotations required: \
cannot resolve `{}`",
predicate);
let mut err = struct_span_err!(
self.tcx.sess,
obligation.cause.span,
E0284,
"type annotations needed: cannot resolve `{}`",
predicate,
);
self.note_obligation_cause(&mut err, obligation);
err.emit();
}
Expand Down Expand Up @@ -1766,7 +1778,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
but not on the corresponding trait method",
predicate));
}
ObligationCauseCode::ReturnType(_) |
ObligationCauseCode::ReturnType |
ObligationCauseCode::ReturnValue(_) |
ObligationCauseCode::BlockTailExpression(_) => (),
ObligationCauseCode::TrivialBound => {
err.help("see issue #48214");
Expand Down
13 changes: 8 additions & 5 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,14 @@ pub enum ObligationCauseCode<'tcx> {
/// Constant expressions must be sized.
ConstSized,

/// static items must have `Sync` type
/// Static items must have `Sync` type
SharedStatic,

BuiltinDerivedObligation(DerivedObligationCause<'tcx>),

ImplDerivedObligation(DerivedObligationCause<'tcx>),

/// error derived when matching traits/impls; see ObligationCause for more details
/// Error derived when matching traits/impls; see ObligationCause for more details
CompareImplMethodObligation {
item_name: ast::Name,
impl_item_def_id: DefId,
Expand Down Expand Up @@ -248,17 +248,20 @@ pub enum ObligationCauseCode<'tcx> {
/// `start` has wrong type
StartFunctionType,

/// intrinsic has wrong type
/// Intrinsic has wrong type
IntrinsicType,

/// method receiver
/// Method receiver
MethodReceiver,

/// `return` with no expression
ReturnNoExpression,

/// `return` with an expression
ReturnType(hir::HirId),
ReturnValue(hir::HirId),

/// Return type of this function
ReturnType,

/// Block implicit return
BlockTailExpression(hir::HirId),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ pub struct SelectionCache<'tcx> {
/// of type variables - it just means the obligation isn't sufficiently
/// elaborated. In that case we report an ambiguity, and the caller can
/// try again after more type information has been gathered or report a
/// "type annotations required" error.
/// "type annotations needed" error.
///
/// However, with type parameters, this can be a real problem - type
/// parameters don't unify with regular types, but they *can* unify
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,8 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::TupleInitializerSized => Some(super::TupleInitializerSized),
super::StructInitializerSized => Some(super::StructInitializerSized),
super::VariableType(id) => Some(super::VariableType(id)),
super::ReturnType(id) => Some(super::ReturnType(id)),
super::ReturnValue(id) => Some(super::ReturnValue(id)),
super::ReturnType => Some(super::ReturnType),
super::SizedArgumentType => Some(super::SizedArgumentType),
super::SizedReturnType => Some(super::SizedReturnType),
super::SizedYieldType => Some(super::SizedYieldType),
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,19 +438,19 @@ bitflags! {

/// `true` if there are "names" of types and regions and so forth
/// that are local to a particular fn
const HAS_FREE_LOCAL_NAMES = 1 << 9;
const HAS_FREE_LOCAL_NAMES = 1 << 9;

/// Present if the type belongs in a local type context.
/// Only set for Infer other than Fresh.
const KEEP_IN_LOCAL_TCX = 1 << 10;

/// Does this have any `ReLateBound` regions? Used to check
/// if a global bound is safe to evaluate.
const HAS_RE_LATE_BOUND = 1 << 11;
const HAS_RE_LATE_BOUND = 1 << 11;

const HAS_TY_PLACEHOLDER = 1 << 12;

const HAS_CT_INFER = 1 << 13;
const HAS_CT_INFER = 1 << 13;
const HAS_CT_PLACEHOLDER = 1 << 14;

const NEEDS_SUBST = TypeFlags::HAS_PARAMS.bits |
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1253,7 +1253,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
expression.map(|expr| (expr, blk_id)),
);
}
ObligationCauseCode::ReturnType(id) => {
ObligationCauseCode::ReturnValue(id) => {
db = self.report_return_mismatched_types(
cause, expected, found, err, fcx, id, None);
}
Expand Down
12 changes: 6 additions & 6 deletions src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,12 +664,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let ret_ty = ret_coercion.borrow().expected_ty();
let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty.clone());
ret_coercion.borrow_mut()
.coerce(self,
&self.cause(return_expr.span,
ObligationCauseCode::ReturnType(return_expr.hir_id)),
return_expr,
return_expr_ty);
ret_coercion.borrow_mut().coerce(
self,
&self.cause(return_expr.span, ObligationCauseCode::ReturnValue(return_expr.hir_id)),
return_expr,
return_expr_ty,
);
}

/// Type check assignment expression `expr` of form `lhs = rhs`.
Expand Down
82 changes: 47 additions & 35 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2698,30 +2698,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
traits::ObligationCause::new(span, self.body_id, code));
}

pub fn require_type_is_sized(&self,
ty: Ty<'tcx>,
span: Span,
code: traits::ObligationCauseCode<'tcx>)
{
let lang_item = self.tcx.require_lang_item(lang_items::SizedTraitLangItem, None);
self.require_type_meets(ty, span, code, lang_item);
pub fn require_type_is_sized(
&self,
ty: Ty<'tcx>,
span: Span,
code: traits::ObligationCauseCode<'tcx>,
) {
if !ty.references_error() {
let lang_item = self.tcx.require_lang_item(lang_items::SizedTraitLangItem, None);
self.require_type_meets(ty, span, code, lang_item);
}
}

pub fn require_type_is_sized_deferred(&self,
ty: Ty<'tcx>,
span: Span,
code: traits::ObligationCauseCode<'tcx>)
{
self.deferred_sized_obligations.borrow_mut().push((ty, span, code));
pub fn require_type_is_sized_deferred(
&self,
ty: Ty<'tcx>,
span: Span,
code: traits::ObligationCauseCode<'tcx>,
) {
if !ty.references_error() {
self.deferred_sized_obligations.borrow_mut().push((ty, span, code));
}
}

pub fn register_bound(&self,
ty: Ty<'tcx>,
def_id: DefId,
cause: traits::ObligationCause<'tcx>)
{
self.fulfillment_cx.borrow_mut()
.register_bound(self, self.param_env, ty, def_id, cause);
pub fn register_bound(
&self,
ty: Ty<'tcx>,
def_id: DefId,
cause: traits::ObligationCause<'tcx>,
) {
if !ty.references_error() {
self.fulfillment_cx.borrow_mut()
.register_bound(self, self.param_env, ty, def_id, cause);
}
}

pub fn to_ty(&self, ast_t: &hir::Ty) -> Ty<'tcx> {
Expand Down Expand Up @@ -2780,22 +2789,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

/// Registers an obligation for checking later, during regionck, that the type `ty` must
/// outlive the region `r`.
pub fn register_wf_obligation(&self,
ty: Ty<'tcx>,
span: Span,
code: traits::ObligationCauseCode<'tcx>)
{
pub fn register_wf_obligation(
&self,
ty: Ty<'tcx>,
span: Span,
code: traits::ObligationCauseCode<'tcx>,
) {
// WF obligations never themselves fail, so no real need to give a detailed cause:
let cause = traits::ObligationCause::new(span, self.body_id, code);
self.register_predicate(traits::Obligation::new(cause,
self.param_env,
ty::Predicate::WellFormed(ty)));
self.register_predicate(
traits::Obligation::new(cause, self.param_env, ty::Predicate::WellFormed(ty)),
);
}

/// Registers obligations that all types appearing in `substs` are well-formed.
pub fn add_wf_bounds(&self, substs: SubstsRef<'tcx>, expr: &hir::Expr) {
for ty in substs.types() {
self.register_wf_obligation(ty, expr.span, traits::MiscObligation);
if !ty.references_error() {
self.register_wf_obligation(ty, expr.span, traits::MiscObligation);
}
}
}

Expand Down Expand Up @@ -2834,12 +2846,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// FIXME(arielb1): use this instead of field.ty everywhere
// Only for fields! Returns <none> for methods>
// Indifferent to privacy flags
pub fn field_ty(&self,
span: Span,
field: &'tcx ty::FieldDef,
substs: SubstsRef<'tcx>)
-> Ty<'tcx>
{
pub fn field_ty(
&self,
span: Span,
field: &'tcx ty::FieldDef,
substs: SubstsRef<'tcx>,
) -> Ty<'tcx> {
self.normalize_associated_types_in(span, &field.ty(self.tcx, substs))
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ fn check_fn_or_method<'fcx, 'tcx>(
}
implied_bounds.extend(sig.inputs());

fcx.register_wf_obligation(sig.output(), span, ObligationCauseCode::MiscObligation);
fcx.register_wf_obligation(sig.output(), span, ObligationCauseCode::ReturnType);

// FIXME(#25759) return types should not be implied bounds
implied_bounds.push(sig.output());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#![feature(trait_alias)]

trait Foo: Iterator<Item = i32> {}
trait Bar: Foo<Item = u32> {} //~ ERROR type annotations required
trait Bar: Foo<Item = u32> {} //~ ERROR type annotations needed

trait I32Iterator = Iterator<Item = i32>;
trait U32Iterator = I32Iterator<Item = u32>;
trait U32Iterator = I32Iterator<Item = u32>; //~ ERROR type annotations needed

fn main() {
let _: &dyn I32Iterator<Item = u32>;
Expand Down
Loading