Skip to content

Commit

Permalink
Auto merge of #42015 - nikomatsakis:chalk-trait-env-2, r=eddyb
Browse files Browse the repository at this point in the history
remove interior mutability of type-flags

We were previously using the flags on `Ty<'tcx>` instances to do some ad-hoc caching schemes around things like `is_sized()`, `is_freeze()`, and `moves_by_default()`. This PR replaces those schemes with a proper query; the query key is based on the pair of a `(ParameterEnvironment<'tcx>, Ty<'tcx>)` pair. This is also intended to be a preliminary template for what trait-selection and projection will eventually look like.

I did some performance measurements. In the past, I observed a noticeable speedup (6%) for building rustc, but since I've rebased, the numbers appear to be more of a wash:

| Crate | Before | After | Percentage |
| --- | --- | --- | -- |
| syntax | 167s | 166s | 0.6% faster |
| rustc | 376s | 382s | 1.5% slower |

Some advantages of this new scheme:

- `is_sized` etc are proper queries
- we get caching across generic fns, so long as trait environment is identical
- dependency tracking is correct
  • Loading branch information
bors committed May 23, 2017
2 parents 852b7cb + 83641a9 commit 9fa25a7
Show file tree
Hide file tree
Showing 37 changed files with 427 additions and 491 deletions.
10 changes: 10 additions & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ pub enum DepNode<D: Clone + Debug> {
SymbolName(D),
SpecializationGraph(D),
ObjectSafety(D),
IsCopy(D),
IsSized(D),
IsFreeze(D),
NeedsDrop(D),

// The set of impls for a given trait. Ultimately, it would be
// nice to get more fine-grained here (e.g., to include a
Expand Down Expand Up @@ -159,6 +163,7 @@ pub enum DepNode<D: Clone + Debug> {
// not a hotspot.
ProjectionCache { def_ids: Vec<D> },

ParamEnv(D),
DescribeDef(D),
DefSpan(D),
Stability(D),
Expand Down Expand Up @@ -233,6 +238,10 @@ impl<D: Clone + Debug> DepNode<D> {
// they are always absolute.
WorkProduct(ref id) => Some(WorkProduct(id.clone())),

IsCopy(ref d) => op(d).map(IsCopy),
IsSized(ref d) => op(d).map(IsSized),
IsFreeze(ref d) => op(d).map(IsFreeze),
NeedsDrop(ref d) => op(d).map(NeedsDrop),
Hir(ref d) => op(d).map(Hir),
HirBody(ref d) => op(d).map(HirBody),
MetaData(ref d) => op(d).map(MetaData),
Expand Down Expand Up @@ -284,6 +293,7 @@ impl<D: Clone + Debug> DepNode<D> {
let def_ids: Option<Vec<E>> = def_ids.iter().map(op).collect();
def_ids.map(|d| ProjectionCache { def_ids: d })
}
ParamEnv(ref d) => op(d).map(ParamEnv),
DescribeDef(ref d) => op(d).map(DescribeDef),
DefSpan(ref d) => op(d).map(DefSpan),
Stability(ref d) => op(d).map(Stability),
Expand Down
40 changes: 19 additions & 21 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
// For region variables.
region_vars: RegionVarBindings<'a, 'gcx, 'tcx>,

pub parameter_environment: ty::ParameterEnvironment<'gcx>,
pub param_env: ty::ParamEnv<'gcx>,

/// Caches the results of trait selection. This cache is used
/// for things that have to do with the parameters in scope.
Expand Down Expand Up @@ -406,41 +406,41 @@ pub trait InferEnv<'a, 'tcx> {
fn to_parts(self, tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>);
Option<ty::ParamEnv<'tcx>>);
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for () {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
Option<ty::ParamEnv<'tcx>>) {
(None, None, None)
}
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for ty::ParameterEnvironment<'tcx> {
impl<'a, 'tcx> InferEnv<'a, 'tcx> for ty::ParamEnv<'tcx> {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
Option<ty::ParamEnv<'tcx>>) {
(None, None, Some(self))
}
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for (&'a ty::TypeckTables<'tcx>, ty::ParameterEnvironment<'tcx>) {
impl<'a, 'tcx> InferEnv<'a, 'tcx> for (&'a ty::TypeckTables<'tcx>, ty::ParamEnv<'tcx>) {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
Option<ty::ParamEnv<'tcx>>) {
(Some(self.0), None, Some(self.1))
}
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for (ty::TypeckTables<'tcx>, ty::ParameterEnvironment<'tcx>) {
impl<'a, 'tcx> InferEnv<'a, 'tcx> for (ty::TypeckTables<'tcx>, ty::ParamEnv<'tcx>) {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
Option<ty::ParamEnv<'tcx>>) {
(None, Some(self.0), Some(self.1))
}
}
Expand All @@ -449,11 +449,11 @@ impl<'a, 'tcx> InferEnv<'a, 'tcx> for hir::BodyId {
fn to_parts(self, tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
Option<ty::ParamEnv<'tcx>>) {
let def_id = tcx.hir.body_owner_def_id(self);
(Some(tcx.typeck_tables_of(def_id)),
None,
Some(tcx.parameter_environment(def_id)))
Some(tcx.param_env(def_id)))
}
}

Expand All @@ -465,7 +465,7 @@ pub struct InferCtxtBuilder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
arena: DroplessArena,
fresh_tables: Option<RefCell<ty::TypeckTables<'tcx>>>,
tables: Option<&'a ty::TypeckTables<'gcx>>,
param_env: Option<ty::ParameterEnvironment<'gcx>>,
param_env: Option<ty::ParamEnv<'gcx>>,
projection_mode: Reveal,
}

Expand Down Expand Up @@ -498,7 +498,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> {
int_unification_table: RefCell::new(UnificationTable::new()),
float_unification_table: RefCell::new(UnificationTable::new()),
region_vars: RegionVarBindings::new(self),
parameter_environment: param_env.unwrap(),
param_env: param_env.unwrap(),
selection_cache: traits::SelectionCache::new(),
evaluation_cache: traits::EvaluationCache::new(),
projection_cache: RefCell::new(traits::ProjectionCache::new()),
Expand Down Expand Up @@ -526,9 +526,7 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
let tables = tables.map(InferTables::Interned).unwrap_or_else(|| {
fresh_tables.as_ref().map_or(InferTables::Missing, InferTables::InProgress)
});
let param_env = param_env.take().unwrap_or_else(|| {
global_tcx.empty_parameter_environment()
});
let param_env = param_env.take().unwrap_or_else(|| ty::ParamEnv::empty());
global_tcx.enter_local(arena, |tcx| f(InferCtxt {
tcx: tcx,
tables: tables,
Expand All @@ -537,7 +535,7 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
int_unification_table: RefCell::new(UnificationTable::new()),
float_unification_table: RefCell::new(UnificationTable::new()),
region_vars: RegionVarBindings::new(tcx),
parameter_environment: param_env,
param_env: param_env,
selection_cache: traits::SelectionCache::new(),
evaluation_cache: traits::EvaluationCache::new(),
reported_trait_errors: RefCell::new(FxHashSet()),
Expand Down Expand Up @@ -650,7 +648,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {
}

pub fn normalize_associated_type_in_env<T>(
self, value: &T, env: &'a ty::ParameterEnvironment<'tcx>
self, value: &T, env: ty::ParamEnv<'tcx>
) -> T
where T: TransNormalize<'tcx>
{
Expand All @@ -662,7 +660,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {
return value;
}

self.infer_ctxt(env.clone(), Reveal::All).enter(|infcx| {
self.infer_ctxt(env, Reveal::All).enter(|infcx| {
value.trans_normalize(&infcx)
})
}
Expand Down Expand Up @@ -1674,8 +1672,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tables.borrow().upvar_capture_map.get(&upvar_id).cloned()
}

pub fn param_env(&self) -> &ty::ParameterEnvironment<'gcx> {
&self.parameter_environment
pub fn param_env(&self) -> ty::ParamEnv<'gcx> {
self.param_env
}

pub fn closure_kind(&self,
Expand Down
20 changes: 10 additions & 10 deletions src/librustc/traits/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -418,16 +418,16 @@ before, and hence the cache lookup would succeed, yielding
One subtle interaction is that the results of trait lookup will vary
depending on what where clauses are in scope. Therefore, we actually
have *two* caches, a local and a global cache. The local cache is
attached to the `ParameterEnvironment` and the global cache attached
to the `tcx`. We use the local cache whenever the result might depend
on the where clauses that are in scope. The determination of which
cache to use is done by the method `pick_candidate_cache` in
`select.rs`. At the moment, we use a very simple, conservative rule:
if there are any where-clauses in scope, then we use the local cache.
We used to try and draw finer-grained distinctions, but that led to a
serious of annoying and weird bugs like #22019 and #18290. This simple
rule seems to be pretty clearly safe and also still retains a very
high hit rate (~95% when compiling rustc).
attached to the `ParamEnv` and the global cache attached to the
`tcx`. We use the local cache whenever the result might depend on the
where clauses that are in scope. The determination of which cache to
use is done by the method `pick_candidate_cache` in `select.rs`. At
the moment, we use a very simple, conservative rule: if there are any
where-clauses in scope, then we use the local cache. We used to try
and draw finer-grained distinctions, but that led to a serious of
annoying and weird bugs like #22019 and #18290. This simple rule seems
to be pretty clearly safe and also still retains a very high hit rate
(~95% when compiling rustc).

# Specialization

Expand Down
18 changes: 9 additions & 9 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,9 +437,9 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
/// Normalizes the parameter environment, reporting errors if they occur.
pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
region_context: DefId,
unnormalized_env: ty::ParameterEnvironment<'tcx>,
unnormalized_env: ty::ParamEnv<'tcx>,
cause: ObligationCause<'tcx>)
-> ty::ParameterEnvironment<'tcx>
-> ty::ParamEnv<'tcx>
{
// I'm not wild about reporting errors here; I'd prefer to
// have the errors get reported at a defined place (e.g.,
Expand Down Expand Up @@ -477,15 +477,15 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
debug!("normalize_param_env_or_error: elaborated-predicates={:?}",
predicates);

let elaborated_env = unnormalized_env.with_caller_bounds(tcx.intern_predicates(&predicates));
let elaborated_env = ty::ParamEnv::new(tcx.intern_predicates(&predicates));

tcx.infer_ctxt(elaborated_env, Reveal::UserFacing).enter(|infcx| {
let predicates = match fully_normalize(
&infcx, cause,
// You would really want to pass infcx.parameter_environment.caller_bounds here,
// You would really want to pass infcx.param_env.caller_bounds here,
// but that is an interned slice, and fully_normalize takes &T and returns T, so
// without further refactoring, a slice can't be used. Luckily, we still have the
// predicate vector from which we created the ParameterEnvironment in infcx, so we
// predicate vector from which we created the ParamEnv in infcx, so we
// can pass that instead. It's roundabout and a bit brittle, but this code path
// ought to be refactored anyway, and until then it saves us from having to copy.
&predicates,
Expand All @@ -494,7 +494,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
Err(errors) => {
infcx.report_fulfillment_errors(&errors);
// An unnormalized env is better than nothing.
return infcx.parameter_environment;
return infcx.param_env;
}
};

Expand All @@ -516,19 +516,19 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
// all things considered.
tcx.sess.span_err(span, &fixup_err.to_string());
// An unnormalized env is better than nothing.
return infcx.parameter_environment;
return infcx.param_env;
}
};

let predicates = match tcx.lift_to_global(&predicates) {
Some(predicates) => predicates,
None => return infcx.parameter_environment
None => return infcx.param_env
};

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

infcx.parameter_environment.with_caller_bounds(tcx.intern_predicates(&predicates))
ty::ParamEnv::new(tcx.intern_predicates(&predicates))
})
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
self.infcx.tcx
}

pub fn param_env(&self) -> &'cx ty::ParameterEnvironment<'gcx> {
pub fn param_env(&self) -> ty::ParamEnv<'gcx> {
self.infcx.param_env()
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub fn specializes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}

// create a parameter environment corresponding to a (skolemized) instantiation of impl1
let penv = tcx.parameter_environment(impl1_def_id);
let penv = tcx.param_env(impl1_def_id);
let impl1_trait_ref = tcx.impl_trait_ref(impl1_def_id).unwrap();

// Create a infcx, taking the predicates of impl1 as assumptions:
Expand Down Expand Up @@ -250,7 +250,7 @@ fn fulfill_implication<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
source_trait_ref,
target_trait_ref,
errors,
infcx.parameter_environment.caller_bounds);
infcx.param_env.caller_bounds);
Err(())
}

Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
let flags = super::flags::FlagComputation::for_sty(&st);
let ty_struct = TyS {
sty: st,
flags: Cell::new(flags.flags),
flags: flags.flags,
region_depth: flags.depth,
};

Expand Down Expand Up @@ -978,8 +978,8 @@ macro_rules! sty_debug_print {
ty::TyError => /* unimportant */ continue,
$(ty::$variant(..) => &mut $variant,)*
};
let region = t.flags.get().intersects(ty::TypeFlags::HAS_RE_INFER);
let ty = t.flags.get().intersects(ty::TypeFlags::HAS_TY_INFER);
let region = t.flags.intersects(ty::TypeFlags::HAS_RE_INFER);
let ty = t.flags.intersects(ty::TypeFlags::HAS_TY_INFER);

variant.total += 1;
total.total += 1;
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 @@ -167,7 +167,7 @@ impl FlagComputation {
}

fn add_ty(&mut self, ty: Ty) {
self.add_flags(ty.flags.get());
self.add_flags(ty.flags);
self.add_depth(ty.region_depth);
}

Expand Down
5 changes: 2 additions & 3 deletions src/librustc/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,9 +625,8 @@ struct HasTypeFlagsVisitor {

impl<'tcx> TypeVisitor<'tcx> for HasTypeFlagsVisitor {
fn visit_ty(&mut self, t: Ty) -> bool {
let flags = t.flags.get();
debug!("HasTypeFlagsVisitor: t={:?} t.flags={:?} self.flags={:?}", t, flags, self.flags);
flags.intersects(self.flags)
debug!("HasTypeFlagsVisitor: t={:?} t.flags={:?} self.flags={:?}", t, t.flags, self.flags);
t.flags.intersects(self.flags)
}

fn visit_region(&mut self, r: ty::Region<'tcx>) -> bool {
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ impl<'a, 'gcx, 'tcx> Layout {
let ptr_layout = |pointee: Ty<'gcx>| {
let non_zero = !ty.is_unsafe_ptr();
let pointee = infcx.normalize_projections(pointee);
if pointee.is_sized(tcx, &infcx.parameter_environment, DUMMY_SP) {
if pointee.is_sized(tcx, infcx.param_env, DUMMY_SP) {
Ok(Scalar { value: Pointer, non_zero: non_zero })
} else {
let unsized_part = tcx.struct_tail(pointee);
Expand Down Expand Up @@ -1268,11 +1268,11 @@ impl<'a, 'gcx, 'tcx> Layout {
let kind = if def.is_enum() || def.variants[0].fields.len() == 0{
StructKind::AlwaysSizedUnivariant
} else {
let param_env = tcx.parameter_environment(def.did);
let param_env = tcx.param_env(def.did);
let fields = &def.variants[0].fields;
let last_field = &fields[fields.len()-1];
let always_sized = tcx.type_of(last_field.did)
.is_sized(tcx, &param_env, DUMMY_SP);
.is_sized(tcx, param_env, DUMMY_SP);
if !always_sized { StructKind::MaybeUnsizedUnivariant }
else { StructKind::AlwaysSizedUnivariant }
};
Expand Down
Loading

0 comments on commit 9fa25a7

Please sign in to comment.