mathieu/os-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #47738 - nikomatsakis:issue-47139-master, r=arielb1

Browse source 
remove intercrate ambiguity hints

The scheme was causing overflows during coherence checking (e.g. #47139). This is sort of a temporary fix; the proper fix I think involves reworking trait selection in deeper ways.

cc @sgrif -- this *should* fix diesel

cc @qnighy -- I'd like to discuss you with alternative techniques for achieving the same end. =) Actually, it might be good to put some energy into refactoring traits first.

r? @eddyb
This commit is contained in:
bors 2018-02-01 07:33:35 +00:00
commit 56733bc9f8
7 changed files with 307 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/librustc/lib.rs
View file

@ -66,6 +66,7 @@
#![feature(specialization)]
#![feature(unboxed_closures)]
#![feature(underscore_lifetimes)]
#![feature(universal_impl_trait)]
#![feature(trace_macros)]
#![feature(catch_expr)]
#![feature(test)]

50
src/librustc/traits/coherence.rs
View file

@ -19,7 +19,7 @@ use ty::{self, Ty, TyCtxt};
use ty::fold::TypeFoldable;
use ty::subst::Subst;
use infer::{InferCtxt, InferOk};
use infer::{InferOk};
/// Whether we do the orphan check relative to this crate or
/// to some remote crate.
@ -40,13 +40,20 @@ pub struct OverlapResult<'tcx> {
pub intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
}
/// If there are types that satisfy both impls, returns a suitably-freshened
/// `ImplHeader` with those types substituted
pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>,
impl1_def_id: DefId,
impl2_def_id: DefId,
intercrate_mode: IntercrateMode)
-> Option<OverlapResult<'tcx>>
/// If there are types that satisfy both impls, invokes `on_overlap`
/// with a suitably-freshened `ImplHeader` with those types
/// substituted. Otherwise, invokes `no_overlap`.
pub fn overlapping_impls<'gcx, F1, F2, R>(
tcx: TyCtxt<'_, 'gcx, 'gcx>,
impl1_def_id: DefId,
impl2_def_id: DefId,
intercrate_mode: IntercrateMode,
on_overlap: F1,
no_overlap: F2,
) -> R
where
F1: FnOnce(OverlapResult<'_>) -> R,
F2: FnOnce() -> R,
{
debug!("impl_can_satisfy(\
impl1_def_id={:?}, \
@ -56,8 +63,23 @@ pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>,
impl2_def_id,
intercrate_mode);
let selcx = &mut SelectionContext::intercrate(infcx, intercrate_mode);
overlap(selcx, impl1_def_id, impl2_def_id)
let overlaps = tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
overlap(selcx, impl1_def_id, impl2_def_id).is_some()
});
if !overlaps {
return no_overlap();
}
// In the case where we detect an error, run the check again, but
// this time tracking intercrate ambuiguity causes for better
// diagnostics. (These take time and can lead to false errors.)
tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
selcx.enable_tracking_intercrate_ambiguity_causes();
on_overlap(overlap(selcx, impl1_def_id, impl2_def_id).unwrap())
})
}
fn with_fresh_ty_vars<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
@ -135,10 +157,10 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
return None
}
Some(OverlapResult {
impl_header: selcx.infcx().resolve_type_vars_if_possible(&a_impl_header),
intercrate_ambiguity_causes: selcx.intercrate_ambiguity_causes().to_vec(),
})
let impl_header = selcx.infcx().resolve_type_vars_if_possible(&a_impl_header);
let intercrate_ambiguity_causes = selcx.take_intercrate_ambiguity_causes();
debug!("overlap: intercrate_ambiguity_causes={:#?}", intercrate_ambiguity_causes);
Some(OverlapResult { impl_header, intercrate_ambiguity_causes })
}
pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,

104
src/librustc/traits/select.rs
View file

@ -92,10 +92,10 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
inferred_obligations: SnapshotVec<InferredObligationsSnapshotVecDelegate<'tcx>>,
intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
intercrate_ambiguity_causes: Option<Vec<IntercrateAmbiguityCause>>,
}
#[derive(Clone)]
#[derive(Clone, Debug)]
pub enum IntercrateAmbiguityCause {
DownstreamCrate {
trait_desc: String,
@ -423,7 +423,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
freshener: infcx.freshener(),
intercrate: None,
inferred_obligations: SnapshotVec::new(),
intercrate_ambiguity_causes: Vec::new(),
intercrate_ambiguity_causes: None,
}
}
@ -435,10 +435,30 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
freshener: infcx.freshener(),
intercrate: Some(mode),
inferred_obligations: SnapshotVec::new(),
intercrate_ambiguity_causes: Vec::new(),
intercrate_ambiguity_causes: None,
}
}
/// Enables tracking of intercrate ambiguity causes. These are
/// used in coherence to give improved diagnostics. We don't do
/// this until we detect a coherence error because it can lead to
/// false overflow results (#47139) and because it costs
/// computation time.
pub fn enable_tracking_intercrate_ambiguity_causes(&mut self) {
assert!(self.intercrate.is_some());
assert!(self.intercrate_ambiguity_causes.is_none());
self.intercrate_ambiguity_causes = Some(vec![]);
debug!("selcx: enable_tracking_intercrate_ambiguity_causes");
}
/// Gets the intercrate ambiguity causes collected since tracking
/// was enabled and disables tracking at the same time. If
/// tracking is not enabled, just returns an empty vector.
pub fn take_intercrate_ambiguity_causes(&mut self) -> Vec<IntercrateAmbiguityCause> {
assert!(self.intercrate.is_some());
self.intercrate_ambiguity_causes.take().unwrap_or(vec![])
}
pub fn infcx(&self) -> &'cx InferCtxt<'cx, 'gcx, 'tcx> {
self.infcx
}
@ -451,10 +471,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
self.infcx
}
pub fn intercrate_ambiguity_causes(&self) -> &[IntercrateAmbiguityCause] {
&self.intercrate_ambiguity_causes
}
/// Wraps the inference context's in_snapshot s.t. snapshot handling is only from the selection
/// context's self.
fn in_snapshot<R, F>(&mut self, f: F) -> R
@ -828,19 +844,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous",
stack.fresh_trait_ref);
// Heuristics: show the diagnostics when there are no candidates in crate.
if let Ok(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 cause = IntercrateAmbiguityCause::DownstreamCrate {
trait_desc: trait_ref.to_string(),
self_desc: if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
},
};
self.intercrate_ambiguity_causes.push(cause);
if self.intercrate_ambiguity_causes.is_some() {
debug!("evaluate_stack: intercrate_ambiguity_causes is some");
if let Ok(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 cause = IntercrateAmbiguityCause::DownstreamCrate {
trait_desc: trait_ref.to_string(),
self_desc: if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
},
};
debug!("evaluate_stack: pushing cause = {:?}", cause);
self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
}
}
}
return EvaluatedToAmbig;
@ -1092,25 +1112,29 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
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);
if self.intercrate_ambiguity_causes.is_some() {
debug!("evaluate_stack: intercrate_ambiguity_causes is some");
// 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 }
};
debug!("evaluate_stack: pushing cause = {:?}", cause);
self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
}
}
return Ok(None);
}

37
src/librustc/traits/specialize/specialization_graph.rs
View file

@ -133,12 +133,12 @@ impl<'a, 'gcx, 'tcx> Children {
};
let tcx = tcx.global_tcx();
let (le, ge) = tcx.infer_ctxt().enter(|infcx| {
let overlap = traits::overlapping_impls(&infcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Issue43355);
if let Some(overlap) = overlap {
let (le, ge) = traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Issue43355,
|overlap| {
if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
return Ok((false, false));
}
@ -151,10 +151,9 @@ impl<'a, 'gcx, 'tcx> Children {
} else {
Ok((le, ge))
}
} else {
Ok((false, false))
}
})?;
},
|| Ok((false, false)),
)?;
if le && !ge {
debug!("descending as child of TraitRef {:?}",
@ -171,16 +170,14 @@ impl<'a, 'gcx, 'tcx> Children {
return Ok(Inserted::Replaced(possible_sibling));
} else {
if !tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
tcx.infer_ctxt().enter(|infcx| {
if let Some(overlap) = traits::overlapping_impls(
&infcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Fixed)
{
last_lint = Some(overlap_error(overlap));
}
});
traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Fixed,
|overlap| last_lint = Some(overlap_error(overlap)),
|| (),
);
}
// no overlap (error bailed already via ?)

46
src/librustc_typeck/coherence/inherent_impls_overlap.rs
View file

@ -82,29 +82,37 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
for (i, &impl1_def_id) in impls.iter().enumerate() {
for &impl2_def_id in &impls[(i + 1)..] {
let used_to_be_allowed = self.tcx.infer_ctxt().enter(|infcx| {
if let Some(overlap) =
traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id,
IntercrateMode::Issue43355)
{
let used_to_be_allowed = traits::overlapping_impls(
self.tcx,
impl1_def_id,
impl2_def_id,
IntercrateMode::Issue43355,
|overlap| {
self.check_for_common_items_in_impls(
impl1_def_id, impl2_def_id, overlap, false);
impl1_def_id,
impl2_def_id,
overlap,
false,
);
false
} else {
true
}
});
},
|| true,
);
if used_to_be_allowed {
self.tcx.infer_ctxt().enter(|infcx| {
if let Some(overlap) =
traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id,
IntercrateMode::Fixed)
{
self.check_for_common_items_in_impls(
impl1_def_id, impl2_def_id, overlap, true);
}
});
traits::overlapping_impls(
self.tcx,
impl1_def_id,
impl2_def_id,
IntercrateMode::Fixed,
|overlap| self.check_for_common_items_in_impls(
impl1_def_id,
impl2_def_id,
overlap,
true,
),
|| (),
);
}
}
}

87
src/test/run-pass/issue-47139-1.rs Normal file
View file

@ -0,0 +1,87 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Regression test for issue #47139:
//
// Coherence was encountering an (unnecessary) overflow trying to
// decide if the two impls of dummy overlap.
//
// The overflow went something like:
//
// - `&'a ?T: Insertable` ?
// - let ?T = Option<?U> ?
// - `Option<?U>: Insertable` ?
// - `Option<&'a ?U>: Insertable` ?
// - `&'a ?U: Insertable` ?
//
// While somewhere in the middle, a projection would occur, which
// broke cycle detection.
//
// It turned out that this cycle was being kicked off due to some
// extended diagnostic attempts in coherence, so removing those
// sidestepped the issue for now.
#![allow(dead_code)]
pub trait Insertable {
type Values;
fn values(self) -> Self::Values;
}
impl<T> Insertable for Option<T>
where
T: Insertable,
T::Values: Default,
{
type Values = T::Values;
fn values(self) -> Self::Values {
self.map(Insertable::values).unwrap_or_default()
}
}
impl<'a, T> Insertable for &'a Option<T>
where
Option<&'a T>: Insertable,
{
type Values = <Option<&'a T> as Insertable>::Values;
fn values(self) -> Self::Values {
self.as_ref().values()
}
}
impl<'a, T> Insertable for &'a [T]
{
type Values = Self;
fn values(self) -> Self::Values {
self
}
}
trait Unimplemented { }
trait Dummy { }
struct Foo<T> { t: T }
impl<'a, U> Dummy for Foo<&'a U>
where &'a U: Insertable
{
}
impl<T> Dummy for T
where T: Unimplemented
{ }
fn main() {
}

75
src/test/run-pass/issue-47139-2.rs Normal file
View file

@ -0,0 +1,75 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Regression test for issue #47139:
//
// Same as issue-47139-1.rs, but the impls of dummy are in the
// opposite order. This influenced the way that coherence ran and in
// some cases caused the overflow to occur when it wouldn't otherwise.
// In an effort to make the regr test more robust, I am including both
// orderings.
#![allow(dead_code)]
pub trait Insertable {
type Values;
fn values(self) -> Self::Values;
}
impl<T> Insertable for Option<T>
where
T: Insertable,
T::Values: Default,
{
type Values = T::Values;
fn values(self) -> Self::Values {
self.map(Insertable::values).unwrap_or_default()
}
}
impl<'a, T> Insertable for &'a Option<T>
where
Option<&'a T>: Insertable,
{
type Values = <Option<&'a T> as Insertable>::Values;
fn values(self) -> Self::Values {
self.as_ref().values()
}
}
impl<'a, T> Insertable for &'a [T]
{
type Values = Self;
fn values(self) -> Self::Values {
self
}
}
trait Unimplemented { }
trait Dummy { }
struct Foo<T> { t: T }
impl<T> Dummy for T
where T: Unimplemented
{ }
impl<'a, U> Dummy for Foo<&'a U>
where &'a U: Insertable
{
}
fn main() {
}