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

Handle GlobalCtxt directly from librustc_interface query system #66791

Merged
merged 20 commits into from
Nov 30, 2019
Merged
Show file tree
Hide file tree
Changes from 13 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
10 changes: 5 additions & 5 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ pub struct Map<'hir> {

map: HirEntryMap<'hir>,

definitions: &'hir Definitions,
definitions: Definitions,

/// The reverse mapping of `node_to_hir_id`.
hir_to_node_id: FxHashMap<HirId, NodeId>,
Expand Down Expand Up @@ -267,8 +267,8 @@ impl<'hir> Map<'hir> {
}

#[inline]
pub fn definitions(&self) -> &'hir Definitions {
self.definitions
pub fn definitions(&self) -> &Definitions {
&self.definitions
}

pub fn def_key(&self, def_id: DefId) -> DefKey {
Expand Down Expand Up @@ -1251,7 +1251,7 @@ impl Named for ImplItem { fn name(&self) -> Name { self.ident.name } }
pub fn map_crate<'hir>(sess: &crate::session::Session,
cstore: &CrateStoreDyn,
forest: &'hir Forest,
definitions: &'hir Definitions)
definitions: Definitions)
-> Map<'hir> {
let _prof_timer = sess.prof.generic_activity("build_hir_map");

Expand All @@ -1260,7 +1260,7 @@ pub fn map_crate<'hir>(sess: &crate::session::Session,
.map(|(node_id, &hir_id)| (hir_id, node_id)).collect();

let (map, crate_hash) = {
let hcx = crate::ich::StableHashingContext::new(sess, &forest.krate, definitions, cstore);
let hcx = crate::ich::StableHashingContext::new(sess, &forest.krate, &definitions, cstore);

let mut collector = NodeCollector::root(sess,
&forest.krate,
Expand Down
191 changes: 99 additions & 92 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,120 +283,127 @@ pub fn run_compiler(
return sess.compile_status();
}

compiler.parse()?;

if let Some(ppm) = &sess.opts.pretty {
if ppm.needs_ast_map() {
compiler.global_ctxt()?.peek_mut().enter(|tcx| {
let expanded_crate = compiler.expansion()?.take().0;
pretty::print_after_hir_lowering(
tcx,
compiler.input(),
&expanded_crate,
let linker = compiler.enter(|queries| {
let early_exit = || sess.compile_status().map(|_| None);
queries.parse()?;

if let Some(ppm) = &sess.opts.pretty {
if ppm.needs_ast_map() {
queries.global_ctxt()?.peek_mut().enter(|tcx| {
let expanded_crate = queries.expansion()?.take().0;
pretty::print_after_hir_lowering(
tcx,
compiler.input(),
&expanded_crate,
*ppm,
compiler.output_file().as_ref().map(|p| &**p),
);
Ok(())
})?;
} else {
let krate = queries.parse()?.take();
pretty::print_after_parsing(
sess,
&compiler.input(),
&krate,
*ppm,
compiler.output_file().as_ref().map(|p| &**p),
);
Ok(())
})?;
} else {
let krate = compiler.parse()?.take();
pretty::print_after_parsing(
sess,
&compiler.input(),
&krate,
*ppm,
compiler.output_file().as_ref().map(|p| &**p),
);
}
return early_exit();
}
return sess.compile_status();
}

if callbacks.after_parsing(compiler) == Compilation::Stop {
return sess.compile_status();
}
if callbacks.after_parsing(compiler) == Compilation::Stop {
return early_exit();
}

if sess.opts.debugging_opts.parse_only ||
sess.opts.debugging_opts.show_span.is_some() ||
sess.opts.debugging_opts.ast_json_noexpand {
return sess.compile_status();
}
if sess.opts.debugging_opts.parse_only ||
sess.opts.debugging_opts.show_span.is_some() ||
sess.opts.debugging_opts.ast_json_noexpand {
return early_exit();
}

{
let (_, lint_store) = &*compiler.register_plugins()?.peek();
{
let (_, lint_store) = &*queries.register_plugins()?.peek();

// Lint plugins are registered; now we can process command line flags.
if sess.opts.describe_lints {
describe_lints(&sess, &lint_store, true);
return sess.compile_status();
// Lint plugins are registered; now we can process command line flags.
if sess.opts.describe_lints {
describe_lints(&sess, &lint_store, true);
return early_exit();
}
}
}

compiler.expansion()?;
if callbacks.after_expansion(compiler) == Compilation::Stop {
return sess.compile_status();
}
queries.expansion()?;
if callbacks.after_expansion(compiler) == Compilation::Stop {
return early_exit();
}

compiler.prepare_outputs()?;
queries.prepare_outputs()?;

if sess.opts.output_types.contains_key(&OutputType::DepInfo)
&& sess.opts.output_types.len() == 1
{
return sess.compile_status();
}
if sess.opts.output_types.contains_key(&OutputType::DepInfo)
&& sess.opts.output_types.len() == 1
{
return early_exit();
}

compiler.global_ctxt()?;
queries.global_ctxt()?;

if sess.opts.debugging_opts.no_analysis ||
sess.opts.debugging_opts.ast_json {
return sess.compile_status();
}
if sess.opts.debugging_opts.no_analysis ||
sess.opts.debugging_opts.ast_json {
return early_exit();
}

if sess.opts.debugging_opts.save_analysis {
let expanded_crate = &compiler.expansion()?.peek().0;
let crate_name = compiler.crate_name()?.peek().clone();
compiler.global_ctxt()?.peek_mut().enter(|tcx| {
let result = tcx.analysis(LOCAL_CRATE);

time(sess, "save analysis", || {
save::process_crate(
tcx,
&expanded_crate,
&crate_name,
&compiler.input(),
None,
DumpHandler::new(compiler.output_dir().as_ref().map(|p| &**p), &crate_name)
)
});

result
// AST will be dropped *after* the `after_analysis` callback
// (needed by the RLS)
})?;
} else {
// Drop AST after creating GlobalCtxt to free memory
mem::drop(compiler.expansion()?.take());
}
if sess.opts.debugging_opts.save_analysis {
let expanded_crate = &queries.expansion()?.peek().0;
let crate_name = queries.crate_name()?.peek().clone();
queries.global_ctxt()?.peek_mut().enter(|tcx| {
let result = tcx.analysis(LOCAL_CRATE);

time(sess, "save analysis", || {
save::process_crate(
tcx,
&expanded_crate,
&crate_name,
&compiler.input(),
None,
DumpHandler::new(
compiler.output_dir().as_ref().map(|p| &**p), &crate_name
)
)
});

compiler.global_ctxt()?.peek_mut().enter(|tcx| tcx.analysis(LOCAL_CRATE))?;
result
// AST will be dropped *after* the `after_analysis` callback
// (needed by the RLS)
})?;
} else {
// Drop AST after creating GlobalCtxt to free memory
mem::drop(queries.expansion()?.take());
}

if callbacks.after_analysis(compiler) == Compilation::Stop {
return sess.compile_status();
}
queries.global_ctxt()?.peek_mut().enter(|tcx| tcx.analysis(LOCAL_CRATE))?;

if sess.opts.debugging_opts.save_analysis {
mem::drop(compiler.expansion()?.take());
}
if callbacks.after_analysis(compiler) == Compilation::Stop {
return early_exit();
}
Comment on lines +386 to +388
Copy link
Member

Choose a reason for hiding this comment

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

Is it maybe a problem that now, after_analysis is called inside compiler.enter, so that if the callback needs access to the ctx and calls compiler.enter itself, we end up with nested compiler.enter?

Copy link
Contributor

Choose a reason for hiding this comment

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

That is indeed the problem. The Callbacks are supposed to take &'tcx Queries<'tcx> now to avoid that. So that has to be fixed before Miri can work again. Do you or @cjgillot want to fix that?

Copy link
Contributor

Choose a reason for hiding this comment

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

There should probably be an assertion to avoid calling enter twice too.

Copy link
Member

Choose a reason for hiding this comment

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

I prepared a PR at #66896 (and tested that this indeed is sufficient to fix Miri).

There should probably be an assertion to avoid calling enter twice too.

I didn't do that as I wouldn't know how.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay. Do you still need me to do something?


compiler.ongoing_codegen()?;
if sess.opts.debugging_opts.save_analysis {
mem::drop(queries.expansion()?.take());
}

// Drop GlobalCtxt after starting codegen to free memory
mem::drop(compiler.global_ctxt()?.take());
queries.ongoing_codegen()?;

if sess.opts.debugging_opts.print_type_sizes {
sess.code_stats.print_type_sizes();
}
if sess.opts.debugging_opts.print_type_sizes {
sess.code_stats.print_type_sizes();
}

compiler.link()?;
let linker = queries.linker()?;
Ok(Some(linker))
})?;

if let Some(linker) = linker {
linker.link()?
}

if sess.opts.debugging_opts.perf_stats {
sess.print_perf_stats();
Expand Down
3 changes: 0 additions & 3 deletions src/librustc_interface/interface.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::queries::Queries;
use crate::util;
pub use crate::passes::BoxedResolver;

Expand Down Expand Up @@ -36,7 +35,6 @@ pub struct Compiler {
pub(crate) input_path: Option<PathBuf>,
pub(crate) output_dir: Option<PathBuf>,
pub(crate) output_file: Option<PathBuf>,
pub(crate) queries: Queries,
pub(crate) crate_name: Option<String>,
pub(crate) register_lints: Option<Box<dyn Fn(&Session, &mut lint::LintStore) + Send + Sync>>,
pub(crate) override_queries:
Expand Down Expand Up @@ -169,7 +167,6 @@ pub fn run_compiler_in_existing_thread_pool<R>(
input_path: config.input_path,
output_dir: config.output_dir,
output_file: config.output_file,
queries: Default::default(),
crate_name: config.crate_name,
register_lints: config.register_lints,
override_queries: config.override_queries,
Expand Down
56 changes: 21 additions & 35 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc_codegen_ssa::back::link::emit_metadata;
use rustc_codegen_utils::codegen_backend::CodegenBackend;
use rustc_codegen_utils::link::filename_for_metadata;
use rustc_data_structures::{box_region_allow_access, declare_box_region_type, parallel};
use rustc_data_structures::sync::{Lrc, ParallelIterator, par_iter};
use rustc_data_structures::sync::{Lrc, Once, ParallelIterator, par_iter};
use rustc_errors::PResult;
use rustc_incremental;
use rustc_metadata::cstore;
Expand Down Expand Up @@ -740,44 +740,41 @@ pub fn default_provide_extern(providers: &mut ty::query::Providers<'_>) {
rustc_codegen_ssa::provide_extern(providers);
}

declare_box_region_type!(
Copy link
Member

Choose a reason for hiding this comment

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

Can this macro now be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it's still used for the resolver.

pub BoxedGlobalCtxt,
for('tcx),
(&'tcx GlobalCtxt<'tcx>) -> ((), ())
);
pub struct BoxedGlobalCtxt<'tcx>(&'tcx GlobalCtxt<'tcx>);
Zoxc marked this conversation as resolved.
Show resolved Hide resolved

impl BoxedGlobalCtxt {
impl<'gcx> BoxedGlobalCtxt<'gcx> {
Zoxc marked this conversation as resolved.
Show resolved Hide resolved
pub fn enter<F, R>(&mut self, f: F) -> R
where
F: for<'tcx> FnOnce(TyCtxt<'tcx>) -> R,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could just be F: FnOnce(TyCtxt<'gcx>) -> R, now.

{
self.access(|gcx| ty::tls::enter_global(gcx, |tcx| f(tcx)))
ty::tls::enter_global(self.0, |tcx| f(tcx))
}

pub fn print_stats(&self) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just remove this function and let its caller call print_stats directly using enter.

self.0.queries.print_stats()
}
}

pub fn create_global_ctxt(
compiler: &Compiler,
pub fn create_global_ctxt<'gcx>(
Zoxc marked this conversation as resolved.
Show resolved Hide resolved
compiler: &'gcx Compiler,
lint_store: Lrc<lint::LintStore>,
mut hir_forest: hir::map::Forest,
hir_forest: &'gcx hir::map::Forest,
mut resolver_outputs: ResolverOutputs,
outputs: OutputFilenames,
crate_name: &str,
) -> BoxedGlobalCtxt {
let sess = compiler.session().clone();
global_ctxt: &'gcx Once<GlobalCtxt<'gcx>>,
arenas: &'gcx Once<AllArenas>,
) -> BoxedGlobalCtxt<'gcx> {
let sess = &compiler.session();
let codegen_backend = compiler.codegen_backend().clone();
let crate_name = crate_name.to_string();
let defs = mem::take(&mut resolver_outputs.definitions);
let override_queries = compiler.override_queries;

let ((), result) = BoxedGlobalCtxt::new(static move || {
let sess = &*sess;

let global_ctxt: Option<GlobalCtxt<'_>>;
let arenas = AllArenas::new();
let arenas = arenas.init_locking(|| AllArenas::new());
Zoxc marked this conversation as resolved.
Show resolved Hide resolved

// Construct the HIR map.
let hir_map = time(sess, "indexing HIR", || {
hir::map::map_crate(sess, &*resolver_outputs.cstore, &mut hir_forest, &defs)
hir::map::map_crate(sess, &*resolver_outputs.cstore, &hir_forest, defs)
});

let query_result_on_disk_cache = time(sess, "load query result cache", || {
Expand All @@ -796,7 +793,7 @@ pub fn create_global_ctxt(
callback(sess, &mut local_providers, &mut extern_providers);
}

let gcx = TyCtxt::create_global_ctxt(
let gcx = global_ctxt.init_locking(move || TyCtxt::create_global_ctxt(
sess,
lint_store,
local_providers,
Expand All @@ -807,26 +804,15 @@ pub fn create_global_ctxt(
query_result_on_disk_cache,
&crate_name,
&outputs
);
));

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

ty::tls::enter_global(gcx, |tcx| {
ty::tls::enter_global(&gcx, |tcx| {
// Do some initialization of the DepGraph that can only be done with the
// tcx available.
time(tcx.sess, "dep graph tcx init", || rustc_incremental::dep_graph_tcx_init(tcx));
});

yield BoxedGlobalCtxt::initial_yield(());
box_region_allow_access!(for('tcx), (&'tcx GlobalCtxt<'tcx>), (gcx));

if sess.opts.debugging_opts.query_stats {
gcx.queries.print_stats();
}
});

result
BoxedGlobalCtxt(gcx)
}

/// Runs the resolution, type-checking, region checking and other
Expand Down
Loading