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

Lockless LintStore #65193

Merged
merged 18 commits into from
Oct 24, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
47a443c
Duplicate lint specifications are always bug!
Mark-Simulacrum Sep 24, 2019
577d442
De-propagate optional session from lint registration
Mark-Simulacrum Sep 24, 2019
2121b04
Handle lints, not passes in push_lints
Mark-Simulacrum Sep 24, 2019
748eccd
Lints being from a plugin is dependent on the lint, not the registration
Mark-Simulacrum Oct 7, 2019
b060f3b
Split module and crate late pass registration
Mark-Simulacrum Oct 7, 2019
e1079c8
Split out just registration to separate function
Mark-Simulacrum Oct 7, 2019
68c07db
No longer implicitly register lints when registering passes
Mark-Simulacrum Oct 7, 2019
7fef397
Make get_lints be a static function
Mark-Simulacrum Oct 7, 2019
2454512
Take lint passes as constructor functions
Mark-Simulacrum Oct 7, 2019
aa4ee2c
Move to storing constructor functions inside LintStore
Mark-Simulacrum Oct 7, 2019
c1abc30
Make declare_lint take any amount of boolean fields
Mark-Simulacrum Oct 8, 2019
7abb1fa
Remove side table of future incompatibility info
Mark-Simulacrum Oct 9, 2019
c4475c7
Access future incompatibility information directly
Mark-Simulacrum Oct 9, 2019
da56d1d
Remove all borrows of lint store from Session from librustc
Mark-Simulacrum Oct 9, 2019
dab3bd6
Create lint store during plugin registration
Mark-Simulacrum Oct 9, 2019
b761367
Fix test fallout
Mark-Simulacrum Oct 9, 2019
6be0a70
Update API to be more compatible with plugin needs
Mark-Simulacrum Oct 10, 2019
4e8d1b2
Add some documentation
Mark-Simulacrum Oct 22, 2019
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3483,6 +3483,7 @@ dependencies = [
"rustc_data_structures",
"rustc_errors",
"rustc_interface",
"rustc_lint",
"rustc_metadata",
"rustc_mir",
"rustc_plugin",
Expand Down
16 changes: 8 additions & 8 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::util::common::time;
use errors::DiagnosticBuilder;
use std::slice;
use std::default::Default as StdDefault;
use rustc_data_structures::sync::{ReadGuard, ParallelIterator, join, par_iter};
use rustc_data_structures::sync::{ParallelIterator, join, par_iter};
use rustc_serialize::{Decoder, Decodable, Encoder, Encodable};
use syntax::ast;
use syntax::util::lev_distance::find_best_match_for_name;
Expand Down Expand Up @@ -452,7 +452,7 @@ pub struct LateContext<'a, 'tcx> {
pub access_levels: &'a AccessLevels,

/// The store of registered lints and the lint levels.
lint_store: ReadGuard<'a, LintStore>,
lint_store: &'tcx LintStore,

last_node_with_lint_attrs: hir::HirId,

Expand Down Expand Up @@ -1320,7 +1320,7 @@ fn late_lint_mod_pass<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(
tables: &ty::TypeckTables::empty(None),
param_env: ty::ParamEnv::empty(),
access_levels,
lint_store: tcx.sess.lint_store.borrow(),
lint_store: &tcx.lint_store,
last_node_with_lint_attrs: tcx.hir().as_local_hir_id(module_def_id).unwrap(),
generics: None,
only_module: true,
Expand Down Expand Up @@ -1352,7 +1352,7 @@ pub fn late_lint_mod<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(

late_lint_mod_pass(tcx, module_def_id, builtin_lints);

let mut passes: Vec<_> = tcx.sess.lint_store.borrow().late_module_passes
let mut passes: Vec<_> = tcx.lint_store.late_module_passes
.iter().map(|pass| (pass)()).collect();

if !passes.is_empty() {
Expand All @@ -1370,7 +1370,7 @@ fn late_lint_pass_crate<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(tcx: TyCtxt<'tc
tables: &ty::TypeckTables::empty(None),
param_env: ty::ParamEnv::empty(),
access_levels,
lint_store: tcx.sess.lint_store.borrow(),
lint_store: &tcx.lint_store,
last_node_with_lint_attrs: hir::CRATE_HIR_ID,
generics: None,
only_module: false,
Expand All @@ -1394,7 +1394,7 @@ fn late_lint_pass_crate<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(tcx: TyCtxt<'tc
}

fn late_lint_crate<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(tcx: TyCtxt<'tcx>, builtin_lints: T) {
let mut passes = tcx.sess.lint_store.borrow()
let mut passes = tcx.lint_store
.late_passes.iter().map(|p| (p)()).collect::<Vec<_>>();

if !tcx.sess.opts.debugging_opts.no_interleave_lints {
Expand All @@ -1410,7 +1410,7 @@ fn late_lint_crate<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(tcx: TyCtxt<'tcx>, b
});
}

let mut passes: Vec<_> = tcx.sess.lint_store.borrow().late_module_passes
let mut passes: Vec<_> = tcx.lint_store.late_module_passes
.iter().map(|pass| (pass)()).collect();

for pass in &mut passes {
Expand Down Expand Up @@ -1571,7 +1571,7 @@ impl Decodable for LintId {
fn decode<D: Decoder>(d: &mut D) -> Result<LintId, D::Error> {
let s = d.read_str()?;
ty::tls::with(|tcx| {
match tcx.sess.lint_store.borrow().find_lints(&s) {
match tcx.lint_store.find_lints(&s) {
Ok(ids) => {
if ids.len() != 0 {
panic!("invalid lint-id `{}`", s);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,11 +777,11 @@ pub fn maybe_lint_level_root(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {

fn lint_levels(tcx: TyCtxt<'_>, cnum: CrateNum) -> &LintLevelMap {
assert_eq!(cnum, LOCAL_CRATE);
let store = tcx.sess.lint_store.borrow();
let store = &tcx.lint_store;
let mut builder = LintLevelMapBuilder {
levels: LintLevelSets::builder(tcx.sess, &store),
tcx: tcx,
store: &*store,
store: store,
};
let krate = tcx.hir().krate();

Expand Down
11 changes: 6 additions & 5 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::util::common::{duration_to_secs_str, ErrorReported};

use rustc_data_structures::base_n;
use rustc_data_structures::sync::{
self, Lrc, Lock, OneThread, Once, RwLock, AtomicU64, AtomicUsize, Ordering,
self, Lrc, Lock, OneThread, Once, AtomicU64, AtomicUsize, Ordering,
Ordering::SeqCst,
};

Expand Down Expand Up @@ -77,9 +77,11 @@ pub struct Session {
/// if the value stored here has been affected by path remapping.
pub working_dir: (PathBuf, bool),

// FIXME: `lint_store` and `buffered_lints` are not thread-safe,
Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved
// but are only used in a single thread.
pub lint_store: RwLock<lint::LintStore>,
/// This is intended to be used from a single thread.
///
/// FIXME: there was a previous comment about this not being thread safe,
/// but it's not clear how or why that's the case. The LintBuffer itself is certainly thread
/// safe at least from a "Rust safety" standpoint.
pub buffered_lints: Lock<Option<lint::LintBuffer>>,

/// Set of `(DiagnosticId, Option<Span>, message)` tuples tracking
Expand Down Expand Up @@ -1213,7 +1215,6 @@ fn build_session_(
sysroot,
local_crate_source_file,
working_dir,
lint_store: RwLock::new(lint::LintStore::new()),
buffered_lints: Lock::new(Some(Default::default())),
one_time_diagnostics: Default::default(),
plugin_llvm_passes: OneThread::new(RefCell::new(Vec::new())),
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,8 @@ pub struct GlobalCtxt<'tcx> {

pub sess: &'tcx Session,

pub lint_store: Lrc<lint::LintStore>,

pub dep_graph: DepGraph,

pub prof: SelfProfilerRef,
Expand Down Expand Up @@ -1199,6 +1201,7 @@ impl<'tcx> TyCtxt<'tcx> {
/// reference to the context, to allow formatting values that need it.
pub fn create_global_ctxt(
s: &'tcx Session,
lint_store: Lrc<lint::LintStore>,
cstore: &'tcx CrateStoreDyn,
local_providers: ty::query::Providers<'tcx>,
extern_providers: ty::query::Providers<'tcx>,
Expand Down Expand Up @@ -1268,6 +1271,7 @@ impl<'tcx> TyCtxt<'tcx> {

GlobalCtxt {
sess: s,
lint_store,
cstore,
arena: WorkerLocal::new(|_| Arena::default()),
interners,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ log = "0.4"
env_logger = { version = "0.7", default-features = false }
rustc = { path = "../librustc" }
rustc_target = { path = "../librustc_target" }
rustc_lint = { path = "../librustc_lint" }
rustc_data_structures = { path = "../librustc_data_structures" }
errors = { path = "../librustc_errors", package = "rustc_errors" }
rustc_metadata = { path = "../librustc_metadata" }
Expand Down
18 changes: 12 additions & 6 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,13 @@ pub fn run_compiler(
interface::run_compiler(config, |compiler| {
let sopts = &compiler.session().opts;
if sopts.describe_lints {
let lint_store = rustc_lint::new_lint_store(
sopts.debugging_opts.no_interleave_lints,
compiler.session().unstable_options(),
);
describe_lints(
compiler.session(),
&*compiler.session().lint_store.borrow(),
&lint_store,
false
);
return;
Expand Down Expand Up @@ -321,12 +325,14 @@ pub fn run_compiler(
return sess.compile_status();
}

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

// Lint plugins are registered; now we can process command line flags.
if sess.opts.describe_lints {
describe_lints(&sess, &sess.lint_store.borrow(), 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 sess.compile_status();
}
}

compiler.expansion()?;
Expand Down
38 changes: 16 additions & 22 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ declare_box_region_type!(
/// Returns `None` if we're aborting after handling -W help.
pub fn configure_and_expand(
sess: Lrc<Session>,
lint_store: Lrc<lint::LintStore>,
cstore: Lrc<CStore>,
krate: ast::Crate,
crate_name: &str,
Expand All @@ -134,6 +135,7 @@ pub fn configure_and_expand(
let resolver_arenas = Resolver::arenas();
let res = configure_and_expand_inner(
sess,
&lint_store,
&*cstore,
krate,
&crate_name,
Expand Down Expand Up @@ -227,7 +229,7 @@ pub fn register_plugins<'a>(
cstore: &'a CStore,
mut krate: ast::Crate,
crate_name: &str,
) -> Result<(ast::Crate, PluginInfo)> {
) -> Result<(ast::Crate, PluginInfo, Lrc<lint::LintStore>)> {
krate = time(sess, "attributes injection", || {
syntax_ext::cmdline_attrs::inject(
krate, &sess.parse_sess, &sess.opts.debugging_opts.crate_attr
Expand Down Expand Up @@ -278,7 +280,12 @@ pub fn register_plugins<'a>(
)
});

let mut registry = Registry::new(sess, krate.span);
let mut lint_store = rustc_lint::new_lint_store(
sess.opts.debugging_opts.no_interleave_lints,
sess.unstable_options(),
);

let mut registry = Registry::new(sess, &mut lint_store, krate.span);

time(sess, "plugin registration", || {
for registrar in registrars {
Expand All @@ -289,36 +296,20 @@ pub fn register_plugins<'a>(

let Registry {
syntax_exts,
early_lint_passes,
late_lint_passes,
lints,
lint_groups,
llvm_passes,
attributes,
..
} = registry;

let mut ls = sess.lint_store.borrow_mut();
ls.register_lints(&lints);
for pass in early_lint_passes {
ls.register_early_pass(pass);
}
for pass in late_lint_passes {
ls.register_late_pass(pass);
}

for (name, (to, deprecated_name)) in lint_groups {
ls.register_group(true, name, deprecated_name, to);
}

*sess.plugin_llvm_passes.borrow_mut() = llvm_passes;
*sess.plugin_attributes.borrow_mut() = attributes;

Ok((krate, PluginInfo { syntax_exts }))
Ok((krate, PluginInfo { syntax_exts }, Lrc::new(lint_store)))
}

fn configure_and_expand_inner<'a>(
sess: &'a Session,
lint_store: &'a lint::LintStore,
cstore: &'a CStore,
mut krate: ast::Crate,
crate_name: &str,
Expand All @@ -329,7 +320,7 @@ fn configure_and_expand_inner<'a>(
time(sess, "pre-AST-expansion lint checks", || {
lint::check_ast_crate(
sess,
&*sess.lint_store.borrow(),
lint_store,
&krate,
true,
rustc_lint::BuiltinCombinedPreExpansionLintPass::new());
Expand Down Expand Up @@ -539,6 +530,7 @@ fn configure_and_expand_inner<'a>(

pub fn lower_to_hir(
sess: &Session,
lint_store: &lint::LintStore,
cstore: &CStore,
resolver: &mut Resolver<'_>,
dep_graph: &DepGraph,
Expand All @@ -559,7 +551,7 @@ pub fn lower_to_hir(
time(sess, "early lint checks", || {
lint::check_ast_crate(
sess,
&*sess.lint_store.borrow(),
lint_store,
&krate,
false,
rustc_lint::BuiltinCombinedEarlyLintPass::new(),
Expand Down Expand Up @@ -826,6 +818,7 @@ impl BoxedGlobalCtxt {

pub fn create_global_ctxt(
compiler: &Compiler,
lint_store: Lrc<lint::LintStore>,
mut hir_forest: hir::map::Forest,
defs: hir::map::Definitions,
resolutions: Resolutions,
Expand Down Expand Up @@ -863,6 +856,7 @@ pub fn create_global_ctxt(

let gcx = TyCtxt::create_global_ctxt(
sess,
lint_store,
cstore,
local_providers,
extern_providers,
Expand Down
21 changes: 15 additions & 6 deletions src/librustc_interface/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ use crate::interface::{Compiler, Result};
use crate::passes::{self, BoxedResolver, ExpansionResult, BoxedGlobalCtxt, PluginInfo};

use rustc_incremental::DepGraphFuture;
use rustc_data_structures::sync::Lrc;
use rustc::session::config::{OutputFilenames, OutputType};
use rustc::util::common::{time, ErrorReported};
use rustc::hir;
use rustc::lint::LintStore;
use rustc::hir::def_id::LOCAL_CRATE;
use rustc::ty::steal::Steal;
use rustc::dep_graph::DepGraph;
Expand Down Expand Up @@ -74,8 +76,8 @@ pub(crate) struct Queries {
dep_graph_future: Query<Option<DepGraphFuture>>,
parse: Query<ast::Crate>,
crate_name: Query<String>,
register_plugins: Query<(ast::Crate, PluginInfo)>,
expansion: Query<(ast::Crate, Steal<Rc<RefCell<BoxedResolver>>>)>,
register_plugins: Query<(ast::Crate, PluginInfo, Lrc<LintStore>)>,
expansion: Query<(ast::Crate, Steal<Rc<RefCell<BoxedResolver>>>, Lrc<LintStore>)>,
dep_graph: Query<DepGraph>,
lower_to_hir: Query<(Steal<hir::map::Forest>, ExpansionResult)>,
prepare_outputs: Query<OutputFilenames>,
Expand Down Expand Up @@ -106,7 +108,7 @@ impl Compiler {
})
}

pub fn register_plugins(&self) -> Result<&Query<(ast::Crate, PluginInfo)>> {
pub fn register_plugins(&self) -> Result<&Query<(ast::Crate, PluginInfo, Lrc<LintStore>)>> {
self.queries.register_plugins.compute(|| {
let crate_name = self.crate_name()?.peek().clone();
let krate = self.parse()?.take();
Expand Down Expand Up @@ -148,17 +150,20 @@ impl Compiler {

pub fn expansion(
&self
) -> Result<&Query<(ast::Crate, Steal<Rc<RefCell<BoxedResolver>>>)>> {
) -> Result<&Query<(ast::Crate, Steal<Rc<RefCell<BoxedResolver>>>, Lrc<LintStore>)>> {
self.queries.expansion.compute(|| {
let crate_name = self.crate_name()?.peek().clone();
let (krate, plugin_info) = self.register_plugins()?.take();
let (krate, plugin_info, lint_store) = self.register_plugins()?.take();
passes::configure_and_expand(
self.sess.clone(),
lint_store.clone(),
self.cstore().clone(),
krate,
&crate_name,
plugin_info,
).map(|(krate, resolver)| (krate, Steal::new(Rc::new(RefCell::new(resolver)))))
).map(|(krate, resolver)| {
(krate, Steal::new(Rc::new(RefCell::new(resolver))), lint_store)
})
})
}

Expand All @@ -185,9 +190,11 @@ impl Compiler {
let peeked = expansion_result.peek();
let krate = &peeked.0;
let resolver = peeked.1.steal();
let lint_store = &peeked.2;
let hir = Steal::new(resolver.borrow_mut().access(|resolver| {
passes::lower_to_hir(
self.session(),
lint_store,
self.cstore(),
resolver,
&*self.dep_graph()?.peek(),
Expand All @@ -212,11 +219,13 @@ impl Compiler {
self.queries.global_ctxt.compute(|| {
let crate_name = self.crate_name()?.peek().clone();
let outputs = self.prepare_outputs()?.peek().clone();
let lint_store = self.expansion()?.peek().2.clone();
let hir = self.lower_to_hir()?;
let hir = hir.peek();
let (ref hir_forest, ref expansion) = *hir;
Ok(passes::create_global_ctxt(
self,
lint_store,
hir_forest.steal(),
expansion.defs.steal(),
expansion.resolutions.steal(),
Expand Down
Loading