From f530156ef5669bffc31d8e4659b444631afad4a6 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 08:06:01 -0400 Subject: [PATCH 1/6] Remove unused get_any method --- src/librustc/lint/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 4da146b1e5d57..3b460c29b0c4a 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -646,11 +646,6 @@ impl LintBuffer { pub fn take(&mut self, id: ast::NodeId) -> Vec { 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 struct_lint_level<'a>(sess: &'a Session, From 1bd6b489143879ab5775d7a92627687c4b9b1857 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 08:08:53 -0400 Subject: [PATCH 2/6] Only permit taking buffered lints inside lint internals --- src/librustc/lint/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 3b460c29b0c4a..49a90aee245c0 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -643,7 +643,7 @@ impl LintBuffer { } } - pub fn take(&mut self, id: ast::NodeId) -> Vec { + fn take(&mut self, id: ast::NodeId) -> Vec { self.map.remove(&id).unwrap_or_default() } } From bb0c930f826bf1929fed70109ecfaaf7e6fbda0d Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 09:15:33 -0400 Subject: [PATCH 3/6] Migrate resolver over to internal lint buffer --- src/librustc/lint/mod.rs | 21 +++++++++++++++++++++ src/librustc/middle/stability.rs | 4 ++-- src/librustc_interface/passes.rs | 8 +++++--- src/librustc_passes/ast_validation.rs | 21 ++++++++++++--------- src/librustc_resolve/check_unused.rs | 6 +++--- src/librustc_resolve/late.rs | 6 +++--- src/librustc_resolve/lib.rs | 16 ++++++++++------ src/librustc_resolve/macros.rs | 16 +++++++++------- src/librustc_resolve/resolve_imports.rs | 10 ++++++---- src/libsyntax_expand/base.rs | 4 ++-- 10 files changed, 73 insertions(+), 39 deletions(-) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 49a90aee245c0..42f33740b23e1 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -646,6 +646,27 @@ impl LintBuffer { fn take(&mut self, id: ast::NodeId) -> Vec { self.map.remove(&id).unwrap_or_default() } + + pub fn buffer_lint>( + &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>( + &mut self, + lint: &'static Lint, + id: ast::NodeId, + sp: S, + msg: &str, + diagnostic: BuiltinLintDiagnostics, + ) { + self.add_lint(lint, id, sp.into(), msg, diagnostic) + } } pub fn struct_lint_level<'a>(sess: &'a Session, diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index e65f17c79497e..93d0627ac6e40 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -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, 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( diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index b8593bd91990e..a5b00568b53f5 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -270,12 +270,14 @@ fn configure_and_expand_inner<'a>( rustc_lint::BuiltinCombinedPreExpansionLintPass::new()); }); + let lint_buffer = lint::LintBuffer::default(); let mut resolver = Resolver::new( sess, &krate, crate_name, metadata_loader, &resolver_arenas, + lint_buffer, ); syntax_ext::register_builtin_macros(&mut resolver, sess.edition()); @@ -366,7 +368,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 +397,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 +466,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); } }); diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 994e9405fb643..e625334040e0a 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -73,6 +73,8 @@ struct AstValidator<'a> { /// these booleans. warning_period_57979_didnt_record_next_impl_trait: bool, warning_period_57979_impl_trait_in_proj: bool, + + lint_buffer: &'a mut lint::LintBuffer, } impl<'a> AstValidator<'a> { @@ -229,7 +231,7 @@ impl<'a> AstValidator<'a> { err.emit(); } - fn check_decl_no_pat(&self, decl: &FnDecl, report_err: ReportFn) { + fn check_decl_no_pat(decl: &FnDecl, mut report_err: F) { for arg in &decl.inputs { match arg.pat.kind { PatKind::Ident(BindingMode::ByValue(Mutability::Immutable), _, None) | @@ -460,7 +462,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { match ty.kind { TyKind::BareFn(ref bfty) => { self.check_fn_decl(&bfty.decl); - self.check_decl_no_pat(&bfty.decl, |span, _| { + Self::check_decl_no_pat(&bfty.decl, |span, _| { struct_span_err!(self.session, span, E0561, "patterns aren't allowed in function pointer types").emit(); }); @@ -483,7 +485,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { TyKind::ImplTrait(_, ref bounds) => { if self.is_impl_trait_banned { if self.warning_period_57979_impl_trait_in_proj { - self.session.buffer_lint( + self.lint_buffer.buffer_lint( NESTED_IMPL_TRAIT, ty.id, ty.span, "`impl Trait` is not allowed in path parameters"); } else { @@ -494,7 +496,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if let Some(outer_impl_trait) = self.outer_impl_trait { if outer_impl_trait.should_warn_instead_of_error() { - self.session.buffer_lint_with_diagnostic( + self.lint_buffer.buffer_lint_with_diagnostic( NESTED_IMPL_TRAIT, ty.id, ty.span, "nested `impl Trait` is not allowed", BuiltinLintDiagnostics::NestedImplTrait { @@ -634,9 +636,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.check_trait_fn_not_async(trait_item.span, sig.header.asyncness.node); self.check_trait_fn_not_const(sig.header.constness); if block.is_none() { - self.check_decl_no_pat(&sig.decl, |span, mut_ident| { + Self::check_decl_no_pat(&sig.decl, |span, mut_ident| { if mut_ident { - self.session.buffer_lint( + self.lint_buffer.buffer_lint( lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY, trait_item.id, span, "patterns aren't allowed in methods without bodies"); @@ -655,7 +657,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if attr::contains_name(&item.attrs, sym::warn_directory_ownership) { let lint = lint::builtin::LEGACY_DIRECTORY_OWNERSHIP; let msg = "cannot declare a new module at this location"; - self.session.buffer_lint(lint, item.id, item.span, msg); + self.lint_buffer.buffer_lint(lint, item.id, item.span, msg); } } ItemKind::Union(ref vdata, _) => { @@ -686,7 +688,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { match fi.kind { ForeignItemKind::Fn(ref decl, _) => { self.check_fn_decl(decl); - self.check_decl_no_pat(decl, |span, _| { + Self::check_decl_no_pat(decl, |span, _| { struct_span_err!(self.session, span, E0130, "patterns aren't allowed in foreign function declarations") .span_label(span, "pattern not allowed in foreign function").emit(); @@ -840,7 +842,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } } -pub fn check_crate(session: &Session, krate: &Crate) -> bool { +pub fn check_crate(session: &Session, krate: &Crate, lints: &mut lint::LintBuffer) -> bool { let mut validator = AstValidator { session, has_proc_macro_decls: false, @@ -849,6 +851,7 @@ pub fn check_crate(session: &Session, krate: &Crate) -> bool { is_assoc_ty_bound_banned: false, warning_period_57979_didnt_record_next_impl_trait: false, warning_period_57979_impl_trait_in_proj: false, + lint_buffer: lints, }; visit::walk_crate(&mut validator, krate); diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs index 737589acf8d81..44b7a9fa047c2 100644 --- a/src/librustc_resolve/check_unused.rs +++ b/src/librustc_resolve/check_unused.rs @@ -232,7 +232,7 @@ impl Resolver<'_> { directive.span.is_dummy() => { if let ImportDirectiveSubclass::MacroUse = directive.subclass { if !directive.span.is_dummy() { - self.session.buffer_lint( + self.lint_buffer.buffer_lint( lint::builtin::MACRO_USE_EXTERN_CRATE, directive.id, directive.span, @@ -250,7 +250,7 @@ impl Resolver<'_> { ImportDirectiveSubclass::MacroUse => { let lint = lint::builtin::UNUSED_IMPORTS; let msg = "unused `#[macro_use]` import"; - self.session.buffer_lint(lint, directive.id, directive.span, msg); + self.lint_buffer.buffer_lint(lint, directive.id, directive.span, msg); } _ => {} } @@ -312,7 +312,7 @@ impl Resolver<'_> { "remove the unused import" }; - visitor.r.session.buffer_lint_with_diagnostic( + visitor.r.lint_buffer.buffer_lint_with_diagnostic( lint::builtin::UNUSED_IMPORTS, unused.use_tree_id, ms, diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 02f4345ac10ed..004d86cee8ded 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -1548,7 +1548,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { if is_expected(ctor_res) && self.r.is_accessible_from(ctor_vis, self.parent_scope.module) { let lint = lint::builtin::LEGACY_CONSTRUCTOR_VISIBILITY; - self.r.session.buffer_lint(lint, id, span, + self.r.lint_buffer.buffer_lint(lint, id, span, "private struct constructors are not usable through \ re-exports in outer modules", ); @@ -1774,7 +1774,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { }; if result.base_res() == unqualified_result { let lint = lint::builtin::UNUSED_QUALIFICATIONS; - self.r.session.buffer_lint(lint, id, span, "unnecessary qualification") + self.r.lint_buffer.buffer_lint(lint, id, span, "unnecessary qualification") } } @@ -2111,7 +2111,7 @@ impl<'a> Resolver<'a> { let mut late_resolution_visitor = LateResolutionVisitor::new(self); visit::walk_crate(&mut late_resolution_visitor, krate); for (id, span) in late_resolution_visitor.diagnostic_metadata.unused_labels.iter() { - self.session.buffer_lint(lint::builtin::UNUSED_LABELS, *id, *span, "unused label"); + self.lint_buffer.buffer_lint(lint::builtin::UNUSED_LABELS, *id, *span, "unused label"); } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index cea205a823665..f012606a5ca3c 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -962,6 +962,8 @@ pub struct Resolver<'a> { /// Stores enum visibilities to properly build a reduced graph /// when visiting the correspondent variants. variant_vis: DefIdMap, + + pub lint_buffer: lint::LintBuffer, } /// Nothing really interesting here; it just provides memory for the rest of the crate. @@ -1088,7 +1090,8 @@ impl<'a> Resolver<'a> { krate: &Crate, crate_name: &str, metadata_loader: &'a MetadataLoaderDyn, - arenas: &'a ResolverArenas<'a>) + arenas: &'a ResolverArenas<'a>, + lint_buffer: lint::LintBuffer) -> Resolver<'a> { let root_def_id = DefId::local(CRATE_DEF_INDEX); let root_module_kind = ModuleKind::Def( @@ -1227,7 +1230,8 @@ impl<'a> Resolver<'a> { features.declared_lib_features.iter().map(|(feat, ..)| *feat) .chain(features.declared_lang_features.iter().map(|(feat, ..)| *feat)) .collect(), - variant_vis: Default::default() + variant_vis: Default::default(), + lint_buffer, } } @@ -1653,7 +1657,7 @@ impl<'a> Resolver<'a> { match result { Ok(binding) => { if let Some(node_id) = poisoned { - self.session.buffer_lint_with_diagnostic( + self.lint_buffer.buffer_lint_with_diagnostic( lint::builtin::PROC_MACRO_DERIVE_RESOLUTION_FALLBACK, node_id, ident.span, &format!("cannot find {} `{}` in this scope", ns.descr(), ident), @@ -2118,7 +2122,7 @@ impl<'a> Resolver<'a> { } fn lint_if_path_starts_with_module( - &self, + &mut self, crate_lint: CrateLint, path: &[Segment], path_span: Span, @@ -2169,7 +2173,7 @@ impl<'a> Resolver<'a> { let diag = lint::builtin::BuiltinLintDiagnostics ::AbsPathWithModule(diag_span); - self.session.buffer_lint_with_diagnostic( + self.lint_buffer.buffer_lint_with_diagnostic( lint::builtin::ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE, diag_id, diag_span, "absolute paths must start with `self`, `super`, \ @@ -2419,7 +2423,7 @@ impl<'a> Resolver<'a> { for &(span_use, span_def) in &self.macro_expanded_macro_export_errors { let msg = "macro-expanded `macro_export` macros from the current crate \ cannot be referred to by absolute paths"; - self.session.buffer_lint_with_diagnostic( + self.lint_buffer.buffer_lint_with_diagnostic( lint::builtin::MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS, CRATE_NODE_ID, span_use, msg, lint::builtin::BuiltinLintDiagnostics:: diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 8b1b6db3ddc23..84d3d3a48b039 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -247,9 +247,9 @@ impl<'a> base::Resolver for Resolver<'a> { Ok(InvocationRes::Single(ext)) } - fn check_unused_macros(&self) { + fn check_unused_macros(&mut self) { for (&node_id, &span) in self.unused_macros.iter() { - self.session.buffer_lint( + self.lint_buffer.buffer_lint( lint::builtin::UNUSED_MACROS, node_id, span, "unused macro definition" ); } @@ -789,15 +789,17 @@ impl<'a> Resolver<'a> { } } - fn check_stability_and_deprecation(&self, ext: &SyntaxExtension, path: &ast::Path) { + fn check_stability_and_deprecation(&mut self, ext: &SyntaxExtension, path: &ast::Path) { let span = path.span; if let Some(stability) = &ext.stability { if let StabilityLevel::Unstable { reason, issue, is_soft } = stability.level { let feature = stability.feature; if !self.active_features.contains(&feature) && !span.allows_unstable(feature) { let node_id = ast::CRATE_NODE_ID; - let soft_handler = - |lint, span, msg: &_| self.session.buffer_lint(lint, node_id, span, msg); + let lint_buffer = &mut self.lint_buffer; + let soft_handler = |lint, span, msg: &_| { + lint_buffer.buffer_lint(lint, node_id, span, msg) + }; stability::report_unstable( self.session, feature, reason, issue, is_soft, span, soft_handler ); @@ -807,14 +809,14 @@ impl<'a> Resolver<'a> { let path = pprust::path_to_string(path); let (message, lint) = stability::rustc_deprecation_message(depr, &path); stability::early_report_deprecation( - self.session, &message, depr.suggestion, lint, span + &mut self.lint_buffer, &message, depr.suggestion, lint, span ); } } if let Some(depr) = &ext.deprecation { let path = pprust::path_to_string(&path); let (message, lint) = stability::deprecation_message(depr, &path); - stability::early_report_deprecation(self.session, &message, None, lint, span); + stability::early_report_deprecation(&mut self.lint_buffer, &message, None, lint, span); } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 03ff8ba7dc158..c39f0c90f98a5 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -496,7 +496,7 @@ impl<'a> Resolver<'a> { if let (&NameBindingKind::Res(_, true), &NameBindingKind::Res(_, true)) = (&old_binding.kind, &binding.kind) { - this.session.buffer_lint_with_diagnostic( + this.lint_buffer.buffer_lint_with_diagnostic( DUPLICATE_MACRO_EXPORTS, CRATE_NODE_ID, binding.span, @@ -979,7 +979,9 @@ impl<'a, 'b> ImportResolver<'a, 'b> { !max_vis.get().is_at_least(directive.vis.get(), &*self) { let msg = "glob import doesn't reexport anything because no candidate is public enough"; - self.r.session.buffer_lint(UNUSED_IMPORTS, directive.id, directive.span, msg); + self.r.lint_buffer.buffer_lint( + UNUSED_IMPORTS, directive.id, directive.span, msg, + ); } return None; } @@ -1148,7 +1150,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { re-exported (error E0365), consider declaring with \ `pub`", ident); - self.r.session.buffer_lint(PUB_USE_OF_PRIVATE_EXTERN_CRATE, + self.r.lint_buffer.buffer_lint(PUB_USE_OF_PRIVATE_EXTERN_CRATE, directive.id, directive.span, &msg); @@ -1273,7 +1275,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { let mut redundant_spans: Vec<_> = redundant_span.present_items().collect(); redundant_spans.sort(); redundant_spans.dedup(); - self.r.session.buffer_lint_with_diagnostic( + self.r.lint_buffer.buffer_lint_with_diagnostic( UNUSED_IMPORTS, directive.id, directive.span, diff --git a/src/libsyntax_expand/base.rs b/src/libsyntax_expand/base.rs index a66263a9a028a..d79b691058790 100644 --- a/src/libsyntax_expand/base.rs +++ b/src/libsyntax_expand/base.rs @@ -858,7 +858,7 @@ pub trait Resolver { &mut self, invoc: &Invocation, eager_expansion_root: ExpnId, force: bool ) -> Result; - fn check_unused_macros(&self); + fn check_unused_macros(&mut self); fn has_derives(&self, expn_id: ExpnId, derives: SpecialDerives) -> bool; fn add_derives(&mut self, expn_id: ExpnId, derives: SpecialDerives); @@ -1053,7 +1053,7 @@ impl<'a> ExtCtxt<'a> { Symbol::intern(st) } - pub fn check_unused_macros(&self) { + pub fn check_unused_macros(&mut self) { self.resolver.check_unused_macros(); } From ea1ff8c07c3937cab6e8d02c2a9f0fbb508046a5 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 09:20:18 -0400 Subject: [PATCH 4/6] Utilize Resolver lint buffer during HIR lowering --- src/librustc/hir/lowering.rs | 11 +++++++---- src/librustc/session/mod.rs | 10 ++++++---- src/librustc_interface/util.rs | 4 ++-- src/librustc_resolve/lib.rs | 4 ++++ 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 12ab44515c38d..6b6032516ca73 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -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); 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,7 +3349,7 @@ 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() @@ -3354,7 +3357,7 @@ impl<'a> LoweringContext<'a> { .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, diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 81f0853201c08..9ab5b9f13b150 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -366,7 +366,7 @@ impl Session { self.diagnostic().span_note_without_error(sp, msg) } - pub fn buffer_lint>( + pub fn buffer_lint_late>( &self, lint: &'static lint::Lint, id: ast::NodeId, @@ -375,13 +375,13 @@ impl Session { ) { match *self.buffered_lints.borrow_mut() { Some(ref mut buffer) => { - buffer.add_lint(lint, id, sp.into(), msg, BuiltinLintDiagnostics::Normal) + buffer.buffer_lint(lint, id, sp, msg); } None => bug!("can't buffer lints after HIR lowering"), } } - pub fn buffer_lint_with_diagnostic>( + pub fn buffer_lint_with_diagnostic_late>( &self, lint: &'static lint::Lint, id: ast::NodeId, @@ -390,7 +390,9 @@ impl Session { diagnostic: BuiltinLintDiagnostics, ) { match *self.buffered_lints.borrow_mut() { - Some(ref mut buffer) => buffer.add_lint(lint, id, sp.into(), msg, diagnostic), + Some(ref mut buffer) => buffer.buffer_lint_with_diagnostic( + lint, id, sp.into(), msg, diagnostic, + ), None => bug!("can't buffer lints after HIR lowering"), } } diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index 8f11dc9372728..1967465932523 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -559,7 +559,7 @@ pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec Vec hir::lowering::Resolver for Resolver<'a> { let expn_id = self.definitions.expansion_that_defined(def_id.index); self.has_derives(expn_id, derives) } + + fn lint_buffer(&mut self) -> &mut lint::LintBuffer { + &mut self.lint_buffer + } } impl<'a> Resolver<'a> { From c0fdddcb6001b6bd0cd7a6397b9e01e019e553ff Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 13:23:18 -0400 Subject: [PATCH 5/6] Move crate type checking later This allows us to directly pass in a lint buffer --- src/librustc/session/config.rs | 2 +- src/librustc_interface/passes.rs | 2 + src/librustc_interface/util.rs | 109 +++++++++++++++++-------------- 3 files changed, 62 insertions(+), 51 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 33b9ddaa62230..2bcddeaf1962d 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -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 } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index a5b00568b53f5..b6395ccb50078 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -295,6 +295,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() ); diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index 1967465932523..d0c15073f1640 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -526,6 +526,63 @@ pub(crate) fn compute_crate_disambiguator(session: &Session) -> CrateDisambiguat CrateDisambiguator::from(hasher.finish::()) } +pub(crate) fn check_attr_crate_type(attrs: &[ast::Attribute], lint_buffer: &mut lint::LintBuffer) { + // Unconditionally collect crate types from attributes to make them used + for a in attrs.iter() { + if a.check_name(sym::crate_type) { + if let Some(n) = a.value_str() { + if let Some(_) = categorize_crate_type(n) { + return; + } + + if let ast::MetaItemKind::NameValue(spanned) = a.meta().unwrap().kind { + let span = spanned.span; + let lev_candidate = find_best_match_for_name( + CRATE_TYPES.iter().map(|(k, _)| k), + &n.as_str(), + None + ); + if let Some(candidate) = lev_candidate { + lint_buffer.buffer_lint_with_diagnostic( + lint::builtin::UNKNOWN_CRATE_TYPES, + ast::CRATE_NODE_ID, + span, + "invalid `crate_type` value", + lint::builtin::BuiltinLintDiagnostics:: + UnknownCrateTypes( + span, + "did you mean".to_string(), + format!("\"{}\"", candidate) + ) + ); + } else { + lint_buffer.buffer_lint( + lint::builtin::UNKNOWN_CRATE_TYPES, + ast::CRATE_NODE_ID, + span, + "invalid `crate_type` value" + ); + } + } + } + } + } +} + +const CRATE_TYPES: &[(Symbol, config::CrateType)] = &[ + (sym::rlib, config::CrateType::Rlib), + (sym::dylib, config::CrateType::Dylib), + (sym::cdylib, config::CrateType::Cdylib), + (sym::lib, config::default_lib_output()), + (sym::staticlib, config::CrateType::Staticlib), + (sym::proc_dash_macro, config::CrateType::ProcMacro), + (sym::bin, config::CrateType::Executable), +]; + +fn categorize_crate_type(s: Symbol) -> Option { + Some(CRATE_TYPES.iter().find(|(key, _)| *key == s)?.1) +} + pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec { // Unconditionally collect crate types from attributes to make them used let attr_types: Vec = attrs @@ -533,56 +590,8 @@ pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec Some(config::CrateType::Rlib), - Some(sym::dylib) => Some(config::CrateType::Dylib), - Some(sym::cdylib) => Some(config::CrateType::Cdylib), - Some(sym::lib) => Some(config::default_lib_output()), - Some(sym::staticlib) => Some(config::CrateType::Staticlib), - Some(sym::proc_dash_macro) => Some(config::CrateType::ProcMacro), - Some(sym::bin) => Some(config::CrateType::Executable), - Some(n) => { - let crate_types = vec![ - sym::rlib, - sym::dylib, - sym::cdylib, - sym::lib, - sym::staticlib, - sym::proc_dash_macro, - sym::bin - ]; - - if let ast::MetaItemKind::NameValue(spanned) = a.meta().unwrap().kind { - let span = spanned.span; - let lev_candidate = find_best_match_for_name( - crate_types.iter(), - &n.as_str(), - None - ); - if let Some(candidate) = lev_candidate { - session.buffer_lint_with_diagnostic_late( - lint::builtin::UNKNOWN_CRATE_TYPES, - ast::CRATE_NODE_ID, - span, - "invalid `crate_type` value", - lint::builtin::BuiltinLintDiagnostics:: - UnknownCrateTypes( - span, - "did you mean".to_string(), - format!("\"{}\"", candidate) - ) - ); - } else { - session.buffer_lint_late( - lint::builtin::UNKNOWN_CRATE_TYPES, - ast::CRATE_NODE_ID, - span, - "invalid `crate_type` value" - ); - } - } - None - } - None => None + Some(s) => categorize_crate_type(s), + _ => None, } } else { None From c68df7c503676a1f63c0fcc2a7f10597fb93c375 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 13:41:51 -0400 Subject: [PATCH 6/6] Delete lint buffer from Session --- src/librustc/lint/context.rs | 26 ++++++++++----------- src/librustc/lint/levels.rs | 16 +++++++++---- src/librustc/lint/mod.rs | 2 +- src/librustc/session/mod.rs | 40 -------------------------------- src/librustc_interface/passes.rs | 12 +++++----- src/librustc_resolve/lib.rs | 11 +++++---- 6 files changed, 39 insertions(+), 68 deletions(-) diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 1cb53d754dcd3..eef1cee8db37a 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -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( 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( lint_store: &LintStore, krate: &ast::Crate, pre_expansion: bool, + lint_buffer: Option, 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( krate, EarlyLintPassObjects { lints: &mut passes[..] }, buffered, + pre_expansion, ); } } else { @@ -1549,6 +1548,7 @@ pub fn check_ast_crate( krate, EarlyLintPassObjects { lints: slice::from_mut(pass) }, buffered, + pre_expansion, ) }); } diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 4c60492e470c2..e470dbdf3239b 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -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, } } diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 42f33740b23e1..11d0d0d90fa86 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -795,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, }; diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 9ab5b9f13b150..13b76b79b3d82 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -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>, - /// Set of `(DiagnosticId, Option, 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,37 +358,6 @@ impl Session { self.diagnostic().span_note_without_error(sp, msg) } - pub fn buffer_lint_late>( - &self, - lint: &'static lint::Lint, - id: ast::NodeId, - sp: S, - msg: &str, - ) { - match *self.buffered_lints.borrow_mut() { - Some(ref mut buffer) => { - buffer.buffer_lint(lint, id, sp, msg); - } - None => bug!("can't buffer lints after HIR lowering"), - } - } - - pub fn buffer_lint_with_diagnostic_late>( - &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.buffer_lint_with_diagnostic( - 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(); @@ -1220,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()), diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index b6395ccb50078..52332744d1ad5 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -267,17 +267,16 @@ fn configure_and_expand_inner<'a>( lint_store, &krate, true, + None, rustc_lint::BuiltinCombinedPreExpansionLintPass::new()); }); - let lint_buffer = lint::LintBuffer::default(); let mut resolver = Resolver::new( sess, &krate, crate_name, metadata_loader, &resolver_arenas, - lint_buffer, ); syntax_ext::register_builtin_macros(&mut resolver, sess.edition()); @@ -295,7 +294,7 @@ fn configure_and_expand_inner<'a>( krate }); - util::check_attr_crate_type(&krate.attrs, &mut resolver.lint_buffer); + 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() @@ -370,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"; - resolver.lint_buffer.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); @@ -399,7 +398,7 @@ fn configure_and_expand_inner<'a>( } let has_proc_macro_decls = time(sess, "AST validation", || { - ast_validation::check_crate(sess, &krate, &mut resolver.lint_buffer) + ast_validation::check_crate(sess, &krate, &mut resolver.lint_buffer()) }); @@ -468,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); - resolver.lint_buffer.buffer_lint(lint, id, span, &msg); + resolver.lint_buffer().buffer_lint(lint, id, span, &msg); } }); @@ -500,6 +499,7 @@ pub fn lower_to_hir( lint_store, &krate, false, + Some(std::mem::take(resolver.lint_buffer())), rustc_lint::BuiltinCombinedEarlyLintPass::new(), ) }); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index baeabbf3c1d18..b45eb356bdbd0 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -963,7 +963,7 @@ pub struct Resolver<'a> { /// when visiting the correspondent variants. variant_vis: DefIdMap, - pub lint_buffer: lint::LintBuffer, + lint_buffer: lint::LintBuffer, } /// Nothing really interesting here; it just provides memory for the rest of the crate. @@ -1094,8 +1094,7 @@ impl<'a> Resolver<'a> { krate: &Crate, crate_name: &str, metadata_loader: &'a MetadataLoaderDyn, - arenas: &'a ResolverArenas<'a>, - lint_buffer: lint::LintBuffer) + arenas: &'a ResolverArenas<'a>) -> Resolver<'a> { let root_def_id = DefId::local(CRATE_DEF_INDEX); let root_module_kind = ModuleKind::Def( @@ -1235,10 +1234,14 @@ impl<'a> Resolver<'a> { .chain(features.declared_lang_features.iter().map(|(feat, ..)| *feat)) .collect(), variant_vis: Default::default(), - lint_buffer, + lint_buffer: lint::LintBuffer::default(), } } + pub fn lint_buffer(&mut self) -> &mut lint::LintBuffer { + &mut self.lint_buffer + } + pub fn arenas() -> ResolverArenas<'a> { Default::default() }