Skip to content

Commit

Permalink
Auto merge of #48557 - matthewjasper:allow-trvial-bounds, r=nikomatsakis
Browse files Browse the repository at this point in the history
Implement RFC 2056 trivial constraints in where clauses

This is an implementation of the new behaviour for #48214. Tests are mostly updated to show the effects of this. Feature gate hasn't been added yet.

Some things that are worth noting and are maybe not want we want

* `&mut T: Copy` doesn't allow as much as someone might expect because there is often an implicit reborrow.
* ~There isn't a check that a where clause is well-formed any more, so `where Vec<str>: Debug` is now allowed (without a `str: Sized` bound).~

r? @nikomatsakis
  • Loading branch information
bors committed May 16, 2018
2 parents 3ec2058 + be2900c commit 448cc57
Show file tree
Hide file tree
Showing 38 changed files with 1,084 additions and 65 deletions.
8 changes: 8 additions & 0 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
ObligationCauseCode::ReturnType(_) |
ObligationCauseCode::BlockTailExpression(_) => (),
ObligationCauseCode::TrivialBound => {
err.help("see issue #48214");
if tcx.sess.opts.unstable_features.is_nightly_build() {
err.help("add #![feature(trivial_bounds)] to the \
crate attributes to enable",
);
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ fn process_predicate<'a, 'gcx, 'tcx>(
ty::Predicate::Trait(ref data) => {
let trait_obligation = obligation.with(data.clone());

if data.is_global() {
if data.is_global() && !data.has_late_bound_regions() {
// no type variables present, can use evaluation for better caching.
// FIXME: consider caching errors too.
if selcx.infcx().predicate_must_hold(&obligation) {
Expand Down
12 changes: 3 additions & 9 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,9 @@ pub enum ObligationCauseCode<'tcx> {

/// Block implicit return
BlockTailExpression(ast::NodeId),

/// #[feature(trivial_bounds)] is not enabled
TrivialBound,
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -641,17 +644,8 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

let predicates: Vec<_> =
util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.to_vec())
.filter(|p| !p.is_global()) // (*)
.collect();

// (*) Any predicate like `i32: Trait<u32>` or whatever doesn't
// need to be in the *environment* to be proven, so screen those
// out. This is important for the soundness of inter-fn
// caching. Note though that we should probably check that these
// predicates hold at the point where the environment is
// constructed, but I am not currently doing so out of laziness.
// -nmatsakis

debug!("normalize_param_env_or_error: elaborated-predicates={:?}",
predicates);

Expand Down
65 changes: 28 additions & 37 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,6 @@ enum BuiltinImplConditions<'tcx> {
/// There is no built-in impl. There may be some other
/// candidate (a where-clause or user-defined impl).
None,
/// There is *no* impl for this, builtin or not. Ignore
/// all where-clauses.
Never,
/// It is unknown whether there is an impl.
Ambiguous
}
Expand Down Expand Up @@ -781,13 +778,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
mut obligation: TraitObligation<'tcx>)
-> Result<EvaluationResult, OverflowError>
{
debug!("evaluate_trait_predicate_recursively({:?})",
obligation);
debug!("evaluate_trait_predicate_recursively({:?})", obligation);

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).
if self.intercrate.is_none() && obligation.is_global()
&& obligation.param_env.caller_bounds.iter().all(|bound| bound.needs_subst()) {
// If a param env has no global bounds, 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.
debug!("evaluate_trait_predicate_recursively({:?}) - in global", obligation);
obligation.param_env = obligation.param_env.without_caller_bounds();
}
Expand Down Expand Up @@ -1451,22 +1448,22 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
let sized_conditions = self.sized_conditions(obligation);
self.assemble_builtin_bound_candidates(sized_conditions,
&mut candidates)?;
} else if lang_items.unsize_trait() == Some(def_id) {
self.assemble_candidates_for_unsizing(obligation, &mut candidates);
} else {
if lang_items.clone_trait() == Some(def_id) {
// Same builtin conditions as `Copy`, i.e. every type which has builtin support
// for `Copy` also has builtin support for `Clone`, + tuples and arrays of `Clone`
// types have builtin support for `Clone`.
let clone_conditions = self.copy_clone_conditions(obligation);
self.assemble_builtin_bound_candidates(clone_conditions, &mut candidates)?;
}

self.assemble_generator_candidates(obligation, &mut candidates)?;
self.assemble_closure_candidates(obligation, &mut candidates)?;
self.assemble_fn_pointer_candidates(obligation, &mut candidates)?;
self.assemble_candidates_from_impls(obligation, &mut candidates)?;
self.assemble_candidates_from_object_ty(obligation, &mut candidates);
} else if lang_items.unsize_trait() == Some(def_id) {
self.assemble_candidates_for_unsizing(obligation, &mut candidates);
} else {
if lang_items.clone_trait() == Some(def_id) {
// Same builtin conditions as `Copy`, i.e. every type which has builtin support
// for `Copy` also has builtin support for `Clone`, + tuples and arrays of `Clone`
// types have builtin support for `Clone`.
let clone_conditions = self.copy_clone_conditions(obligation);
self.assemble_builtin_bound_candidates(clone_conditions, &mut candidates)?;
}

self.assemble_generator_candidates(obligation, &mut candidates)?;
self.assemble_closure_candidates(obligation, &mut candidates)?;
self.assemble_fn_pointer_candidates(obligation, &mut candidates)?;
self.assemble_candidates_from_impls(obligation, &mut candidates)?;
self.assemble_candidates_from_object_ty(obligation, &mut candidates);
}

self.assemble_candidates_from_projected_tys(obligation, &mut candidates);
Expand Down Expand Up @@ -2081,13 +2078,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// BUILTIN BOUNDS
//
// These cover the traits that are built-in to the language
// itself. This includes `Copy` and `Sized` for sure. For the
// moment, it also includes `Send` / `Sync` and a few others, but
// those will hopefully change to library-defined traits in the
// future.
// itself: `Copy`, `Clone` and `Sized`.

// HACK: if this returns an error, selection exits without considering
// other impls.
fn assemble_builtin_bound_candidates<'o>(&mut self,
conditions: BuiltinImplConditions<'tcx>,
candidates: &mut SelectionCandidateSet<'tcx>)
Expand All @@ -2106,14 +2098,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("assemble_builtin_bound_candidates: ambiguous builtin");
Ok(candidates.ambiguous = true)
}
BuiltinImplConditions::Never => { Err(Unimplemented) }
}
}

fn sized_conditions(&mut self, obligation: &TraitObligation<'tcx>)
-> BuiltinImplConditions<'tcx>
{
use self::BuiltinImplConditions::{Ambiguous, None, Never, Where};
use self::BuiltinImplConditions::{Ambiguous, None, Where};

// NOTE: binder moved to (*)
let self_ty = self.infcx.shallow_resolve(
Expand All @@ -2130,7 +2121,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
Where(ty::Binder::dummy(Vec::new()))
}

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

ty::TyTuple(tys) => {
Where(ty::Binder::bind(tys.last().into_iter().cloned().collect()))
Expand Down Expand Up @@ -2164,7 +2155,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
let self_ty = self.infcx.shallow_resolve(
obligation.predicate.skip_binder().self_ty());

use self::BuiltinImplConditions::{Ambiguous, None, Never, Where};
use self::BuiltinImplConditions::{Ambiguous, None, Where};

match self_ty.sty {
ty::TyInfer(ty::IntVar(_)) | ty::TyInfer(ty::FloatVar(_)) |
Expand All @@ -2182,7 +2173,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
ty::TyDynamic(..) | ty::TyStr | ty::TySlice(..) |
ty::TyGenerator(..) | ty::TyGeneratorWitness(..) | ty::TyForeign(..) |
ty::TyRef(_, _, hir::MutMutable) => {
Never
None
}

ty::TyArray(element_ty, _) => {
Expand All @@ -2202,7 +2193,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
if is_copy_trait || is_clone_trait {
Where(ty::Binder::bind(substs.upvar_tys(def_id, self.tcx()).collect()))
} else {
Never
None
}
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::IntrinsicType => Some(super::IntrinsicType),
super::MethodReceiver => Some(super::MethodReceiver),
super::BlockTailExpression(id) => Some(super::BlockTailExpression(id)),
super::TrivialBound => Some(super::TrivialBound),
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl FlagComputation {
}

&ty::TyParam(ref p) => {
self.add_flags(TypeFlags::HAS_LOCAL_NAMES);
self.add_flags(TypeFlags::HAS_FREE_LOCAL_NAMES);
if p.is_self() {
self.add_flags(TypeFlags::HAS_SELF);
} else {
Expand All @@ -89,7 +89,7 @@ impl FlagComputation {

&ty::TyGenerator(_, ref substs, _) => {
self.add_flags(TypeFlags::HAS_TY_CLOSURE);
self.add_flags(TypeFlags::HAS_LOCAL_NAMES);
self.add_flags(TypeFlags::HAS_FREE_LOCAL_NAMES);
self.add_substs(&substs.substs);
}

Expand All @@ -101,12 +101,12 @@ impl FlagComputation {

&ty::TyClosure(_, ref substs) => {
self.add_flags(TypeFlags::HAS_TY_CLOSURE);
self.add_flags(TypeFlags::HAS_LOCAL_NAMES);
self.add_flags(TypeFlags::HAS_FREE_LOCAL_NAMES);
self.add_substs(&substs.substs);
}

&ty::TyInfer(infer) => {
self.add_flags(TypeFlags::HAS_LOCAL_NAMES); // it might, right?
self.add_flags(TypeFlags::HAS_FREE_LOCAL_NAMES); // it might, right?
self.add_flags(TypeFlags::HAS_TY_INFER);
match infer {
ty::FreshTy(_) |
Expand Down
10 changes: 7 additions & 3 deletions src/librustc/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,14 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {

/// Indicates whether this value references only 'global'
/// types/lifetimes that are the same regardless of what fn we are
/// in. This is used for caching. Errs on the side of returning
/// false.
/// in. This is used for caching.
fn is_global(&self) -> bool {
!self.has_type_flags(TypeFlags::HAS_LOCAL_NAMES)
!self.has_type_flags(TypeFlags::HAS_FREE_LOCAL_NAMES)
}

/// True if there are any late-bound regions
fn has_late_bound_regions(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_RE_LATE_BOUND)
}
}

Expand Down
11 changes: 8 additions & 3 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ bitflags! {

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

// Present if the type belongs in a local type context.
// Only set for TyInfer other than Fresh.
Expand All @@ -455,6 +455,10 @@ bitflags! {
// ought to be true only for the results of canonicalization.
const HAS_CANONICAL_VARS = 1 << 13;

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

const NEEDS_SUBST = TypeFlags::HAS_PARAMS.bits |
TypeFlags::HAS_SELF.bits |
TypeFlags::HAS_RE_EARLY_BOUND.bits;
Expand All @@ -472,9 +476,10 @@ bitflags! {
TypeFlags::HAS_TY_ERR.bits |
TypeFlags::HAS_PROJECTION.bits |
TypeFlags::HAS_TY_CLOSURE.bits |
TypeFlags::HAS_LOCAL_NAMES.bits |
TypeFlags::HAS_FREE_LOCAL_NAMES.bits |
TypeFlags::KEEP_IN_LOCAL_TCX.bits |
TypeFlags::HAS_CANONICAL_VARS.bits;
TypeFlags::HAS_CANONICAL_VARS.bits |
TypeFlags::HAS_RE_LATE_BOUND.bits;
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1268,7 +1268,9 @@ impl RegionKind {
flags = flags | TypeFlags::HAS_FREE_REGIONS;
flags = flags | TypeFlags::HAS_RE_SKOL;
}
ty::ReLateBound(..) => { }
ty::ReLateBound(..) => {
flags = flags | TypeFlags::HAS_RE_LATE_BOUND;
}
ty::ReEarlyBound(..) => {
flags = flags | TypeFlags::HAS_FREE_REGIONS;
flags = flags | TypeFlags::HAS_RE_EARLY_BOUND;
Expand All @@ -1291,8 +1293,8 @@ impl RegionKind {
}

match *self {
ty::ReStatic | ty::ReEmpty | ty::ReErased => (),
_ => flags = flags | TypeFlags::HAS_LOCAL_NAMES,
ty::ReStatic | ty::ReEmpty | ty::ReErased | ty::ReLateBound(..) => (),
_ => flags = flags | TypeFlags::HAS_FREE_LOCAL_NAMES,
}

debug!("type_flags({:?}) = {:?}", self, flags);
Expand Down
58 changes: 58 additions & 0 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1591,3 +1591,61 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExternCrate {
self.0 += 1;
}
}

/// Lint for trait and lifetime bounds that don't depend on type parameters
/// which either do nothing, or stop the item from being used.
pub struct TrivialConstraints;

declare_lint! {
TRIVIAL_BOUNDS,
Warn,
"these bounds don't depend on an type parameters"
}

impl LintPass for TrivialConstraints {
fn get_lints(&self) -> LintArray {
lint_array!(TRIVIAL_BOUNDS)
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TrivialConstraints {
fn check_item(
&mut self,
cx: &LateContext<'a, 'tcx>,
item: &'tcx hir::Item,
) {
use rustc::ty::fold::TypeFoldable;
use rustc::ty::Predicate::*;


if cx.tcx.features().trivial_bounds {
let def_id = cx.tcx.hir.local_def_id(item.id);
let predicates = cx.tcx.predicates_of(def_id);
for predicate in &predicates.predicates {
let predicate_kind_name = match *predicate {
Trait(..) => "Trait",
TypeOutlives(..) |
RegionOutlives(..) => "Lifetime",

// Ignore projections, as they can only be global
// if the trait bound is global
Projection(..) |
// Ignore bounds that a user can't type
WellFormed(..) |
ObjectSafe(..) |
ClosureKind(..) |
Subtype(..) |
ConstEvaluatable(..) => continue,
};
if predicate.is_global() {
cx.span_lint(
TRIVIAL_BOUNDS,
item.span,
&format!("{} bound {} does not depend on any type \
or lifetime parameters", predicate_kind_name, predicate),
);
}
}
}
}
}
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
UnreachablePub,
TypeAliasBounds,
UnusedBrokenConst,
TrivialConstraints,
);

add_builtin_with_new!(sess,
Expand Down
Loading

0 comments on commit 448cc57

Please sign in to comment.