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 15 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
5 changes: 3 additions & 2 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,7 @@ impl<'tcx> Deref for TyCtxt<'tcx> {
}

pub struct GlobalCtxt<'tcx> {
pub arena: WorkerLocal<Arena<'tcx>>,
pub arena: &'tcx WorkerLocal<Arena<'tcx>>,

interners: CtxtInterners<'tcx>,

Expand Down Expand Up @@ -1170,6 +1170,7 @@ impl<'tcx> TyCtxt<'tcx> {
local_providers: ty::query::Providers<'tcx>,
extern_providers: ty::query::Providers<'tcx>,
arenas: &'tcx AllArenas,
local_arena: &'tcx WorkerLocal<Arena<'tcx>>,
Zoxc marked this conversation as resolved.
Show resolved Hide resolved
resolutions: ty::ResolverOutputs,
hir: hir_map::Map<'tcx>,
on_disk_query_result_cache: query::OnDiskCache<'tcx>,
Expand Down Expand Up @@ -1225,7 +1226,7 @@ impl<'tcx> TyCtxt<'tcx> {
sess: s,
lint_store,
cstore,
arena: WorkerLocal::new(|_| Arena::default()),
arena: local_arena,
interners,
dep_graph,
prof: s.prof.clone(),
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
Loading