Skip to content

Commit

Permalink
Rollup merge of rust-lang#132410 - bjorn3:yet_another_driver_refactor…
Browse files Browse the repository at this point in the history
…_round, r=cjgillot

Some more refactorings towards removing driver queries

Follow up to rust-lang#127184

## Custom driver breaking change

The `after_analysis` callback is changed to accept `TyCtxt` instead of `Queries`. The only safe query in `Queries` to call at this point is `global_ctxt()` which allows you to enter the `TyCtxt` either way. To fix your custom driver, replace the `queries: &'tcx Queries<'tcx>` argument with `tcx: TyCtxt<'tcx>` and remove your `queries.global_ctxt().unwrap().enter(|tcx| { ... })` call and only keep the contents of the closure.

## Custom driver deprecation

The `after_crate_root_parsing` callback is now deprecated. Several custom drivers are incorrectly calling `queries.global_ctxt()` from inside of it, which causes some driver code to be skipped. As such I would like to either remove it in the future or if custom drivers still need it, change it to accept an `&rustc_ast::Crate` instead.
  • Loading branch information
matthiaskrgr authored Nov 27, 2024
2 parents 6b6a867 + dc65c63 commit af1ca15
Show file tree
Hide file tree
Showing 33 changed files with 227 additions and 195 deletions.
108 changes: 51 additions & 57 deletions compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ pub trait Callbacks {
/// Called after parsing the crate root. Submodules are not yet parsed when
/// this callback is called. Return value instructs the compiler whether to
/// continue the compilation afterwards (defaults to `Compilation::Continue`)
#[deprecated = "This callback will likely be removed or stop giving access \
to the TyCtxt in the future. Use either the after_expansion \
or the after_analysis callback instead."]
fn after_crate_root_parsing<'tcx>(
&mut self,
_compiler: &interface::Compiler,
Expand All @@ -181,7 +184,7 @@ pub trait Callbacks {
fn after_analysis<'tcx>(
&mut self,
_compiler: &interface::Compiler,
_queries: &'tcx Queries<'tcx>,
_tcx: TyCtxt<'tcx>,
) -> Compilation {
Compilation::Continue
}
Expand Down Expand Up @@ -335,19 +338,12 @@ fn run_compiler(
expanded_args: args,
};

let has_input = match make_input(&default_early_dcx, &matches.free) {
Err(reported) => return Err(reported),
Ok(Some(input)) => {
let has_input = match make_input(&default_early_dcx, &matches.free)? {
Some(input) => {
config.input = input;
true // has input: normal compilation
}
Ok(None) => match matches.free.as_slice() {
[] => false, // no input: we will exit early
[_] => panic!("make_input should have provided valid inputs"),
[fst, snd, ..] => default_early_dcx.early_fatal(format!(
"multiple input filenames provided (first two filenames are `{fst}` and `{snd}`)"
)),
},
None => false, // no input: we will exit early
};

drop(default_early_dcx);
Expand Down Expand Up @@ -405,10 +401,6 @@ fn run_compiler(
queries.global_ctxt()?.enter(|tcx| {
tcx.ensure().early_lint_checks(());
pretty::print(sess, pp_mode, pretty::PrintExtra::NeedsAstMap { tcx });
Ok(())
})?;

queries.global_ctxt()?.enter(|tcx| {
passes::write_dep_info(tcx);
});
} else {
Expand All @@ -421,6 +413,7 @@ fn run_compiler(
return early_exit();
}

#[allow(deprecated)]
if callbacks.after_crate_root_parsing(compiler, queries) == Compilation::Stop {
return early_exit();
}
Expand All @@ -442,25 +435,23 @@ fn run_compiler(

queries.global_ctxt()?.enter(|tcx| {
passes::write_dep_info(tcx);
});

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

if sess.opts.unstable_opts.no_analysis {
return early_exit();
}
if sess.opts.unstable_opts.no_analysis {
return early_exit();
}

queries.global_ctxt()?.enter(|tcx| tcx.analysis(()))?;
tcx.analysis(())?;

if callbacks.after_analysis(compiler, queries) == Compilation::Stop {
return early_exit();
}
if callbacks.after_analysis(compiler, tcx) == Compilation::Stop {
return early_exit();
}

queries.global_ctxt()?.enter(|tcx| {
Ok(Some(Linker::codegen_and_build_linker(tcx, &*compiler.codegen_backend)?))
})
})?;
Expand Down Expand Up @@ -509,37 +500,40 @@ fn make_input(
early_dcx: &EarlyDiagCtxt,
free_matches: &[String],
) -> Result<Option<Input>, ErrorGuaranteed> {
let [input_file] = free_matches else { return Ok(None) };

if input_file != "-" {
// Normal `Input::File`
return Ok(Some(Input::File(PathBuf::from(input_file))));
}

// read from stdin as `Input::Str`
let mut input = String::new();
if io::stdin().read_to_string(&mut input).is_err() {
// Immediately stop compilation if there was an issue reading
// the input (for example if the input stream is not UTF-8).
let reported =
early_dcx.early_err("couldn't read from stdin, as it did not contain valid UTF-8");
return Err(reported);
}
match free_matches {
[] => Ok(None), // no input: we will exit early,
[ifile] if ifile == "-" => {
// read from stdin as `Input::Str`
let mut input = String::new();
if io::stdin().read_to_string(&mut input).is_err() {
// Immediately stop compilation if there was an issue reading
// the input (for example if the input stream is not UTF-8).
let reported = early_dcx
.early_err("couldn't read from stdin, as it did not contain valid UTF-8");
return Err(reported);
}

let name = match env::var("UNSTABLE_RUSTDOC_TEST_PATH") {
Ok(path) => {
let line = env::var("UNSTABLE_RUSTDOC_TEST_LINE").expect(
"when UNSTABLE_RUSTDOC_TEST_PATH is set \
let name = match env::var("UNSTABLE_RUSTDOC_TEST_PATH") {
Ok(path) => {
let line = env::var("UNSTABLE_RUSTDOC_TEST_LINE").expect(
"when UNSTABLE_RUSTDOC_TEST_PATH is set \
UNSTABLE_RUSTDOC_TEST_LINE also needs to be set",
);
let line = isize::from_str_radix(&line, 10)
.expect("UNSTABLE_RUSTDOC_TEST_LINE needs to be an number");
FileName::doc_test_source_code(PathBuf::from(path), line)
}
Err(_) => FileName::anon_source_code(&input),
};
);
let line = isize::from_str_radix(&line, 10)
.expect("UNSTABLE_RUSTDOC_TEST_LINE needs to be an number");
FileName::doc_test_source_code(PathBuf::from(path), line)
}
Err(_) => FileName::anon_source_code(&input),
};

Ok(Some(Input::Str { name, input }))
Ok(Some(Input::Str { name, input }))
}
[ifile] => Ok(Some(Input::File(PathBuf::from(ifile)))),
[ifile1, ifile2, ..] => early_dcx.early_fatal(format!(
"multiple input filenames provided (first two filenames are `{}` and `{}`)",
ifile1, ifile2
)),
}
}

/// Whether to stop or continue compilation.
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_incremental/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@ mod persist;

pub use persist::{
LoadResult, copy_cgu_workproduct_to_incr_comp_cache_dir, finalize_session_directory,
in_incr_comp_dir, in_incr_comp_dir_sess, load_query_result_cache, save_dep_graph,
save_work_product_index, setup_dep_graph,
in_incr_comp_dir, in_incr_comp_dir_sess, load_query_result_cache, save_work_product_index,
setup_dep_graph,
};
use rustc_middle::util::Providers;

#[allow(missing_docs)]
pub fn provide(providers: &mut Providers) {
providers.hooks.save_dep_graph =
|tcx| tcx.sess.time("serialize_dep_graph", || persist::save_dep_graph(tcx.tcx));
}

rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
3 changes: 2 additions & 1 deletion compiler/rustc_incremental/src/persist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ mod work_product;

pub use fs::{finalize_session_directory, in_incr_comp_dir, in_incr_comp_dir_sess};
pub use load::{LoadResult, load_query_result_cache, setup_dep_graph};
pub use save::{save_dep_graph, save_work_product_index};
pub(crate) use save::save_dep_graph;
pub use save::save_work_product_index;
pub use work_product::copy_cgu_workproduct_to_incr_comp_cache_dir;
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/persist/save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::errors;
///
/// This function should only run after all queries have completed.
/// Trying to execute a query afterwards would attempt to read the result cache we just dropped.
pub fn save_dep_graph(tcx: TyCtxt<'_>) {
pub(crate) fn save_dep_graph(tcx: TyCtxt<'_>) {
debug!("save_dep_graph()");
tcx.dep_graph.with_ignore(|| {
let sess = tcx.sess;
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,10 +689,12 @@ pub static DEFAULT_QUERY_PROVIDERS: LazyLock<Providers> = LazyLock::new(|| {
rustc_const_eval::provide(providers);
rustc_middle::hir::provide(providers);
rustc_borrowck::provide(providers);
rustc_incremental::provide(providers);
rustc_mir_build::provide(providers);
rustc_mir_transform::provide(providers);
rustc_monomorphize::provide(providers);
rustc_privacy::provide(providers);
rustc_query_impl::provide(providers);
rustc_resolve::provide(providers);
rustc_hir_analysis::provide(providers);
rustc_hir_typeck::provide(providers);
Expand Down
32 changes: 7 additions & 25 deletions compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@ use rustc_hir::def_id::LOCAL_CRATE;
use rustc_middle::arena::Arena;
use rustc_middle::dep_graph::DepGraph;
use rustc_middle::ty::{GlobalCtxt, TyCtxt};
use rustc_serialize::opaque::FileEncodeResult;
use rustc_session::Session;
use rustc_session::config::{self, OutputFilenames, OutputType};

use crate::errors::FailedWritingFile;
use crate::interface::{Compiler, Result};
use crate::{errors, passes};
use crate::passes;

/// Represent the result of a query.
///
Expand Down Expand Up @@ -62,7 +61,7 @@ impl<'a, T> std::ops::DerefMut for QueryResult<'a, T> {

impl<'a, 'tcx> QueryResult<'a, &'tcx GlobalCtxt<'tcx>> {
pub fn enter<T>(&mut self, f: impl FnOnce(TyCtxt<'tcx>) -> T) -> T {
(*self.0).get_mut().enter(f)
(*self.0).borrow().enter(f)
}
}

Expand Down Expand Up @@ -90,8 +89,10 @@ impl<'tcx> Queries<'tcx> {
}
}

pub fn finish(&self) -> FileEncodeResult {
if let Some(gcx) = self.gcx_cell.get() { gcx.finish() } else { Ok(0) }
pub fn finish(&'tcx self) {
if let Some(gcx) = self.gcx_cell.get() {
gcx.finish();
}
}

pub fn parse(&self) -> Result<QueryResult<'_, ast::Crate>> {
Expand Down Expand Up @@ -209,29 +210,10 @@ impl Compiler {
let queries = Queries::new(self);
let ret = f(&queries);

// NOTE: intentionally does not compute the global context if it hasn't been built yet,
// since that likely means there was a parse error.
if let Some(Ok(gcx)) = &mut *queries.gcx.result.borrow_mut() {
let gcx = gcx.get_mut();
// We assume that no queries are run past here. If there are new queries
// after this point, they'll show up as "<unknown>" in self-profiling data.
{
let _prof_timer =
queries.compiler.sess.prof.generic_activity("self_profile_alloc_query_strings");
gcx.enter(rustc_query_impl::alloc_self_profile_query_strings);
}

self.sess.time("serialize_dep_graph", || gcx.enter(rustc_incremental::save_dep_graph));

gcx.enter(rustc_query_impl::query_key_hash_verify_all);
}

// The timer's lifetime spans the dropping of `queries`, which contains
// the global context.
_timer = self.sess.timer("free_global_ctxt");
if let Err((path, error)) = queries.finish() {
self.sess.dcx().emit_fatal(errors::FailedWritingFile { path: &path, error });
}
queries.finish();

ret
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ middle_drop_check_overflow =
middle_erroneous_constant = erroneous constant encountered
middle_failed_writing_file =
failed to write file {$path}: {$error}"
middle_layout_references_error =
the type has an unknown layout
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_middle/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::fmt;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::{fmt, io};

use rustc_errors::codes::*;
use rustc_errors::{DiagArgName, DiagArgValue, DiagMessage};
Expand All @@ -18,6 +18,13 @@ pub struct DropCheckOverflow<'tcx> {
pub overflow_ty: Ty<'tcx>,
}

#[derive(Diagnostic)]
#[diag(middle_failed_writing_file)]
pub struct FailedWritingFile<'a> {
pub path: &'a Path,
pub error: io::Error,
}

#[derive(Diagnostic)]
#[diag(middle_opaque_hidden_type_mismatch)]
pub struct OpaqueHiddenTypeMismatch<'tcx> {
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_middle/src/hooks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ declare_hooks! {
/// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we
/// can just link to the upstream crate and therefore don't need a mono item.
hook should_codegen_locally(instance: crate::ty::Instance<'tcx>) -> bool;

hook alloc_self_profile_query_strings() -> ();

/// Saves and writes the DepGraph to the file system.
///
/// This function saves both the dep-graph and the query result cache,
/// and drops the result cache.
///
/// This function should only run after all queries have completed.
/// Trying to execute a query afterwards would attempt to read the result cache we just dropped.
hook save_dep_graph() -> ();

hook query_key_hash_verify_all() -> ();
}

#[cold]
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1371,8 +1371,17 @@ impl<'tcx> GlobalCtxt<'tcx> {
tls::enter_context(&icx, || f(icx.tcx))
}

pub fn finish(&self) -> FileEncodeResult {
self.dep_graph.finish_encoding()
pub fn finish(&'tcx self) {
// We assume that no queries are run past here. If there are new queries
// after this point, they'll show up as "<unknown>" in self-profiling data.
self.enter(|tcx| tcx.alloc_self_profile_query_strings());

self.enter(|tcx| tcx.save_dep_graph());
self.enter(|tcx| tcx.query_key_hash_verify_all());

if let Err((path, error)) = self.dep_graph.finish_encoding() {
self.sess.dcx().emit_fatal(crate::error::FailedWritingFile { path: &path, error });
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_query_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,9 @@ pub fn query_system<'tcx>(
}

rustc_middle::rustc_query_append! { define_queries! }

pub fn provide(providers: &mut rustc_middle::util::Providers) {
providers.hooks.alloc_self_profile_query_strings =
|tcx| alloc_self_profile_query_strings(tcx.tcx);
providers.hooks.query_key_hash_verify_all = |tcx| query_key_hash_verify_all(tcx.tcx);
}
2 changes: 2 additions & 0 deletions compiler/rustc_query_impl/src/profiling_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ pub fn alloc_self_profile_query_strings(tcx: TyCtxt<'_>) {
return;
}

let _prof_timer = tcx.sess.prof.generic_activity("self_profile_alloc_query_strings");

let mut string_cache = QueryKeyStringCache::new();

for alloc in super::ALLOC_SELF_PROFILE_QUERY_STRINGS.iter() {
Expand Down
Loading

0 comments on commit af1ca15

Please sign in to comment.