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

coherence: fix is_knowable logic #46192

Merged
merged 6 commits into from
Dec 6, 2017
Merged
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
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
@@ -204,6 +204,12 @@ declare_lint! {
"detects generic lifetime arguments in path segments with late bound lifetime parameters"
}

declare_lint! {
pub INCOHERENT_FUNDAMENTAL_IMPLS,
Warn,
"potentially-conflicting impls were erroneously allowed"
}

declare_lint! {
pub DEPRECATED,
Warn,
@@ -267,6 +273,7 @@ impl LintPass for HardwiredLints {
MISSING_FRAGMENT_SPECIFIER,
PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
LATE_BOUND_LIFETIME_ARGUMENTS,
INCOHERENT_FUNDAMENTAL_IMPLS,
DEPRECATED,
UNUSED_UNSAFE,
UNUSED_MUT,
249 changes: 192 additions & 57 deletions src/librustc/traits/coherence.rs

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
@@ -60,6 +60,13 @@ mod structural_impls;
pub mod trans;
mod util;

// Whether to enable bug compatibility with issue #43355
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum IntercrateMode {
Issue43355,
Fixed
}

/// An `Obligation` represents some trait reference (e.g. `int:Eq`) for
/// which the vtable must be found. The process of finding a vtable is
/// called "resolving" the `Obligation`. This process consists of
99 changes: 60 additions & 39 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
@@ -13,8 +13,9 @@
use self::SelectionCandidate::*;
use self::EvaluationResult::*;

use super::coherence;
use super::coherence::{self, Conflict};
use super::DerivedObligationCause;
use super::IntercrateMode;
use super::project;
use super::project::{normalize_with_depth, Normalized, ProjectionCacheKey};
use super::{PredicateObligation, TraitObligation, ObligationCause};
@@ -87,7 +88,7 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
/// other words, we consider `$0 : Bar` to be unimplemented if
/// there is no type that the user could *actually name* that
/// would satisfy it. This avoids crippling inference, basically.
intercrate: bool,
intercrate: Option<IntercrateMode>,

inferred_obligations: SnapshotVec<InferredObligationsSnapshotVecDelegate<'tcx>>,

@@ -111,21 +112,24 @@ impl IntercrateAmbiguityCause {
/// See #23980 for details.
pub fn add_intercrate_ambiguity_hint<'a, 'tcx>(&self,
err: &mut ::errors::DiagnosticBuilder) {
err.note(&self.intercrate_ambiguity_hint());
}

pub fn intercrate_ambiguity_hint(&self) -> String {
match self {
&IntercrateAmbiguityCause::DownstreamCrate { ref trait_desc, ref self_desc } => {
let self_desc = if let &Some(ref ty) = self_desc {
format!(" for type `{}`", ty)
} else { "".to_string() };
err.note(&format!("downstream crates may implement trait `{}`{}",
trait_desc, self_desc));
format!("downstream crates may implement trait `{}`{}", trait_desc, self_desc)
}
&IntercrateAmbiguityCause::UpstreamCrateUpdate { ref trait_desc, ref self_desc } => {
let self_desc = if let &Some(ref ty) = self_desc {
format!(" for type `{}`", ty)
} else { "".to_string() };
err.note(&format!("upstream crates may add new impl of trait `{}`{} \
in future versions",
trait_desc, self_desc));
format!("upstream crates may add new impl of trait `{}`{} \
in future versions",
trait_desc, self_desc)
}
}
}
@@ -417,17 +421,19 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
SelectionContext {
infcx,
freshener: infcx.freshener(),
intercrate: false,
intercrate: None,
inferred_obligations: SnapshotVec::new(),
intercrate_ambiguity_causes: Vec::new(),
}
}

pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>) -> SelectionContext<'cx, 'gcx, 'tcx> {
pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>,
mode: IntercrateMode) -> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("intercrate({:?})", mode);
SelectionContext {
infcx,
freshener: infcx.freshener(),
intercrate: true,
intercrate: Some(mode),
inferred_obligations: SnapshotVec::new(),
intercrate_ambiguity_causes: Vec::new(),
}
@@ -758,7 +764,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("evaluate_trait_predicate_recursively({:?})",
obligation);

if !self.intercrate && obligation.is_global() {
if !self.intercrate.is_some() && obligation.is_global() {
// If a param env is consistent, global obligations do not depend on its particular
// value in order to work, so we can clear out the param env and get better
// caching. (If the current param env is inconsistent, we don't care what happens).
@@ -814,7 +820,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// terms of `Fn` etc, but we could probably make this more
// precise still.
let unbound_input_types = stack.fresh_trait_ref.input_types().any(|ty| ty.is_fresh());
if unbound_input_types && self.intercrate {
// this check was an imperfect workaround for a bug n the old
// intercrate mode, it should be removed when that goes away.
if unbound_input_types &&
self.intercrate == Some(IntercrateMode::Issue43355)
{
debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous",
stack.fresh_trait_ref);
// Heuristics: show the diagnostics when there are no candidates in crate.
@@ -1077,28 +1087,32 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
return Ok(None);
}

if !self.is_knowable(stack) {
debug!("coherence stage: not knowable");
// Heuristics: show the diagnostics when there are no candidates in crate.
let candidate_set = self.assemble_candidates(stack)?;
if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
let self_ty = trait_ref.self_ty();
let trait_desc = trait_ref.to_string();
let self_desc = if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
};
let cause = if !coherence::trait_ref_is_local_or_fundamental(self.tcx(),
trait_ref) {
IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc }
} else {
IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
};
self.intercrate_ambiguity_causes.push(cause);
match self.is_knowable(stack) {
None => {}
Some(conflict) => {
debug!("coherence stage: not knowable");
// Heuristics: show the diagnostics when there are no candidates in crate.
let candidate_set = self.assemble_candidates(stack)?;
if !candidate_set.ambiguous && candidate_set.vec.iter().all(|c| {
!self.evaluate_candidate(stack, &c).may_apply()
}) {
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
let self_ty = trait_ref.self_ty();
let trait_desc = trait_ref.to_string();
let self_desc = if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
};
let cause = if let Conflict::Upstream = conflict {
IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc }
} else {
IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
};
self.intercrate_ambiguity_causes.push(cause);
}
return Ok(None);
}
return Ok(None);
}

let candidate_set = self.assemble_candidates(stack)?;
@@ -1205,12 +1219,12 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {

fn is_knowable<'o>(&mut self,
stack: &TraitObligationStack<'o, 'tcx>)
-> bool
-> Option<Conflict>
{
debug!("is_knowable(intercrate={})", self.intercrate);
debug!("is_knowable(intercrate={:?})", self.intercrate);

if !self.intercrate {
return true;
if !self.intercrate.is_some() {
return None;
}

let obligation = &stack.obligation;
@@ -1221,7 +1235,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// bound regions
let trait_ref = predicate.skip_binder().trait_ref;

coherence::trait_ref_is_knowable(self.tcx(), trait_ref)
let result = coherence::trait_ref_is_knowable(self.tcx(), trait_ref);
if let (Some(Conflict::Downstream { used_to_be_broken: true }),
Some(IntercrateMode::Issue43355)) = (result, self.intercrate) {
debug!("is_knowable: IGNORING conflict to be bug-compatible with #43355");
None
} else {
result
}
}

/// Returns true if the global caches can be used.
@@ -1246,7 +1267,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// the master cache. Since coherence executes pretty quickly,
// it's not worth going to more trouble to increase the
// hit-rate I don't think.
if self.intercrate {
if self.intercrate.is_some() {
return false;
}

39 changes: 29 additions & 10 deletions src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
@@ -30,6 +30,8 @@ use ty::{self, TyCtxt, TypeFoldable};
use syntax_pos::DUMMY_SP;
use std::rc::Rc;

use lint;

pub mod specialization_graph;

/// Information pertinent to an overlapping impl error.
@@ -325,16 +327,33 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx
// This is where impl overlap checking happens:
let insert_result = sg.insert(tcx, impl_def_id);
// Report error if there was one.
if let Err(overlap) = insert_result {
let mut err = struct_span_err!(tcx.sess,
tcx.span_of_impl(impl_def_id).unwrap(),
E0119,
"conflicting implementations of trait `{}`{}:",
overlap.trait_desc,
overlap.self_desc.clone().map_or(String::new(),
|ty| {
format!(" for type `{}`", ty)
}));
let (overlap, used_to_be_allowed) = match insert_result {
Err(overlap) => (Some(overlap), false),
Ok(opt_overlap) => (opt_overlap, true)
};

if let Some(overlap) = overlap {
let msg = format!("conflicting implementations of trait `{}`{}:{}",
overlap.trait_desc,
overlap.self_desc.clone().map_or(
String::new(), |ty| {
format!(" for type `{}`", ty)
}),
if used_to_be_allowed { " (E0119)" } else { "" }
);
let mut err = if used_to_be_allowed {
tcx.struct_span_lint_node(
lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS,
tcx.hir.as_local_node_id(impl_def_id).unwrap(),
tcx.span_of_impl(impl_def_id).unwrap(),
&msg)
} else {
struct_span_err!(tcx.sess,
tcx.span_of_impl(impl_def_id).unwrap(),
E0119,
"{}",
msg)
};

match tcx.span_of_impl(overlap.with_impl) {
Ok(span) => {
Loading