Auto merge of #85885 - bjorn3:remove_box_region, r=cjgillot

Don't use a generator for BoxedResolver

The generator is non-trivial and requires unsafe code anyway. Using regular unsafe code without a generator is much easier to follow.

Based on #85810 as it touches rustc_interface too.
This commit is contained in:
bors 2021-06-11 16:11:20 +00:00
commit 0a8629bff6
6 changed files with 91 additions and 215 deletions

View File

@ -1,169 +0,0 @@
//! This module provides a way to deal with self-referential data.
//!
//! The main idea is to allocate such data in a generator frame and then
//! give access to it by executing user-provided closures inside that generator.
//! The module provides a safe abstraction for the latter task.
//!
//! The interface consists of two exported macros meant to be used together:
//! * `declare_box_region_type` wraps a generator inside a struct with `access`
//! method which accepts closures.
//! * `box_region_allow_access` is a helper which should be called inside
//! a generator to actually execute those closures.
use std::marker::PhantomData;
use std::ops::{Generator, GeneratorState};
use std::pin::Pin;
#[derive(Copy, Clone)]
pub struct AccessAction(*mut dyn FnMut());
impl AccessAction {
pub fn get(self) -> *mut dyn FnMut() {
self.0
}
}
#[derive(Copy, Clone)]
pub enum Action {
Initial,
Access(AccessAction),
Complete,
}
pub struct PinnedGenerator<I, A, R> {
generator: Pin<Box<dyn Generator<Action, Yield = YieldType<I, A>, Return = R>>>,
}
impl<I, A, R> PinnedGenerator<I, A, R> {
pub fn new<T: Generator<Action, Yield = YieldType<I, A>, Return = R> + 'static>(
generator: T,
) -> (I, Self) {
let mut result = PinnedGenerator { generator: Box::pin(generator) };
// Run it to the first yield to set it up
let init = match Pin::new(&mut result.generator).resume(Action::Initial) {
GeneratorState::Yielded(YieldType::Initial(y)) => y,
_ => panic!(),
};
(init, result)
}
pub unsafe fn access(&mut self, closure: *mut dyn FnMut()) {
// Call the generator, which in turn will call the closure
if let GeneratorState::Complete(_) =
Pin::new(&mut self.generator).resume(Action::Access(AccessAction(closure)))
{
panic!()
}
}
pub fn complete(&mut self) -> R {
// Tell the generator we want it to complete, consuming it and yielding a result
let result = Pin::new(&mut self.generator).resume(Action::Complete);
if let GeneratorState::Complete(r) = result { r } else { panic!() }
}
}
#[derive(PartialEq)]
pub struct Marker<T>(PhantomData<T>);
impl<T> Marker<T> {
pub unsafe fn new() -> Self {
Marker(PhantomData)
}
}
pub enum YieldType<I, A> {
Initial(I),
Accessor(Marker<A>),
}
#[macro_export]
#[allow_internal_unstable(fn_traits)]
macro_rules! declare_box_region_type {
(impl $v:vis
$name: ident,
$yield_type:ty,
for($($lifetimes:tt)*),
($($args:ty),*) -> ($reti:ty, $retc:ty)
) => {
$v struct $name($crate::box_region::PinnedGenerator<
$reti,
for<$($lifetimes)*> fn(($($args,)*)),
$retc
>);
impl $name {
fn new<T: ::std::ops::Generator<$crate::box_region::Action, Yield = $yield_type, Return = $retc> + 'static>(
generator: T
) -> ($reti, Self) {
let (initial, pinned) = $crate::box_region::PinnedGenerator::new(generator);
(initial, $name(pinned))
}
$v fn access<F: for<$($lifetimes)*> FnOnce($($args,)*) -> R, R>(&mut self, f: F) -> R {
// Turn the FnOnce closure into *mut dyn FnMut()
// so we can pass it in to the generator
let mut r = None;
let mut f = Some(f);
let mut_f: &mut dyn for<$($lifetimes)*> FnMut(($($args,)*)) =
&mut |args| {
let f = f.take().unwrap();
r = Some(FnOnce::call_once(f, args));
};
let mut_f = mut_f as *mut dyn for<$($lifetimes)*> FnMut(($($args,)*));
// Get the generator to call our closure
unsafe {
self.0.access(::std::mem::transmute(mut_f));
}
// Unwrap the result
r.unwrap()
}
$v fn complete(mut self) -> $retc {
self.0.complete()
}
fn initial_yield(value: $reti) -> $yield_type {
$crate::box_region::YieldType::Initial(value)
}
}
};
($v:vis $name: ident, for($($lifetimes:tt)*), ($($args:ty),*) -> ($reti:ty, $retc:ty)) => {
declare_box_region_type!(
impl $v $name,
$crate::box_region::YieldType<$reti, for<$($lifetimes)*> fn(($($args,)*))>,
for($($lifetimes)*),
($($args),*) -> ($reti, $retc)
);
};
}
#[macro_export]
#[allow_internal_unstable(fn_traits)]
macro_rules! box_region_allow_access {
(for($($lifetimes:tt)*), ($($args:ty),*), ($($exprs:expr),*), $action:ident) => {
loop {
match $action {
$crate::box_region::Action::Access(accessor) => {
let accessor: &mut dyn for<$($lifetimes)*> FnMut($($args),*) = unsafe {
::std::mem::transmute(accessor.get())
};
(*accessor)(($($exprs),*));
unsafe {
let marker = $crate::box_region::Marker::<
for<$($lifetimes)*> fn(($($args,)*))
>::new();
$action = yield $crate::box_region::YieldType::Accessor(marker);
};
}
$crate::box_region::Action::Complete => break,
$crate::box_region::Action::Initial => panic!("unexpected box_region action: Initial"),
}
}
}
}

View File

@ -10,7 +10,6 @@
#![feature(array_windows)]
#![feature(control_flow_enum)]
#![feature(in_band_lifetimes)]
#![feature(generator_trait)]
#![feature(min_specialization)]
#![feature(auto_traits)]
#![feature(nll)]
@ -63,7 +62,6 @@ macro_rules! unlikely {
pub mod base_n;
pub mod binary_search_util;
pub mod box_region;
pub mod captures;
pub mod flock;
pub mod functor;

View File

@ -2,8 +2,6 @@
#![feature(box_patterns)]
#![feature(internal_output_capture)]
#![feature(nll)]
#![feature(generator_trait)]
#![feature(generators)]
#![feature(once_cell)]
#![recursion_limit = "256"]

View File

@ -6,10 +6,10 @@ use rustc_ast::mut_visit::MutVisitor;
use rustc_ast::{self as ast, visit};
use rustc_codegen_ssa::back::link::emit_metadata;
use rustc_codegen_ssa::traits::CodegenBackend;
use rustc_data_structures::parallel;
use rustc_data_structures::steal::Steal;
use rustc_data_structures::sync::{par_iter, Lrc, OnceCell, ParallelIterator, WorkerLocal};
use rustc_data_structures::temp_dir::MaybeTempDir;
use rustc_data_structures::{box_region_allow_access, declare_box_region_type, parallel};
use rustc_errors::{ErrorReported, PResult};
use rustc_expand::base::ExtCtxt;
use rustc_hir::def_id::LOCAL_CRATE;
@ -47,7 +47,9 @@ use std::cell::RefCell;
use std::ffi::OsString;
use std::io::{self, BufWriter, Write};
use std::lazy::SyncLazy;
use std::marker::PhantomPinned;
use std::path::PathBuf;
use std::pin::Pin;
use std::rc::Rc;
use std::{env, fs, iter};
@ -85,11 +87,83 @@ fn count_nodes(krate: &ast::Crate) -> usize {
counter.count
}
declare_box_region_type!(
pub BoxedResolver,
for(),
(&mut Resolver<'_>) -> (Result<ast::Crate>, ResolverOutputs)
);
pub use boxed_resolver::BoxedResolver;
mod boxed_resolver {
use super::*;
pub struct BoxedResolver(Pin<Box<BoxedResolverInner>>);
struct BoxedResolverInner {
session: Lrc<Session>,
resolver_arenas: Option<ResolverArenas<'static>>,
resolver: Option<Resolver<'static>>,
_pin: PhantomPinned,
}
// Note: Drop order is important to prevent dangling references. Resolver must be dropped first,
// then resolver_arenas and finally session.
impl Drop for BoxedResolverInner {
fn drop(&mut self) {
self.resolver.take();
self.resolver_arenas.take();
}
}
impl BoxedResolver {
pub(super) fn new<F>(session: Lrc<Session>, make_resolver: F) -> Result<(ast::Crate, Self)>
where
F: for<'a> FnOnce(
&'a Session,
&'a ResolverArenas<'a>,
) -> Result<(ast::Crate, Resolver<'a>)>,
{
let mut boxed_resolver = Box::new(BoxedResolverInner {
session,
resolver_arenas: Some(Resolver::arenas()),
resolver: None,
_pin: PhantomPinned,
});
// SAFETY: `make_resolver` takes a resolver arena with an arbitrary lifetime and
// returns a resolver with the same lifetime as the arena. We ensure that the arena
// outlives the resolver in the drop impl and elsewhere so these transmutes are sound.
unsafe {
let (crate_, resolver) = make_resolver(
std::mem::transmute::<&Session, &Session>(&boxed_resolver.session),
std::mem::transmute::<&ResolverArenas<'_>, &ResolverArenas<'_>>(
boxed_resolver.resolver_arenas.as_ref().unwrap(),
),
)?;
boxed_resolver.resolver = Some(resolver);
Ok((crate_, BoxedResolver(Pin::new_unchecked(boxed_resolver))))
}
}
pub fn access<F: for<'a> FnOnce(&mut Resolver<'a>) -> R, R>(&mut self, f: F) -> R {
// SAFETY: The resolver doesn't need to be pinned.
let mut resolver = unsafe {
self.0.as_mut().map_unchecked_mut(|boxed_resolver| &mut boxed_resolver.resolver)
};
f((&mut *resolver).as_mut().unwrap())
}
pub fn to_resolver_outputs(resolver: Rc<RefCell<BoxedResolver>>) -> ResolverOutputs {
match Rc::try_unwrap(resolver) {
Ok(resolver) => {
let mut resolver = resolver.into_inner();
// SAFETY: The resolver doesn't need to be pinned.
let mut resolver = unsafe {
resolver
.0
.as_mut()
.map_unchecked_mut(|boxed_resolver| &mut boxed_resolver.resolver)
};
resolver.take().unwrap().into_outputs()
}
Err(resolver) => resolver.borrow_mut().access(|resolver| resolver.clone_outputs()),
}
}
}
}
/// Runs the "early phases" of the compiler: initial `cfg` processing, loading compiler plugins,
/// syntax expansion, secondary `cfg` expansion, synthesis of a test
@ -111,41 +185,16 @@ pub fn configure_and_expand(
// its contents but the results of name resolution on those contents. Hopefully we'll push
// this back at some point.
let crate_name = crate_name.to_string();
let (result, resolver) = BoxedResolver::new(static move |mut action| {
let _ = action;
let sess = &*sess;
let resolver_arenas = Resolver::arenas();
let res = configure_and_expand_inner(
BoxedResolver::new(sess, move |sess, resolver_arenas| {
configure_and_expand_inner(
sess,
&lint_store,
krate,
&crate_name,
&resolver_arenas,
&*metadata_loader,
);
let mut resolver = match res {
Err(v) => {
yield BoxedResolver::initial_yield(Err(v));
panic!()
}
Ok((krate, resolver)) => {
action = yield BoxedResolver::initial_yield(Ok(krate));
resolver
}
};
box_region_allow_access!(for(), (&mut Resolver<'_>), (&mut resolver), action);
resolver.into_outputs()
});
result.map(|k| (k, resolver))
}
impl BoxedResolver {
pub fn to_resolver_outputs(resolver: Rc<RefCell<BoxedResolver>>) -> ResolverOutputs {
match Rc::try_unwrap(resolver) {
Ok(resolver) => resolver.into_inner().complete(),
Err(resolver) => resolver.borrow_mut().access(|resolver| resolver.clone_outputs()),
}
}
metadata_loader,
)
})
}
pub fn register_plugins<'a>(
@ -231,11 +280,11 @@ fn pre_expansion_lint(
fn configure_and_expand_inner<'a>(
sess: &'a Session,
lint_store: &'a LintStore,
lint_store: &LintStore,
mut krate: ast::Crate,
crate_name: &str,
resolver_arenas: &'a ResolverArenas<'a>,
metadata_loader: &'a MetadataLoaderDyn,
metadata_loader: Box<MetadataLoaderDyn>,
) -> Result<(ast::Crate, Resolver<'a>)> {
tracing::trace!("configure_and_expand_inner");
pre_expansion_lint(sess, lint_store, &krate, crate_name);

View File

@ -54,7 +54,7 @@ pub struct CStore {
pub struct CrateLoader<'a> {
// Immutable configuration.
sess: &'a Session,
metadata_loader: &'a MetadataLoaderDyn,
metadata_loader: Box<MetadataLoaderDyn>,
local_crate_name: Symbol,
// Mutable output.
cstore: CStore,
@ -219,7 +219,7 @@ impl CStore {
impl<'a> CrateLoader<'a> {
pub fn new(
sess: &'a Session,
metadata_loader: &'a MetadataLoaderDyn,
metadata_loader: Box<MetadataLoaderDyn>,
local_crate_name: &str,
) -> Self {
let local_crate_stable_id =
@ -544,7 +544,7 @@ impl<'a> CrateLoader<'a> {
info!("falling back to a load");
let mut locator = CrateLocator::new(
self.sess,
self.metadata_loader,
&*self.metadata_loader,
name,
hash,
host_hash,

View File

@ -1198,7 +1198,7 @@ impl<'a> Resolver<'a> {
session: &'a Session,
krate: &Crate,
crate_name: &str,
metadata_loader: &'a MetadataLoaderDyn,
metadata_loader: Box<MetadataLoaderDyn>,
arenas: &'a ResolverArenas<'a>,
) -> Resolver<'a> {
let root_local_def_id = LocalDefId { local_def_index: CRATE_DEF_INDEX };