Skip to content

Commit

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

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
Centril authored Oct 26, 2019
2 parents 8ce925a + a44126a commit ee82c8c
Show file tree
Hide file tree
Showing 16 changed files with 170 additions and 152 deletions.
11 changes: 7 additions & 4 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 13 additions & 13 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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,
};

Expand All @@ -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(
Expand All @@ -1538,6 +1536,7 @@ pub fn check_ast_crate<T: EarlyLintPass>(
krate,
EarlyLintPassObjects { lints: &mut passes[..] },
buffered,
pre_expansion,
);
}
} else {
Expand All @@ -1549,6 +1548,7 @@ pub fn check_ast_crate<T: EarlyLintPass>(
krate,
EarlyLintPassObjects { lints: slice::from_mut(pass) },
buffered,
pre_expansion,
)
});
}
Expand Down
16 changes: 12 additions & 4 deletions src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
}
}

Expand Down
26 changes: 21 additions & 5 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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,
};
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
38 changes: 0 additions & 38 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -363,35 +355,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();

Expand Down Expand Up @@ -1215,7 +1178,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()),
Expand Down
10 changes: 7 additions & 3 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ fn configure_and_expand_inner<'a>(
lint_store,
&krate,
true,
None,
rustc_lint::BuiltinCombinedPreExpansionLintPass::new());
});

Expand All @@ -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()
);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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())
});


Expand Down Expand Up @@ -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);
}
});

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

0 comments on commit ee82c8c

Please sign in to comment.