Skip to content

Commit

Permalink
Auto merge of #65835 - Mark-Simulacrum:lockless-lintbuffer, r=nikomat…
Browse files Browse the repository at this point in the history
…sakis

Remove LintBuffer from Session

This moves the `LintBuffer` from `Session` into the `Resolver`, where it is used until lowering is done and then consumed by early lint passes. This also happily removes the failure mode of buffering lints too late where it would have previously lead to ICEs; it is statically no longer possible to do so.

I suspect that with a bit more work a similar move could be done for the lint buffer inside `ParseSess`, but this PR doesn't touch it (in part to keep itself small).

The last commit is the "interesting" commit -- the ones before it don't work (though they compile) as they sort of prepare the various crates for the lint buffer to be passed in rather than accessed through Session.
  • Loading branch information
bors committed Nov 4, 2019
2 parents cba9368 + c68df7c commit ab6e478
Showing 16 changed files with 173 additions and 153 deletions.
11 changes: 7 additions & 4 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
@@ -43,6 +43,7 @@ use crate::hir::def_id::{DefId, DefIndex, CRATE_DEF_INDEX};
use crate::hir::def::{Namespace, Res, DefKind, PartialRes, PerNS};
use crate::hir::{GenericArg, ConstArg};
use crate::hir::ptr::P;
use crate::lint;
use crate::lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
ELIDED_LIFETIMES_IN_PATHS};
use crate::middle::cstore::CrateStore;
@@ -184,6 +185,8 @@ pub trait Resolver {
) -> (ast::Path, Res<NodeId>);

fn has_derives(&self, node_id: NodeId, derives: SpecialDerives) -> bool;

fn lint_buffer(&mut self) -> &mut lint::LintBuffer;
}

type NtToTokenstream = fn(&Nonterminal, &ParseSess, Span) -> TokenStream;
@@ -1857,7 +1860,7 @@ impl<'a> LoweringContext<'a> {
GenericArgs::Parenthesized(ref data) => match parenthesized_generic_args {
ParenthesizedGenericArgs::Ok => self.lower_parenthesized_parameter_data(data),
ParenthesizedGenericArgs::Warn => {
self.sess.buffer_lint(
self.resolver.lint_buffer().buffer_lint(
PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
CRATE_NODE_ID,
data.span,
@@ -1953,7 +1956,7 @@ impl<'a> LoweringContext<'a> {
}
AnonymousLifetimeMode::PassThrough |
AnonymousLifetimeMode::ReportError => {
self.sess.buffer_lint_with_diagnostic(
self.resolver.lint_buffer().buffer_lint_with_diagnostic(
ELIDED_LIFETIMES_IN_PATHS,
CRATE_NODE_ID,
path_span,
@@ -3346,15 +3349,15 @@ impl<'a> LoweringContext<'a> {
}
}

fn maybe_lint_bare_trait(&self, span: Span, id: NodeId, is_global: bool) {
fn maybe_lint_bare_trait(&mut self, span: Span, id: NodeId, is_global: bool) {
// FIXME(davidtwco): This is a hack to detect macros which produce spans of the
// call site which do not have a macro backtrace. See #61963.
let is_macro_callsite = self.sess.source_map()
.span_to_snippet(span)
.map(|snippet| snippet.starts_with("#["))
.unwrap_or(true);
if !is_macro_callsite {
self.sess.buffer_lint_with_diagnostic(
self.resolver.lint_buffer().buffer_lint_with_diagnostic(
builtin::BARE_TRAIT_OBJECTS,
id,
span,
26 changes: 13 additions & 13 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
@@ -34,7 +34,6 @@ use crate::util::common::time;

use errors::DiagnosticBuilder;
use std::slice;
use std::default::Default as StdDefault;
use rustc_data_structures::sync::{self, ParallelIterator, join, par_iter};
use rustc_serialize::{Decoder, Decodable, Encoder, Encodable};
use syntax::ast;
@@ -584,12 +583,13 @@ impl<'a> EarlyContext<'a> {
lint_store: &'a LintStore,
krate: &'a ast::Crate,
buffered: LintBuffer,
warn_about_weird_lints: bool,
) -> EarlyContext<'a> {
EarlyContext {
sess,
krate,
lint_store,
builder: LintLevelSets::builder(sess, lint_store),
builder: LintLevelSets::builder(sess, warn_about_weird_lints, lint_store),
buffered,
}
}
@@ -1490,9 +1490,10 @@ fn early_lint_crate<T: EarlyLintPass>(
krate: &ast::Crate,
pass: T,
buffered: LintBuffer,
warn_about_weird_lints: bool,
) -> LintBuffer {
let mut cx = EarlyContextAndPass {
context: EarlyContext::new(sess, lint_store, krate, buffered),
context: EarlyContext::new(sess, lint_store, krate, buffered, warn_about_weird_lints),
pass,
};

@@ -1514,22 +1515,19 @@ pub fn check_ast_crate<T: EarlyLintPass>(
lint_store: &LintStore,
krate: &ast::Crate,
pre_expansion: bool,
lint_buffer: Option<LintBuffer>,
builtin_lints: T,
) {
let (mut passes, mut buffered): (Vec<_>, _) = if pre_expansion {
(
lint_store.pre_expansion_passes.iter().map(|p| (p)()).collect(),
LintBuffer::default(),
)
let mut passes: Vec<_> = if pre_expansion {
lint_store.pre_expansion_passes.iter().map(|p| (p)()).collect()
} else {
(
lint_store.early_passes.iter().map(|p| (p)()).collect(),
sess.buffered_lints.borrow_mut().take().unwrap(),
)
lint_store.early_passes.iter().map(|p| (p)()).collect()
};
let mut buffered = lint_buffer.unwrap_or_default();

if !sess.opts.debugging_opts.no_interleave_lints {
buffered = early_lint_crate(sess, lint_store, krate, builtin_lints, buffered);
buffered = early_lint_crate(sess, lint_store, krate, builtin_lints, buffered,
pre_expansion);

if !passes.is_empty() {
buffered = early_lint_crate(
@@ -1538,6 +1536,7 @@ pub fn check_ast_crate<T: EarlyLintPass>(
krate,
EarlyLintPassObjects { lints: &mut passes[..] },
buffered,
pre_expansion,
);
}
} else {
@@ -1549,6 +1548,7 @@ pub fn check_ast_crate<T: EarlyLintPass>(
krate,
EarlyLintPassObjects { lints: slice::from_mut(pass) },
buffered,
pre_expansion,
)
});
}
16 changes: 12 additions & 4 deletions src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
@@ -44,8 +44,12 @@ impl LintLevelSets {
return me
}

pub fn builder<'a>(sess: &'a Session, store: &LintStore) -> LintLevelsBuilder<'a> {
LintLevelsBuilder::new(sess, LintLevelSets::new(sess, store))
pub fn builder<'a>(
sess: &'a Session,
warn_about_weird_lints: bool,
store: &LintStore,
) -> LintLevelsBuilder<'a> {
LintLevelsBuilder::new(sess, warn_about_weird_lints, LintLevelSets::new(sess, store))
}

fn process_command_line(&mut self, sess: &Session, store: &LintStore) {
@@ -160,14 +164,18 @@ pub struct BuilderPush {
}

impl<'a> LintLevelsBuilder<'a> {
pub fn new(sess: &'a Session, sets: LintLevelSets) -> LintLevelsBuilder<'a> {
pub fn new(
sess: &'a Session,
warn_about_weird_lints: bool,
sets: LintLevelSets,
) -> LintLevelsBuilder<'a> {
assert_eq!(sets.list.len(), 1);
LintLevelsBuilder {
sess,
sets,
cur: 0,
id_to_set: Default::default(),
warn_about_weird_lints: sess.buffered_lints.borrow().is_some(),
warn_about_weird_lints,
}
}

26 changes: 21 additions & 5 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
@@ -643,13 +643,29 @@ impl LintBuffer {
}
}

pub fn take(&mut self, id: ast::NodeId) -> Vec<BufferedEarlyLint> {
fn take(&mut self, id: ast::NodeId) -> Vec<BufferedEarlyLint> {
self.map.remove(&id).unwrap_or_default()
}

pub fn get_any(&self) -> Option<&[BufferedEarlyLint]> {
let key = self.map.keys().next().map(|k| *k);
key.map(|k| &self.map[&k][..])
pub fn buffer_lint<S: Into<MultiSpan>>(
&mut self,
lint: &'static Lint,
id: ast::NodeId,
sp: S,
msg: &str,
) {
self.add_lint(lint, id, sp.into(), msg, BuiltinLintDiagnostics::Normal)
}

pub fn buffer_lint_with_diagnostic<S: Into<MultiSpan>>(
&mut self,
lint: &'static Lint,
id: ast::NodeId,
sp: S,
msg: &str,
diagnostic: BuiltinLintDiagnostics,
) {
self.add_lint(lint, id, sp.into(), msg, diagnostic)
}
}

@@ -779,7 +795,7 @@ fn lint_levels(tcx: TyCtxt<'_>, cnum: CrateNum) -> &LintLevelMap {
assert_eq!(cnum, LOCAL_CRATE);
let store = &tcx.lint_store;
let mut builder = LintLevelMapBuilder {
levels: LintLevelSets::builder(tcx.sess, &store),
levels: LintLevelSets::builder(tcx.sess, false, &store),
tcx: tcx,
store: store,
};
4 changes: 2 additions & 2 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
@@ -586,7 +586,7 @@ pub fn rustc_deprecation_message(depr: &RustcDeprecation, path: &str) -> (String
}

pub fn early_report_deprecation(
sess: &Session,
lint_buffer: &'a mut lint::LintBuffer,
message: &str,
suggestion: Option<Symbol>,
lint: &'static Lint,
@@ -597,7 +597,7 @@ pub fn early_report_deprecation(
}

let diag = BuiltinLintDiagnostics::DeprecatedMacro(suggestion, span);
sess.buffer_lint_with_diagnostic(lint, CRATE_NODE_ID, span, message, diag);
lint_buffer.buffer_lint_with_diagnostic(lint, CRATE_NODE_ID, span, message, diag);
}

fn late_report_deprecation(
2 changes: 1 addition & 1 deletion src/librustc/session/config.rs
Original file line number Diff line number Diff line change
@@ -1469,7 +1469,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
(such as entering an empty infinite loop) by inserting llvm.sideeffect"),
}

pub fn default_lib_output() -> CrateType {
pub const fn default_lib_output() -> CrateType {
CrateType::Rlib
}

38 changes: 0 additions & 38 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
@@ -6,7 +6,6 @@ use crate::hir::def_id::CrateNum;
use rustc_data_structures::fingerprint::Fingerprint;

use crate::lint;
use crate::lint::builtin::BuiltinLintDiagnostics;
use crate::session::config::{OutputType, PrintRequest, Sanitizer, SwitchWithOptPath};
use crate::session::search_paths::{PathKind, SearchPath};
use crate::util::nodemap::{FxHashMap, FxHashSet};
@@ -77,13 +76,6 @@ pub struct Session {
/// if the value stored here has been affected by path remapping.
pub working_dir: (PathBuf, bool),

/// 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
/// (sub)diagnostics that have been set once, but should not be set again,
/// in order to avoid redundantly verbose output (Issue #24690, #44953).
@@ -366,35 +358,6 @@ impl Session {
self.diagnostic().span_note_without_error(sp, msg)
}

pub fn buffer_lint<S: Into<MultiSpan>>(
&self,
lint: &'static lint::Lint,
id: ast::NodeId,
sp: S,
msg: &str,
) {
match *self.buffered_lints.borrow_mut() {
Some(ref mut buffer) => {
buffer.add_lint(lint, id, sp.into(), msg, BuiltinLintDiagnostics::Normal)
}
None => bug!("can't buffer lints after HIR lowering"),
}
}

pub fn buffer_lint_with_diagnostic<S: Into<MultiSpan>>(
&self,
lint: &'static lint::Lint,
id: ast::NodeId,
sp: S,
msg: &str,
diagnostic: BuiltinLintDiagnostics,
) {
match *self.buffered_lints.borrow_mut() {
Some(ref mut buffer) => buffer.add_lint(lint, id, sp.into(), msg, diagnostic),
None => bug!("can't buffer lints after HIR lowering"),
}
}

pub fn reserve_node_ids(&self, count: usize) -> ast::NodeId {
let id = self.next_node_id.get();

@@ -1218,7 +1181,6 @@ fn build_session_(
sysroot,
local_crate_source_file,
working_dir,
buffered_lints: Lock::new(Some(Default::default())),
one_time_diagnostics: Default::default(),
plugin_llvm_passes: OneThread::new(RefCell::new(Vec::new())),
plugin_attributes: Lock::new(Vec::new()),
10 changes: 7 additions & 3 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
@@ -267,6 +267,7 @@ fn configure_and_expand_inner<'a>(
lint_store,
&krate,
true,
None,
rustc_lint::BuiltinCombinedPreExpansionLintPass::new());
});

@@ -293,6 +294,8 @@ fn configure_and_expand_inner<'a>(
krate
});

util::check_attr_crate_type(&krate.attrs, &mut resolver.lint_buffer());

syntax_ext::plugin_macro_defs::inject(
&mut krate, &mut resolver, plugin_info.syntax_exts, sess.edition()
);
@@ -366,7 +369,7 @@ fn configure_and_expand_inner<'a>(
for span in missing_fragment_specifiers {
let lint = lint::builtin::MISSING_FRAGMENT_SPECIFIER;
let msg = "missing fragment specifier";
sess.buffer_lint(lint, ast::CRATE_NODE_ID, span, msg);
resolver.lint_buffer().buffer_lint(lint, ast::CRATE_NODE_ID, span, msg);
}
if cfg!(windows) {
env::set_var("PATH", &old_path);
@@ -395,7 +398,7 @@ fn configure_and_expand_inner<'a>(
}

let has_proc_macro_decls = time(sess, "AST validation", || {
ast_validation::check_crate(sess, &krate)
ast_validation::check_crate(sess, &krate, &mut resolver.lint_buffer())
});


@@ -464,7 +467,7 @@ fn configure_and_expand_inner<'a>(
info!("{} parse sess buffered_lints", buffered_lints.len());
for BufferedEarlyLint{id, span, msg, lint_id} in buffered_lints.drain(..) {
let lint = lint::Lint::from_parser_lint_id(lint_id);
sess.buffer_lint(lint, id, span, &msg);
resolver.lint_buffer().buffer_lint(lint, id, span, &msg);
}
});

@@ -496,6 +499,7 @@ pub fn lower_to_hir(
lint_store,
&krate,
false,
Some(std::mem::take(resolver.lint_buffer())),
rustc_lint::BuiltinCombinedEarlyLintPass::new(),
)
});
Loading

0 comments on commit ab6e478

Please sign in to comment.