From 055ce62a1dfcd1f8d7b6dc224bf14af09ede39de Mon Sep 17 00:00:00 2001
From: Markus Westerlind <markus.westerlind@imperva.com>
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 44258b62ca32..06652df5a2dc 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<S: Into<MultiSpan>>(&self, lint: &'static Lint, span: Option<S>, msg: &str) {
-        self.lookup(lint, span, msg).emit();
+    fn lookup_and_emit<S: Into<MultiSpan>>(
+        &self,
+        lint: &'static Lint,
+        span: Option<S>,
+        msg: impl std::fmt::Display,
+    ) {
+        self.lookup(lint, span, &msg).emit();
     }
 
     fn lookup_and_emit_with_diagnostics<S: Into<MultiSpan>>(
@@ -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<S>,
-        msg: &str,
+        msg: impl std::fmt::Display,
     ) -> DiagnosticBuilder<'_>;
 
     /// Emit a lint at the appropriate level, for a particular span.
-    fn span_lint<S: Into<MultiSpan>>(&self, lint: &'static Lint, span: S, msg: &str) {
+    fn span_lint<S: Into<MultiSpan>>(
+        &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<S>,
-        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<S>,
-        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 edf7df16c87f..8981660ab07e 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<MultiSpan>,
-        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 f4684bd52224..d671c2b2c9a3 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<MultiSpan>,
-    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<MultiSpan>,
+    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 b84b0e4f45dc..64666b39ed4e 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 0f7bdaaf582e..0831a57ee61e 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<T>` 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);