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

Add warning for () to ! switch #39009

Merged
merged 9 commits into from
Feb 5, 2017
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
8 changes: 8 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ declare_lint! {
"lifetimes or labels named `'_` were erroneously allowed"
}

declare_lint! {
pub RESOLVE_TRAIT_ON_DEFAULTED_UNIT,
Warn,
"attempt to resolve a trait on an expression whose type cannot be inferred but which \
currently defaults to ()"
}

declare_lint! {
pub SAFE_EXTERN_STATICS,
Warn,
Expand Down Expand Up @@ -272,6 +279,7 @@ impl LintPass for HardwiredLints {
SUPER_OR_SELF_IN_GLOBAL_PATH,
HR_LIFETIME_IN_ASSOC_TYPE,
LIFETIME_UNDERSCORE,
RESOLVE_TRAIT_ON_DEFAULTED_UNIT,
SAFE_EXTERN_STATICS,
PATTERNS_IN_FNS_WITHOUT_BODY,
EXTRA_REQUIREMENT_IN_IMPL,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
PatKind::Tuple(ref subpats, ddpos) => {
// (p1, ..., pN)
let expected_len = match self.pat_ty(&pat)?.sty {
ty::TyTuple(ref tys) => tys.len(),
ty::TyTuple(ref tys, _) => tys.len(),
ref ty => span_bug!(pat.span, "tuple pattern unexpected type {:?}", ty),
};
for (i, subpat) in subpats.iter().enumerate_and_adjust(expected_len, ddpos) {
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl<'tcx> Rvalue<'tcx> {
let lhs_ty = lhs.ty(mir, tcx);
let rhs_ty = rhs.ty(mir, tcx);
let ty = op.ty(tcx, lhs_ty, rhs_ty);
let ty = tcx.intern_tup(&[ty, tcx.types.bool]);
let ty = tcx.intern_tup(&[ty, tcx.types.bool], false);
Some(ty)
}
&Rvalue::UnaryOp(_, ref operand) => {
Expand All @@ -184,7 +184,8 @@ impl<'tcx> Rvalue<'tcx> {
}
AggregateKind::Tuple => {
Some(tcx.mk_tup(
ops.iter().map(|op| op.ty(mir, tcx))
ops.iter().map(|op| op.ty(mir, tcx)),
false
))
}
AggregateKind::Adt(def, _, substs, _) => {
Expand Down
60 changes: 52 additions & 8 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ use std::mem;
use std::rc::Rc;
use syntax::abi::Abi;
use hir;
use lint;
use util::nodemap::FxHashMap;

struct InferredObligationsSnapshotVecDelegate<'tcx> {
Expand Down Expand Up @@ -407,19 +408,62 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("select({:?})", obligation);
assert!(!obligation.predicate.has_escaping_regions());

let tcx = self.tcx();
let dep_node = obligation.predicate.dep_node();
let _task = self.tcx().dep_graph.in_task(dep_node);
let _task = tcx.dep_graph.in_task(dep_node);

let stack = self.push_stack(TraitObligationStackList::empty(), obligation);
match self.candidate_from_obligation(&stack)? {
None => Ok(None),
let ret = match self.candidate_from_obligation(&stack)? {
None => None,
Some(candidate) => {
let mut candidate = self.confirm_candidate(obligation, candidate)?;
let inferred_obligations = (*self.inferred_obligations).into_iter().cloned();
candidate.nested_obligations_mut().extend(inferred_obligations);
Ok(Some(candidate))
Some(candidate)
},
};

// Test whether this is a `()` which was produced by defaulting a
// diverging type variable with `!` disabled. If so, we may need
// to raise a warning.
if obligation.predicate.skip_binder().self_ty().is_defaulted_unit() {
let mut raise_warning = true;
// Don't raise a warning if the trait is implemented for ! and only
// permits a trivial implementation for !. This stops us warning
// about (for example) `(): Clone` becoming `!: Clone` because such
// a switch can't cause code to stop compiling or execute
// differently.
let mut never_obligation = obligation.clone();
let def_id = never_obligation.predicate.skip_binder().trait_ref.def_id;
never_obligation.predicate = never_obligation.predicate.map_bound(|mut trait_pred| {
// Swap out () with ! so we can check if the trait is impld for !
{
let mut trait_ref = &mut trait_pred.trait_ref;
let unit_substs = trait_ref.substs;
let mut never_substs = Vec::with_capacity(unit_substs.len());
never_substs.push(From::from(tcx.types.never));
never_substs.extend(&unit_substs[1..]);
trait_ref.substs = tcx.intern_substs(&never_substs);
}
trait_pred
});
if let Ok(Some(..)) = self.select(&never_obligation) {
if !tcx.trait_relevant_for_never(def_id) {
// The trait is also implemented for ! and the resulting
// implementation cannot actually be invoked in any way.
raise_warning = false;
}
}

if raise_warning {
tcx.sess.add_lint(lint::builtin::RESOLVE_TRAIT_ON_DEFAULTED_UNIT,
obligation.cause.body_id,
obligation.cause.span,
format!("code relies on type inference rules which are likely \
to change"));
}
}
Ok(ret)
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1744,15 +1788,15 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {

ty::TyStr | ty::TySlice(_) | ty::TyDynamic(..) => Never,

ty::TyTuple(tys) => {
ty::TyTuple(tys, _) => {
Where(ty::Binder(tys.last().into_iter().cloned().collect()))
}

ty::TyAdt(def, substs) => {
let sized_crit = def.sized_constraint(self.tcx());
// (*) binder moved here
Where(ty::Binder(match sized_crit.sty {
ty::TyTuple(tys) => tys.to_vec().subst(self.tcx(), substs),
ty::TyTuple(tys, _) => tys.to_vec().subst(self.tcx(), substs),
ty::TyBool => vec![],
_ => vec![sized_crit.subst(self.tcx(), substs)]
}))
Expand Down Expand Up @@ -1799,7 +1843,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
Where(ty::Binder(vec![element_ty]))
}

ty::TyTuple(tys) => {
ty::TyTuple(tys, _) => {
// (*) binder moved here
Where(ty::Binder(tys.to_vec()))
}
Expand Down Expand Up @@ -1874,7 +1918,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
vec![element_ty]
}

ty::TyTuple(ref tys) => {
ty::TyTuple(ref tys, _) => {
// (T1, ..., Tn) -- meets any bound that all of T1...Tn meet
tys.to_vec()
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
let arguments_tuple = match tuple_arguments {
TupleArgumentsFlag::No => sig.skip_binder().inputs()[0],
TupleArgumentsFlag::Yes =>
self.intern_tup(sig.skip_binder().inputs()),
self.intern_tup(sig.skip_binder().inputs(), false),
};
let trait_ref = ty::TraitRef {
def_id: fn_trait_def_id,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/contents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
|ty| tc_ty(tcx, &ty, cache))
}

ty::TyTuple(ref tys) => {
ty::TyTuple(ref tys, _) => {
TypeContents::union(&tys[..],
|ty| tc_ty(tcx, *ty, cache))
}
Expand Down
13 changes: 7 additions & 6 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1379,23 +1379,24 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.mk_ty(TySlice(ty))
}

pub fn intern_tup(self, ts: &[Ty<'tcx>]) -> Ty<'tcx> {
self.mk_ty(TyTuple(self.intern_type_list(ts)))
pub fn intern_tup(self, ts: &[Ty<'tcx>], defaulted: bool) -> Ty<'tcx> {
self.mk_ty(TyTuple(self.intern_type_list(ts), defaulted))
}

pub fn mk_tup<I: InternAs<[Ty<'tcx>], Ty<'tcx>>>(self, iter: I) -> I::Output {
iter.intern_with(|ts| self.mk_ty(TyTuple(self.intern_type_list(ts))))
pub fn mk_tup<I: InternAs<[Ty<'tcx>], Ty<'tcx>>>(self, iter: I,
defaulted: bool) -> I::Output {
iter.intern_with(|ts| self.mk_ty(TyTuple(self.intern_type_list(ts), defaulted)))
}

pub fn mk_nil(self) -> Ty<'tcx> {
self.intern_tup(&[])
self.intern_tup(&[], false)
}

pub fn mk_diverging_default(self) -> Ty<'tcx> {
if self.sess.features.borrow().never_type {
self.types.never
} else {
self.mk_nil()
self.intern_tup(&[], true)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl<'a, 'gcx, 'lcx, 'tcx> ty::TyS<'tcx> {
match self.sty {
ty::TyBool | ty::TyChar | ty::TyInt(_) |
ty::TyUint(_) | ty::TyFloat(_) | ty::TyStr | ty::TyNever => self.to_string(),
ty::TyTuple(ref tys) if tys.is_empty() => self.to_string(),
ty::TyTuple(ref tys, _) if tys.is_empty() => self.to_string(),

ty::TyAdt(def, _) => format!("{} `{}`", def.descr(), tcx.item_path_str(def.did)),
ty::TyArray(_, n) => format!("array of {} elements", n),
Expand Down Expand Up @@ -209,7 +209,7 @@ impl<'a, 'gcx, 'lcx, 'tcx> ty::TyS<'tcx> {
|p| format!("trait {}", tcx.item_path_str(p.def_id())))
}
ty::TyClosure(..) => "closure".to_string(),
ty::TyTuple(_) => "tuple".to_string(),
ty::TyTuple(..) => "tuple".to_string(),
ty::TyInfer(ty::TyVar(_)) => "inferred type".to_string(),
ty::TyInfer(ty::IntVar(_)) => "integral variable".to_string(),
ty::TyInfer(ty::FloatVar(_)) => "floating-point variable".to_string(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/fast_reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub fn simplify_type<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
Some(ClosureSimplifiedType(def_id))
}
ty::TyNever => Some(NeverSimplifiedType),
ty::TyTuple(ref tys) => {
ty::TyTuple(ref tys, _) => {
Some(TupleSimplifiedType(tys.len()))
}
ty::TyFnDef(.., ref f) | ty::TyFnPtr(ref f) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl FlagComputation {
self.add_ty(m.ty);
}

&ty::TyTuple(ref ts) => {
&ty::TyTuple(ref ts, _) => {
self.add_tys(&ts[..]);
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl<'a, 'gcx, 'tcx> TyS<'tcx> {
},

TyNever => DefIdForest::full(tcx),
TyTuple(ref tys) => {
TyTuple(ref tys, _) => {
DefIdForest::union(tcx, tys.iter().map(|ty| {
ty.uninhabited_from(visited, tcx)
}))
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/item_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,9 @@ pub fn characteristic_def_id_of_type(ty: Ty) -> Option<DefId> {
ty::TyRawPtr(mt) |
ty::TyRef(_, mt) => characteristic_def_id_of_type(mt.ty),

ty::TyTuple(ref tys) => tys.iter()
.filter_map(|ty| characteristic_def_id_of_type(ty))
.next(),
ty::TyTuple(ref tys, _) => tys.iter()
.filter_map(|ty| characteristic_def_id_of_type(ty))
.next(),

ty::TyFnDef(def_id, ..) |
ty::TyClosure(def_id, _) => Some(def_id),
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ impl<'a, 'gcx, 'tcx> Struct {
Some(&variant.memory_index[..]))
}
// Can we use one of the fields in this tuple?
(&Univariant { ref variant, .. }, &ty::TyTuple(tys)) => {
(&Univariant { ref variant, .. }, &ty::TyTuple(tys, _)) => {
Struct::non_zero_field_paths(infcx, tys.iter().cloned(),
Some(&variant.memory_index[..]))
}
Expand Down Expand Up @@ -1158,7 +1158,7 @@ impl<'a, 'gcx, 'tcx> Layout {
Univariant { variant: st, non_zero: false }
}

ty::TyTuple(tys) => {
ty::TyTuple(tys, _) => {
// FIXME(camlorn): if we ever allow unsized tuples, this needs to be checked.
// See the univariant case below to learn how.
let st = Struct::new(dl,
Expand Down
23 changes: 20 additions & 3 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,17 @@ impl AssociatedItem {
AssociatedKind::Type => Def::AssociatedTy(self.def_id),
}
}

/// Tests whether the associated item admits a non-trivial implementation
/// for !
pub fn relevant_for_never<'tcx>(&self) -> bool {
match self.kind {
AssociatedKind::Const => true,
AssociatedKind::Type => true,
// FIXME(canndrew): Be more thorough here, check if any argument is uninhabited.
AssociatedKind::Method => !self.method_has_self_argument,
}
}
}

#[derive(Clone, Debug, PartialEq, Eq, Copy, RustcEncodable, RustcDecodable)]
Expand Down Expand Up @@ -1603,7 +1614,7 @@ impl<'a, 'gcx, 'tcx> AdtDef {
_ if tys.references_error() => tcx.types.err,
0 => tcx.types.bool,
1 => tys[0],
_ => tcx.intern_tup(&tys[..])
_ => tcx.intern_tup(&tys[..], false)
};

let old = tcx.adt_sized_constraint.borrow().get(&self.did).cloned();
Expand Down Expand Up @@ -1638,7 +1649,7 @@ impl<'a, 'gcx, 'tcx> AdtDef {
vec![ty]
}

TyTuple(ref tys) => {
TyTuple(ref tys, _) => {
match tys.last() {
None => vec![],
Some(ty) => self.sized_constraint_for_ty(tcx, stack, ty)
Expand All @@ -1652,7 +1663,7 @@ impl<'a, 'gcx, 'tcx> AdtDef {
.subst(tcx, substs);
debug!("sized_constraint_for_ty({:?}) intermediate = {:?}",
ty, adt_ty);
if let ty::TyTuple(ref tys) = adt_ty.sty {
if let ty::TyTuple(ref tys, _) = adt_ty.sty {
tys.iter().flat_map(|ty| {
self.sized_constraint_for_ty(tcx, stack, ty)
}).collect()
Expand Down Expand Up @@ -2010,6 +2021,12 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}
}

pub fn trait_relevant_for_never(self, did: DefId) -> bool {
self.associated_items(did).any(|item| {
item.relevant_for_never()
})
}

pub fn custom_coerce_unsized_kind(self, did: DefId) -> adjustment::CustomCoerceUnsized {
self.custom_coerce_unsized_kinds.memoize(did, || {
let (kind, src) = if did.krate != LOCAL_CRATE {
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,10 +447,11 @@ pub fn super_relate_tys<'a, 'gcx, 'tcx, R>(relation: &mut R,
Ok(tcx.mk_slice(t))
}

(&ty::TyTuple(as_), &ty::TyTuple(bs)) =>
(&ty::TyTuple(as_, a_defaulted), &ty::TyTuple(bs, b_defaulted)) =>
{
if as_.len() == bs.len() {
Ok(tcx.mk_tup(as_.iter().zip(bs).map(|(a, b)| relation.relate(a, b)))?)
let defaulted = a_defaulted || b_defaulted;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I wonder if this should be a_defaulted && b_defaulted? My thinking is this: if we are relating two () values, and one of them came directly from the user, then that type will continue to be () even when ! fallback is changed. Probably it doesn't matter because in such a case the fallback won't actually trigger anyhow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I couldn't see when it would ever matter so I decided to err on the side of creating more warnings.

Ok(tcx.mk_tup(as_.iter().zip(bs).map(|(a, b)| relation.relate(a, b)), defaulted)?)
} else if !(as_.is_empty() || bs.is_empty()) {
Err(TypeError::TupleSize(
expected_found(relation, &as_.len(), &bs.len())))
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ impl<'tcx> TypeFoldable<'tcx> for Ty<'tcx> {
ty::TyAdt(tid, substs) => ty::TyAdt(tid, substs.fold_with(folder)),
ty::TyDynamic(ref trait_ty, ref region) =>
ty::TyDynamic(trait_ty.fold_with(folder), region.fold_with(folder)),
ty::TyTuple(ts) => ty::TyTuple(ts.fold_with(folder)),
ty::TyTuple(ts, defaulted) => ty::TyTuple(ts.fold_with(folder), defaulted),
ty::TyFnDef(def_id, substs, f) => {
ty::TyFnDef(def_id,
substs.fold_with(folder),
Expand Down Expand Up @@ -511,7 +511,7 @@ impl<'tcx> TypeFoldable<'tcx> for Ty<'tcx> {
ty::TyAdt(_, substs) => substs.visit_with(visitor),
ty::TyDynamic(ref trait_ty, ref reg) =>
trait_ty.visit_with(visitor) || reg.visit_with(visitor),
ty::TyTuple(ts) => ts.visit_with(visitor),
ty::TyTuple(ts, _) => ts.visit_with(visitor),
ty::TyFnDef(_, substs, ref f) => {
substs.visit_with(visitor) || f.visit_with(visitor)
}
Expand Down
Loading