Skip to content

Commit

Permalink
perf: Delay formatting of lint messages until they are know to be used
Browse files Browse the repository at this point in the history
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!`.
  • Loading branch information
Marwes committed Dec 31, 2019
1 parent 580ac0b commit 055ce62
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 36 deletions.
32 changes: 21 additions & 11 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>(
Expand All @@ -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();
}
Expand All @@ -517,21 +522,26 @@ 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);
}

fn struct_span_lint<S: Into<MultiSpan>>(
&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.
Expand All @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -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;

Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 19 additions & 6 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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, \
Expand Down
12 changes: 6 additions & 6 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
}
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
27 changes: 15 additions & 12 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);
}
}
}
Expand Down Expand Up @@ -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",
);
}
}
Expand Down Expand Up @@ -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),
);
}
}
Expand Down Expand Up @@ -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`",
)
}
}
Expand Down Expand Up @@ -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",
)
}
}
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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",
);
}

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

0 comments on commit 055ce62

Please sign in to comment.