Skip to content

Commit

Permalink
convert EXTRA_REQUIREMENT_IN_IMPL into a hard error
Browse files Browse the repository at this point in the history
  • Loading branch information
nikomatsakis committed Nov 15, 2017
1 parent 64206b4 commit 0d78e40
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 93 deletions.
5 changes: 2 additions & 3 deletions src/librustc/infer/error_reporting/mod.rs
Expand Up @@ -880,14 +880,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
};

if let SubregionOrigin::CompareImplMethodObligation {
span, item_name, impl_item_def_id, trait_item_def_id, lint_id
span, item_name, impl_item_def_id, trait_item_def_id,
} = origin {
self.report_extra_impl_obligation(span,
item_name,
impl_item_def_id,
trait_item_def_id,
&format!("`{}: {}`", bound_kind, sub),
lint_id)
&format!("`{}: {}`", bound_kind, sub))
.emit();
return;
}
Expand Down
6 changes: 2 additions & 4 deletions src/librustc/infer/error_reporting/note.rs
Expand Up @@ -445,14 +445,12 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
infer::CompareImplMethodObligation { span,
item_name,
impl_item_def_id,
trait_item_def_id,
lint_id } => {
trait_item_def_id } => {
self.report_extra_impl_obligation(span,
item_name,
impl_item_def_id,
trait_item_def_id,
&format!("`{}: {}`", sup, sub),
lint_id)
&format!("`{}: {}`", sup, sub))
}
}
}
Expand Down
8 changes: 1 addition & 7 deletions src/librustc/infer/mod.rs
Expand Up @@ -274,10 +274,6 @@ pub enum SubregionOrigin<'tcx> {
item_name: ast::Name,
impl_item_def_id: DefId,
trait_item_def_id: DefId,

// this is `Some(_)` if this error arises from the bug fix for
// #18937. This is a temporary measure.
lint_id: Option<ast::NodeId>,
},
}

Expand Down Expand Up @@ -1532,14 +1528,12 @@ impl<'tcx> SubregionOrigin<'tcx> {

traits::ObligationCauseCode::CompareImplMethodObligation { item_name,
impl_item_def_id,
trait_item_def_id,
lint_id } =>
trait_item_def_id, } =>
SubregionOrigin::CompareImplMethodObligation {
span: cause.span,
item_name,
impl_item_def_id,
trait_item_def_id,
lint_id,
},

_ => default(),
Expand Down
7 changes: 0 additions & 7 deletions src/librustc/lint/builtin.rs
Expand Up @@ -161,12 +161,6 @@ declare_lint! {
"patterns in functions without body were erroneously allowed"
}

declare_lint! {
pub EXTRA_REQUIREMENT_IN_IMPL,
Deny,
"detects extra requirements in impls that were erroneously allowed"
}

declare_lint! {
pub LEGACY_DIRECTORY_OWNERSHIP,
Deny,
Expand Down Expand Up @@ -254,7 +248,6 @@ impl LintPass for HardwiredLints {
RESOLVE_TRAIT_ON_DEFAULTED_UNIT,
SAFE_EXTERN_STATICS,
PATTERNS_IN_FNS_WITHOUT_BODY,
EXTRA_REQUIREMENT_IN_IMPL,
LEGACY_DIRECTORY_OWNERSHIP,
LEGACY_IMPORTS,
LEGACY_CONSTRUCTOR_VISIBILITY,
Expand Down
32 changes: 7 additions & 25 deletions src/librustc/traits/error_reporting.rs
Expand Up @@ -33,7 +33,6 @@ use hir::def_id::DefId;
use infer::{self, InferCtxt};
use infer::type_variable::TypeVariableOrigin;
use middle::const_val;
use rustc::lint::builtin::EXTRA_REQUIREMENT_IN_IMPL;
use std::fmt;
use syntax::ast;
use session::DiagnosticMessageId;
Expand Down Expand Up @@ -481,30 +480,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
item_name: ast::Name,
_impl_item_def_id: DefId,
trait_item_def_id: DefId,
requirement: &fmt::Display,
lint_id: Option<ast::NodeId>) // (*)
requirement: &fmt::Display)
-> DiagnosticBuilder<'tcx>
{
// (*) This parameter is temporary and used only for phasing
// in the bug fix to #18937. If it is `Some`, it has a kind of
// weird effect -- the diagnostic is reported as a lint, and
// the builder which is returned is marked as canceled.

let msg = "impl has stricter requirements than trait";
let mut err = match lint_id {
Some(node_id) => {
self.tcx.struct_span_lint_node(EXTRA_REQUIREMENT_IN_IMPL,
node_id,
error_span,
msg)
}
None => {
struct_span_err!(self.tcx.sess,
error_span,
E0276,
"{}", msg)
}
};
let mut err = struct_span_err!(self.tcx.sess,
error_span,
E0276,
"{}", msg);

if let Some(trait_item_span) = self.tcx.hir.span_if_local(trait_item_def_id) {
let span = self.tcx.sess.codemap().def_span(trait_item_span);
Expand Down Expand Up @@ -543,15 +526,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
let mut err = match *error {
SelectionError::Unimplemented => {
if let ObligationCauseCode::CompareImplMethodObligation {
item_name, impl_item_def_id, trait_item_def_id, lint_id
item_name, impl_item_def_id, trait_item_def_id,
} = obligation.cause.code {
self.report_extra_impl_obligation(
span,
item_name,
impl_item_def_id,
trait_item_def_id,
&format!("`{}`", obligation.predicate),
lint_id)
&format!("`{}`", obligation.predicate))
.emit();
return;
}
Expand Down
1 change: 0 additions & 1 deletion src/librustc/traits/mod.rs
Expand Up @@ -152,7 +152,6 @@ pub enum ObligationCauseCode<'tcx> {
item_name: ast::Name,
impl_item_def_id: DefId,
trait_item_def_id: DefId,
lint_id: Option<ast::NodeId>,
},

/// Checking that this expression can be assigned where it needs to be
Expand Down
4 changes: 1 addition & 3 deletions src/librustc/traits/structural_impls.rs
Expand Up @@ -214,13 +214,11 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
}
super::CompareImplMethodObligation { item_name,
impl_item_def_id,
trait_item_def_id,
lint_id } => {
trait_item_def_id } => {
Some(super::CompareImplMethodObligation {
item_name,
impl_item_def_id,
trait_item_def_id,
lint_id,
})
}
super::ExprAssignable => Some(super::ExprAssignable),
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_lint/lib.rs
Expand Up @@ -207,10 +207,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
id: LintId::of(INVALID_TYPE_PARAM_DEFAULT),
reference: "issue #36887 <https://github.com/rust-lang/rust/issues/36887>",
},
FutureIncompatibleInfo {
id: LintId::of(EXTRA_REQUIREMENT_IN_IMPL),
reference: "issue #37166 <https://github.com/rust-lang/rust/issues/37166>",
},
FutureIncompatibleInfo {
id: LintId::of(LEGACY_DIRECTORY_OWNERSHIP),
reference: "issue #37872 <https://github.com/rust-lang/rust/issues/37872>",
Expand Down Expand Up @@ -276,4 +272,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
"converted into hard error, see https://github.com/rust-lang/rust/issues/36891");
store.register_removed("lifetime_underscore",
"converted into hard error, see https://github.com/rust-lang/rust/issues/36892");
store.register_removed("extra_requirement_in_impl",
"converted into hard error, see https://github.com/rust-lang/rust/issues/37166");
}
30 changes: 5 additions & 25 deletions src/librustc_typeck/check/compare_method.rs
Expand Up @@ -10,8 +10,6 @@

use rustc::hir::{self, ImplItemKind, TraitItemKind};
use rustc::infer::{self, InferOk};
use rustc::middle::free_region::FreeRegionMap;
use rustc::middle::region;
use rustc::ty::{self, TyCtxt};
use rustc::ty::util::ExplicitSelf;
use rustc::traits::{self, ObligationCause, ObligationCauseCode, Reveal};
Expand All @@ -38,8 +36,7 @@ pub fn compare_impl_method<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
impl_m_span: Span,
trait_m: &ty::AssociatedItem,
impl_trait_ref: ty::TraitRef<'tcx>,
trait_item_span: Option<Span>,
old_broken_mode: bool) {
trait_item_span: Option<Span>) {
debug!("compare_impl_method(impl_trait_ref={:?})",
impl_trait_ref);

Expand Down Expand Up @@ -71,8 +68,7 @@ pub fn compare_impl_method<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
impl_m,
impl_m_span,
trait_m,
impl_trait_ref,
old_broken_mode) {
impl_trait_ref) {
return;
}
}
Expand All @@ -81,8 +77,7 @@ fn compare_predicate_entailment<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
impl_m: &ty::AssociatedItem,
impl_m_span: Span,
trait_m: &ty::AssociatedItem,
impl_trait_ref: ty::TraitRef<'tcx>,
old_broken_mode: bool)
impl_trait_ref: ty::TraitRef<'tcx>)
-> Result<(), ErrorReported> {
let trait_to_impl_substs = impl_trait_ref.substs;

Expand All @@ -98,7 +93,6 @@ fn compare_predicate_entailment<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
item_name: impl_m.name,
impl_item_def_id: impl_m.def_id,
trait_item_def_id: trait_m.def_id,
lint_id: if !old_broken_mode { Some(impl_m_node_id) } else { None },
},
};

Expand Down Expand Up @@ -334,22 +328,8 @@ fn compare_predicate_entailment<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

// Finally, resolve all regions. This catches wily misuses of
// lifetime parameters.
if old_broken_mode {
// FIXME(#18937) -- this is how the code used to
// work. This is buggy because the fulfillment cx creates
// region obligations that get overlooked. The right
// thing to do is the code below. But we keep this old
// pass around temporarily.
let region_scope_tree = region::ScopeTree::default();
let mut free_regions = FreeRegionMap::new();
free_regions.relate_free_regions_from_predicates(&param_env.caller_bounds);
infcx.resolve_regions_and_report_errors(impl_m.def_id,
&region_scope_tree,
&free_regions);
} else {
let fcx = FnCtxt::new(&inh, param_env, impl_m_node_id);
fcx.regionck_item(impl_m_node_id, impl_m_span, &[]);
}
let fcx = FnCtxt::new(&inh, param_env, impl_m_node_id);
fcx.regionck_item(impl_m_node_id, impl_m_span, &[]);

Ok(())
})
Expand Down
14 changes: 1 addition & 13 deletions src/librustc_typeck/check/mod.rs
Expand Up @@ -1339,24 +1339,12 @@ fn check_impl_items_against_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
hir::ImplItemKind::Method(..) => {
let trait_span = tcx.hir.span_if_local(ty_trait_item.def_id);
if ty_trait_item.kind == ty::AssociatedKind::Method {
let err_count = tcx.sess.err_count();
compare_impl_method(tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
trait_span,
true); // start with old-broken-mode
if err_count == tcx.sess.err_count() {
// old broken mode did not report an error. Try with the new mode.
compare_impl_method(tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
trait_span,
false); // use the new mode
}
trait_span);
} else {
let mut err = struct_span_err!(tcx.sess, impl_item.span, E0324,
"item `{}` is an associated method, \
Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/issue-18937.rs
Expand Up @@ -27,7 +27,6 @@ trait A<'a> {

impl<'a> A<'a> for B {
fn foo<F>(&mut self, f: F) //~ ERROR impl has stricter
//~^ WARNING future release
where F: fmt::Debug + 'static,
{
self.list.push(Box::new(f));
Expand Down

0 comments on commit 0d78e40

Please sign in to comment.