From 055ce62a1dfcd1f8d7b6dc224bf14af09ede39de Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 31 Dec 2019 14:24:56 +0100 Subject: [PATCH] perf: Delay formatting of lint messages until they are know to be used Found `check` builts in one of my crates spent 5% of the time just formatting types for the sake of the `BoxPointers` lint, only for the strings to get thrown away immediately since it is an `allow` lint. This does the bare minimum to instead pass a `fmt::Display` instance to the lint so that `msg` may only be created if it is needed. Unsure if this is the desired way to solve this problem so I didn't go through the lint system throughly to see if there were more places that should take/pass `format_args!` instances instead of using `format!`. --- src/librustc/lint/context.rs | 32 +++++++++++++++++++++----------- src/librustc/lint/levels.rs | 2 +- src/librustc/lint/mod.rs | 25 +++++++++++++++++++------ src/librustc/ty/context.rs | 12 ++++++------ src/librustc_lint/builtin.rs | 27 +++++++++++++++------------ 5 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 44258b62ca32f..06652df5a2dc3 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -497,8 +497,13 @@ pub trait LintContext: Sized { fn sess(&self) -> &Session; fn lints(&self) -> &LintStore; - fn lookup_and_emit>(&self, lint: &'static Lint, span: Option, msg: &str) { - self.lookup(lint, span, msg).emit(); + fn lookup_and_emit>( + &self, + lint: &'static Lint, + span: Option, + msg: impl std::fmt::Display, + ) { + self.lookup(lint, span, &msg).emit(); } fn lookup_and_emit_with_diagnostics>( @@ -508,7 +513,7 @@ pub trait LintContext: Sized { msg: &str, diagnostic: BuiltinLintDiagnostics, ) { - let mut db = self.lookup(lint, span, msg); + let mut db = self.lookup(lint, span, &msg); diagnostic.run(self.sess(), &mut db); db.emit(); } @@ -517,11 +522,16 @@ pub trait LintContext: Sized { &self, lint: &'static Lint, span: Option, - msg: &str, + msg: impl std::fmt::Display, ) -> DiagnosticBuilder<'_>; /// Emit a lint at the appropriate level, for a particular span. - fn span_lint>(&self, lint: &'static Lint, span: S, msg: &str) { + fn span_lint>( + &self, + lint: &'static Lint, + span: S, + msg: impl std::fmt::Display, + ) { self.lookup_and_emit(lint, Some(span), msg); } @@ -529,9 +539,9 @@ pub trait LintContext: Sized { &self, lint: &'static Lint, span: S, - msg: &str, + msg: impl std::fmt::Display, ) -> DiagnosticBuilder<'_> { - self.lookup(lint, Some(span), msg) + self.lookup(lint, Some(span), &msg) } /// Emit a lint and note at the appropriate level, for a particular span. @@ -543,7 +553,7 @@ pub trait LintContext: Sized { note_span: Span, note: &str, ) { - let mut err = self.lookup(lint, Some(span), msg); + let mut err = self.lookup(lint, Some(span), &msg); if note_span == span { err.note(note); } else { @@ -554,7 +564,7 @@ pub trait LintContext: Sized { /// Emit a lint and help at the appropriate level, for a particular span. fn span_lint_help(&self, lint: &'static Lint, span: Span, msg: &str, help: &str) { - let mut err = self.lookup(lint, Some(span), msg); + let mut err = self.lookup(lint, Some(span), &msg); self.span_lint(lint, span, msg); err.span_help(span, help); err.emit(); @@ -646,7 +656,7 @@ impl LintContext for LateContext<'_, '_> { &self, lint: &'static Lint, span: Option, - msg: &str, + msg: impl std::fmt::Display, ) -> DiagnosticBuilder<'_> { let hir_id = self.last_node_with_lint_attrs; @@ -673,7 +683,7 @@ impl LintContext for EarlyContext<'_> { &self, lint: &'static Lint, span: Option, - msg: &str, + msg: impl std::fmt::Display, ) -> DiagnosticBuilder<'_> { self.builder.struct_lint(lint, span.map(|s| s.into()), msg) } diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index edf7df16c87fa..8981660ab07ec 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -471,7 +471,7 @@ impl<'a> LintLevelsBuilder<'a> { &self, lint: &'static Lint, span: Option, - msg: &str, + msg: impl std::fmt::Display, ) -> DiagnosticBuilder<'a> { let (level, src) = self.sets.get_lint_level(lint, self.cur, None, self.sess); lint::struct_lint_level(self.sess, lint, level, src, span, msg) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index f4684bd52224f..d671c2b2c9a3d 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -437,14 +437,27 @@ pub fn struct_lint_level<'a>( level: Level, src: LintSource, span: Option, - msg: &str, + msg: impl std::fmt::Display, +) -> DiagnosticBuilder<'a> { + struct_lint_level_(sess, lint, level, src, span, &msg) +} + +fn struct_lint_level_<'a>( + sess: &'a Session, + lint: &'static Lint, + level: Level, + src: LintSource, + span: Option, + msg: &dyn std::fmt::Display, ) -> DiagnosticBuilder<'a> { let mut err = match (level, span) { (Level::Allow, _) => return sess.diagnostic().struct_dummy(), - (Level::Warn, Some(span)) => sess.struct_span_warn(span, msg), - (Level::Warn, None) => sess.struct_warn(msg), - (Level::Deny, Some(span)) | (Level::Forbid, Some(span)) => sess.struct_span_err(span, msg), - (Level::Deny, None) | (Level::Forbid, None) => sess.struct_err(msg), + (Level::Warn, Some(span)) => sess.struct_span_warn(span, &msg.to_string()), + (Level::Warn, None) => sess.struct_warn(&msg.to_string()), + (Level::Deny, Some(span)) | (Level::Forbid, Some(span)) => { + sess.struct_span_err(span, &msg.to_string()) + } + (Level::Deny, None) | (Level::Forbid, None) => sess.struct_err(&msg.to_string()), }; // Check for future incompatibility lints and issue a stronger warning. @@ -536,7 +549,7 @@ pub fn struct_lint_level<'a>( if let Some(future_incompatible) = future_incompatible { const STANDARD_MESSAGE: &str = "this was previously accepted by the compiler but is being phased out; \ - it will become a hard error"; + it will become a hard error"; let explanation = if lint_id == LintId::of(builtin::UNSTABLE_NAME_COLLISIONS) { "once this method is added to the standard library, \ diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index b84b0e4f45dcb..64666b39ed4ea 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -211,7 +211,7 @@ fn validate_hir_id_for_typeck_tables( ty::tls::with(|tcx| { bug!( "node {} with HirId::owner {:?} cannot be placed in \ - TypeckTables with local_id_root {:?}", + TypeckTables with local_id_root {:?}", tcx.hir().node_to_string(hir_id), DefId::local(hir_id.owner), local_id_root @@ -2553,7 +2553,7 @@ impl<'tcx> TyCtxt<'tcx> { lint: &'static Lint, hir_id: HirId, span: S, - msg: &str, + msg: impl std::fmt::Display, ) { self.struct_span_lint_hir(lint, hir_id, span.into(), msg).emit() } @@ -2563,7 +2563,7 @@ impl<'tcx> TyCtxt<'tcx> { lint: &'static Lint, hir_id: HirId, span: S, - msg: &str, + msg: impl std::fmt::Display, note: &str, ) { let mut err = self.struct_span_lint_hir(lint, hir_id, span.into(), msg); @@ -2576,7 +2576,7 @@ impl<'tcx> TyCtxt<'tcx> { lint: &'static Lint, id: hir::HirId, span: S, - msg: &str, + msg: impl std::fmt::Display, note: &str, ) { let mut err = self.struct_span_lint_hir(lint, id, span.into(), msg); @@ -2629,7 +2629,7 @@ impl<'tcx> TyCtxt<'tcx> { lint: &'static Lint, hir_id: HirId, span: S, - msg: &str, + msg: impl std::fmt::Display, ) -> DiagnosticBuilder<'tcx> { let (level, src) = self.lint_level_at_node(lint, hir_id); lint::struct_lint_level(self.sess, lint, level, src, Some(span.into()), msg) @@ -2639,7 +2639,7 @@ impl<'tcx> TyCtxt<'tcx> { self, lint: &'static Lint, id: HirId, - msg: &str, + msg: impl std::fmt::Display, ) -> DiagnosticBuilder<'tcx> { let (level, src) = self.lint_level_at_node(lint, id); lint::struct_lint_level(self.sess, lint, level, src, None, msg) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 0f7bdaaf582e1..0831a57ee61eb 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -109,8 +109,11 @@ impl BoxPointers { fn check_heap_type(&self, cx: &LateContext<'_, '_>, span: Span, ty: Ty<'_>) { for leaf_ty in ty.walk() { if leaf_ty.is_box() { - let m = format!("type uses owned (Box type) pointers: {}", ty); - cx.span_lint(BOX_POINTERS, span, &m); + cx.span_lint( + BOX_POINTERS, + span, + format_args!("type uses owned (Box type) pointers: {}", ty), + ); } } } @@ -235,8 +238,8 @@ impl EarlyLintPass for UnsafeCode { cx, attr.span, "`allow_internal_unsafe` allows defining \ - macros using unsafe without triggering \ - the `unsafe_code` lint at their call site", + macros using unsafe without triggering \ + the `unsafe_code` lint at their call site", ); } } @@ -379,7 +382,7 @@ impl MissingDoc { cx.span_lint( MISSING_DOCS, cx.tcx.sess.source_map().def_span(sp), - &format!("missing documentation for {}", desc), + format_args!("missing documentation for {}", desc), ); } } @@ -563,7 +566,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingCopyImplementations { MISSING_COPY_IMPLEMENTATIONS, item.span, "type could implement `Copy`; consider adding `impl \ - Copy`", + Copy`", ) } } @@ -617,7 +620,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDebugImplementations { MISSING_DEBUG_IMPLEMENTATIONS, item.span, "type does not implement `fmt::Debug`; consider adding `#[derive(Debug)]` \ - or a manual implementation", + or a manual implementation", ) } } @@ -663,7 +666,7 @@ impl EarlyLintPass for AnonymousParameters { .span_suggestion( arg.pat.span, "Try naming the parameter or explicitly \ - ignoring it", + ignoring it", format!("_: {}", ty_snip), appl, ) @@ -783,7 +786,7 @@ impl UnusedDocComment { if is_macro_expansion { err.help( "to document an item produced by a macro, \ - the macro must produce the documentation as part of its expansion", + the macro must produce the documentation as part of its expansion", ); } @@ -1249,7 +1252,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TrivialConstraints { span, &format!( "{} bound {} does not depend on any type \ - or lifetime parameters", + or lifetime parameters", predicate_kind_name, predicate ), ); @@ -1473,7 +1476,7 @@ impl KeywordIdents { let mut lint = cx.struct_span_lint( KEYWORD_IDENTS, ident.span, - &format!("`{}` is a keyword in the {} edition", ident, next_edition), + format_args!("`{}` is a keyword in the {} edition", ident, next_edition), ); lint.span_suggestion( ident.span, @@ -2033,7 +2036,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { err.span_label( expr.span, "help: use `MaybeUninit` instead, \ - and only call `assume_init` after initialization is done", + and only call `assume_init` after initialization is done", ); if let Some(span) = span { err.span_note(span, &msg);