Skip to content

Make the 'a lifetime on TyCtxt useless #56601

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

Merged
merged 2 commits into from
Dec 19, 2018
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
9 changes: 7 additions & 2 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use ty::error::{ExpectedFound, TypeError, UnconstrainedNumeric};
use ty::fold::TypeFoldable;
use ty::relate::RelateResult;
use ty::subst::{Kind, Substs};
use ty::{self, GenericParamDefKind, Ty, TyCtxt};
use ty::{self, GenericParamDefKind, Ty, TyCtxt, CtxtInterners};
use ty::{FloatVid, IntVid, TyVid};
use util::nodemap::FxHashMap;

Expand Down Expand Up @@ -471,6 +471,7 @@ impl fmt::Display for FixupError {
pub struct InferCtxtBuilder<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
global_tcx: TyCtxt<'a, 'gcx, 'gcx>,
arena: SyncDroplessArena,
interners: Option<CtxtInterners<'tcx>>,
fresh_tables: Option<RefCell<ty::TypeckTables<'tcx>>>,
}

Expand All @@ -479,6 +480,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> {
InferCtxtBuilder {
global_tcx: self,
arena: SyncDroplessArena::default(),
interners: None,
fresh_tables: None,
}
}
Expand Down Expand Up @@ -519,10 +521,13 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
let InferCtxtBuilder {
global_tcx,
ref arena,
ref mut interners,
ref fresh_tables,
} = *self;
let in_progress_tables = fresh_tables.as_ref();
global_tcx.enter_local(arena, |tcx| {
// Check that we haven't entered before
assert!(interners.is_none());
global_tcx.enter_local(arena, interners, |tcx| {
f(InferCtxt {
tcx,
in_progress_tables,
Expand Down
55 changes: 35 additions & 20 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ use std::ops::{Deref, Bound};
use std::iter;
use std::sync::mpsc;
use std::sync::Arc;
use std::marker::PhantomData;
use rustc_target::spec::abi;
use syntax::ast::{self, NodeId};
use syntax::attr;
Expand All @@ -86,13 +87,15 @@ use hir;
pub struct AllArenas<'tcx> {
pub global: WorkerLocal<GlobalArenas<'tcx>>,
pub interner: SyncDroplessArena,
global_ctxt: Option<GlobalCtxt<'tcx>>,
}

impl<'tcx> AllArenas<'tcx> {
pub fn new() -> Self {
AllArenas {
global: WorkerLocal::new(|_| GlobalArenas::default()),
interner: SyncDroplessArena::default(),
global_ctxt: None,
}
}
}
Expand Down Expand Up @@ -869,12 +872,13 @@ pub struct FreeRegionInfo {
/// [rustc guide]: https://rust-lang.github.io/rustc-guide/ty.html
#[derive(Copy, Clone)]
pub struct TyCtxt<'a, 'gcx: 'tcx, 'tcx: 'a> {
gcx: &'a GlobalCtxt<'gcx>,
interners: &'a CtxtInterners<'tcx>
gcx: &'gcx GlobalCtxt<'gcx>,
interners: &'tcx CtxtInterners<'tcx>,
dummy: PhantomData<&'a ()>,
}

impl<'a, 'gcx, 'tcx> Deref for TyCtxt<'a, 'gcx, 'tcx> {
type Target = &'a GlobalCtxt<'gcx>;
impl<'gcx> Deref for TyCtxt<'_, 'gcx, '_> {
type Target = &'gcx GlobalCtxt<'gcx>;
#[inline(always)]
fn deref(&self) -> &Self::Target {
&self.gcx
Expand Down Expand Up @@ -964,10 +968,11 @@ pub struct GlobalCtxt<'tcx> {
impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
/// Get the global TyCtxt.
#[inline]
pub fn global_tcx(self) -> TyCtxt<'a, 'gcx, 'gcx> {
pub fn global_tcx(self) -> TyCtxt<'gcx, 'gcx, 'gcx> {
TyCtxt {
gcx: self.gcx,
interners: &self.gcx.global_interners,
dummy: PhantomData,
}
}

Expand Down Expand Up @@ -1105,7 +1110,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
cstore: &'tcx CrateStoreDyn,
local_providers: ty::query::Providers<'tcx>,
extern_providers: ty::query::Providers<'tcx>,
arenas: &'tcx AllArenas<'tcx>,
arenas: &'tcx mut AllArenas<'tcx>,
resolutions: ty::Resolutions,
hir: hir_map::Map<'tcx>,
on_disk_query_result_cache: query::OnDiskCache<'tcx>,
Expand Down Expand Up @@ -1166,7 +1171,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
Lrc::new(StableVec::new(v)));
}

let gcx = &GlobalCtxt {
arenas.global_ctxt = Some(GlobalCtxt {
sess: s,
cstore,
global_arenas: &arenas.global,
Expand Down Expand Up @@ -1209,7 +1214,9 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
alloc_map: Lock::new(interpret::AllocMap::new()),
tx_to_llvm_workers: Lock::new(tx),
output_filenames: Arc::new(output_filenames.clone()),
};
});

let gcx = arenas.global_ctxt.as_ref().unwrap();

sync::assert_send_val(&gcx);

Expand Down Expand Up @@ -1609,20 +1616,25 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {
}
}

impl<'gcx: 'tcx, 'tcx> GlobalCtxt<'gcx> {
impl<'gcx> GlobalCtxt<'gcx> {
/// Call the closure with a local `TyCtxt` using the given arena.
Copy link
Contributor

@nikomatsakis nikomatsakis Dec 11, 2018

Choose a reason for hiding this comment

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

Can we add a comment here:


The interners parameter is a reference to a storage slot with 'tcx lifetime that we can use. It is expected to be None on entry and will be reset to None on return. This allows us to create a &'tcx reference to the interners while the tcx is active.


that presumes, of course, that you accept my proposal above =)

pub fn enter_local<F, R>(
&self,
/// `interners` is a slot passed so we can create a CtxtInterners
/// with the same lifetime as `arena`.
pub fn enter_local<'tcx, F, R>(
&'gcx self,
arena: &'tcx SyncDroplessArena,
interners: &'tcx mut Option<CtxtInterners<'tcx>>,
f: F
) -> R
where
F: for<'a> FnOnce(TyCtxt<'a, 'gcx, 'tcx>) -> R
F: FnOnce(TyCtxt<'tcx, 'gcx, 'tcx>) -> R,
'gcx: 'tcx,
{
let interners = CtxtInterners::new(arena);
*interners = Some(CtxtInterners::new(&arena));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should assert that this is None and set it back to None before we return, then?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it hardly matters, it's just the interning maps.

let tcx = TyCtxt {
gcx: self,
interners: &interners,
interners: interners.as_ref().unwrap(),
dummy: PhantomData,
};
ty::tls::with_related_context(tcx.global_tcx(), |icx| {
let new_icx = ty::tls::ImplicitCtxt {
Expand All @@ -1631,8 +1643,8 @@ impl<'gcx: 'tcx, 'tcx> GlobalCtxt<'gcx> {
layout_depth: icx.layout_depth,
task: icx.task,
};
ty::tls::enter_context(&new_icx, |new_icx| {
f(new_icx.tcx)
ty::tls::enter_context(&new_icx, |_| {
f(tcx)
})
})
}
Expand Down Expand Up @@ -1872,6 +1884,7 @@ pub mod tls {

use std::fmt;
use std::mem;
use std::marker::PhantomData;
use syntax_pos;
use ty::query;
use errors::{Diagnostic, TRACK_DIAGNOSTICS};
Expand All @@ -1891,10 +1904,10 @@ pub mod tls {
/// you should also have access to an ImplicitCtxt through the functions
/// in this module.
#[derive(Clone)]
pub struct ImplicitCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
pub struct ImplicitCtxt<'a, 'gcx: 'tcx, 'tcx> {
/// The current TyCtxt. Initially created by `enter_global` and updated
/// by `enter_local` with a new local interner
pub tcx: TyCtxt<'a, 'gcx, 'tcx>,
pub tcx: TyCtxt<'tcx, 'gcx, 'tcx>,

/// The current query job, if any. This is updated by start_job in
/// ty::query::plumbing when executing a query
Expand Down Expand Up @@ -2008,8 +2021,8 @@ pub mod tls {
/// creating a initial TyCtxt and ImplicitCtxt.
/// This happens once per rustc session and TyCtxts only exists
/// inside the `f` function.
pub fn enter_global<'gcx, F, R>(gcx: &GlobalCtxt<'gcx>, f: F) -> R
where F: for<'a> FnOnce(TyCtxt<'a, 'gcx, 'gcx>) -> R
pub fn enter_global<'gcx, F, R>(gcx: &'gcx GlobalCtxt<'gcx>, f: F) -> R
where F: FnOnce(TyCtxt<'gcx, 'gcx, 'gcx>) -> R
{
with_thread_locals(|| {
// Update GCX_PTR to indicate there's a GlobalCtxt available
Expand All @@ -2024,6 +2037,7 @@ pub mod tls {
let tcx = TyCtxt {
gcx,
interners: &gcx.global_interners,
dummy: PhantomData,
};
let icx = ImplicitCtxt {
tcx,
Expand Down Expand Up @@ -2053,6 +2067,7 @@ pub mod tls {
let tcx = TyCtxt {
gcx,
interners: &gcx.global_interners,
dummy: PhantomData,
};
let icx = ImplicitCtxt {
query: None,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub use self::binding::BindingMode;
pub use self::binding::BindingMode::*;

pub use self::context::{TyCtxt, FreeRegionInfo, GlobalArenas, AllArenas, tls, keep_local};
pub use self::context::{Lift, TypeckTables};
pub use self::context::{Lift, TypeckTables, CtxtInterners};

pub use self::instance::{Instance, InstanceDef};

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> {
let r = tls::with_related_context(tcx, move |current_icx| {
// Update the ImplicitCtxt to point to our new query job
let new_icx = tls::ImplicitCtxt {
tcx,
tcx: tcx.global_tcx(),
query: Some(self.job.clone()),
layout_depth: current_icx.layout_depth,
task: current_icx.task,
Expand Down
13 changes: 4 additions & 9 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,6 @@ pub fn compile_input(
}
}

let arenas = AllArenas::new();

// Construct the HIR map
let hir_map = time(sess, "indexing hir", || {
hir_map::map_crate(sess, cstore, &mut hir_forest, &defs)
Expand All @@ -263,7 +261,6 @@ pub fn compile_input(
sess,
outdir,
output,
&arenas,
&cstore,
&hir_map,
&analysis,
Expand All @@ -284,6 +281,8 @@ pub fn compile_input(
None
};

let mut arenas = AllArenas::new();

phase_3_run_analysis_passes(
&*codegen_backend,
control,
Expand All @@ -292,7 +291,7 @@ pub fn compile_input(
hir_map,
analysis,
resolutions,
&arenas,
&mut arenas,
&crate_name,
&outputs,
|tcx, analysis, rx, result| {
Expand Down Expand Up @@ -533,7 +532,6 @@ pub struct CompileState<'a, 'tcx: 'a> {
pub output_filenames: Option<&'a OutputFilenames>,
pub out_dir: Option<&'a Path>,
pub out_file: Option<&'a Path>,
pub arenas: Option<&'tcx AllArenas<'tcx>>,
pub expanded_crate: Option<&'a ast::Crate>,
pub hir_crate: Option<&'a hir::Crate>,
pub hir_map: Option<&'a hir_map::Map<'tcx>>,
Expand All @@ -549,7 +547,6 @@ impl<'a, 'tcx> CompileState<'a, 'tcx> {
session,
out_dir: out_dir.as_ref().map(|s| &**s),
out_file: None,
arenas: None,
krate: None,
registry: None,
cstore: None,
Expand Down Expand Up @@ -605,7 +602,6 @@ impl<'a, 'tcx> CompileState<'a, 'tcx> {
session: &'tcx Session,
out_dir: &'a Option<PathBuf>,
out_file: &'a Option<PathBuf>,
arenas: &'tcx AllArenas<'tcx>,
cstore: &'tcx CStore,
hir_map: &'a hir_map::Map<'tcx>,
analysis: &'a ty::CrateAnalysis,
Expand All @@ -617,7 +613,6 @@ impl<'a, 'tcx> CompileState<'a, 'tcx> {
) -> Self {
CompileState {
crate_name: Some(crate_name),
arenas: Some(arenas),
cstore: Some(cstore),
hir_map: Some(hir_map),
analysis: Some(analysis),
Expand Down Expand Up @@ -1216,7 +1211,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(
hir_map: hir_map::Map<'tcx>,
mut analysis: ty::CrateAnalysis,
resolutions: Resolutions,
arenas: &'tcx AllArenas<'tcx>,
arenas: &'tcx mut AllArenas<'tcx>,
name: &str,
output_filenames: &OutputFilenames,
f: F,
Expand Down
1 change: 0 additions & 1 deletion src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,6 @@ impl<'a> CompilerCalls<'a> for RustcDefaultCalls {
&state.expanded_crate.take().unwrap(),
state.crate_name.unwrap(),
ppm,
state.arenas.unwrap(),
state.output_filenames.unwrap(),
opt_uii.clone(),
state.out_file);
Expand Down
Loading