Skip to content

Commit

Permalink
Auto merge of #68725 - jumbatm:invert-control-in-struct_lint_level, r…
Browse files Browse the repository at this point in the history 
…=Centril

Invert control in struct_lint_level.

Closes #67927

Changes the `struct_lint*` methods to take a  `decorate` function instead of a message string. This decorate function is also responsible for eventually stashing, emitting or cancelling the diagnostic. If the lint was allowed after all, the decorate function is not run at all, saving us from spending time formatting messages (and potentially other expensive work) for lints that don't end up being emitted.

r? @Centril
  • Loading branch information
@bors
bors committed Feb 11, 2020
2 parents b6690a8 + b959da2 commit 95e0a2c
Show file tree
Hide file tree
Showing 59 changed files with 1,795 additions and 1,611 deletions.
2 changes: 1 addition & 1 deletion src/librustc/infer/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
sub: Region<'tcx>,
) {
self.construct_generic_bound_failure(region_scope_tree, span, origin, bound_kind, sub)
.emit()
.emit();
}

pub fn construct_generic_bound_failure(
Expand Down
244 changes: 138 additions & 106 deletions src/librustc/lint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,132 +174,164 @@ impl<'a> HashStable<StableHashingContext<'a>> for LintLevelMap {
}
}

pub fn struct_lint_level<'a>(
sess: &'a Session,
pub struct LintDiagnosticBuilder<'a>(DiagnosticBuilder<'a>);

impl<'a> LintDiagnosticBuilder<'a> {
/// Return the inner DiagnosticBuilder, first setting the primary message to `msg`.
pub fn build(mut self, msg: &str) -> DiagnosticBuilder<'a> {
self.0.set_primary_message(msg);
self.0
}

/// Create a LintDiagnosticBuilder from some existing DiagnosticBuilder.
pub fn new(err: DiagnosticBuilder<'a>) -> LintDiagnosticBuilder<'a> {
LintDiagnosticBuilder(err)
}
}

pub fn struct_lint_level<'s, 'd>(
sess: &'s Session,
lint: &'static Lint,
level: Level,
src: LintSource,
span: Option<MultiSpan>,
msg: &str,
) -> 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),
};
decorate: impl for<'a> FnOnce(LintDiagnosticBuilder<'a>) + 'd,
) {
// Avoid codegen bloat from monomorphization by immediately doing dyn dispatch of `decorate` to
// the "real" work.
fn struct_lint_level_impl(
sess: &'s Session,
lint: &'static Lint,
level: Level,
src: LintSource,
span: Option<MultiSpan>,
decorate: Box<dyn for<'b> FnOnce(LintDiagnosticBuilder<'b>) + 'd>,
) {
let mut err = match (level, span) {
(Level::Allow, _) => {
return;
}
(Level::Warn, Some(span)) => sess.struct_span_warn(span, ""),
(Level::Warn, None) => sess.struct_warn(""),
(Level::Deny, Some(span)) | (Level::Forbid, Some(span)) => {
sess.struct_span_err(span, "")
}
(Level::Deny, None) | (Level::Forbid, None) => sess.struct_err(""),
};

// Check for future incompatibility lints and issue a stronger warning.
let lint_id = LintId::of(lint);
let future_incompatible = lint.future_incompatible;

// If this code originates in a foreign macro, aka something that this crate
// did not itself author, then it's likely that there's nothing this crate
// can do about it. We probably want to skip the lint entirely.
if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
// Any suggestions made here are likely to be incorrect, so anything we
// emit shouldn't be automatically fixed by rustfix.
err.allow_suggestions(false);

// If this is a future incompatible lint it'll become a hard error, so
// we have to emit *something*. Also allow lints to whitelist themselves
// on a case-by-case basis for emission in a foreign macro.
if future_incompatible.is_none() && !lint.report_in_external_macro {
err.cancel();
// Don't continue further, since we don't want to have
// `diag_span_note_once` called for a diagnostic that isn't emitted.
return err;
// Check for future incompatibility lints and issue a stronger warning.
let lint_id = LintId::of(lint);
let future_incompatible = lint.future_incompatible;

// If this code originates in a foreign macro, aka something that this crate
// did not itself author, then it's likely that there's nothing this crate
// can do about it. We probably want to skip the lint entirely.
if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
// Any suggestions made here are likely to be incorrect, so anything we
// emit shouldn't be automatically fixed by rustfix.
err.allow_suggestions(false);

// If this is a future incompatible lint it'll become a hard error, so
// we have to emit *something*. Also allow lints to whitelist themselves
// on a case-by-case basis for emission in a foreign macro.
if future_incompatible.is_none() && !lint.report_in_external_macro {
err.cancel();
// Don't continue further, since we don't want to have
// `diag_span_note_once` called for a diagnostic that isn't emitted.
return;
}
}
}

let name = lint.name_lower();
match src {
LintSource::Default => {
sess.diag_note_once(
&mut err,
DiagnosticMessageId::from(lint),
&format!("`#[{}({})]` on by default", level.as_str(), name),
);
}
LintSource::CommandLine(lint_flag_val) => {
let flag = match level {
Level::Warn => "-W",
Level::Deny => "-D",
Level::Forbid => "-F",
Level::Allow => panic!(),
};
let hyphen_case_lint_name = name.replace("_", "-");
if lint_flag_val.as_str() == name {
let name = lint.name_lower();
match src {
LintSource::Default => {
sess.diag_note_once(
&mut err,
DiagnosticMessageId::from(lint),
&format!(
"requested on the command line with `{} {}`",
flag, hyphen_case_lint_name
),
);
} else {
let hyphen_case_flag_val = lint_flag_val.as_str().replace("_", "-");
sess.diag_note_once(
&mut err,
DiagnosticMessageId::from(lint),
&format!(
"`{} {}` implied by `{} {}`",
flag, hyphen_case_lint_name, flag, hyphen_case_flag_val
),
&format!("`#[{}({})]` on by default", level.as_str(), name),
);
}
}
LintSource::Node(lint_attr_name, src, reason) => {
if let Some(rationale) = reason {
err.note(&rationale.as_str());
LintSource::CommandLine(lint_flag_val) => {
let flag = match level {
Level::Warn => "-W",
Level::Deny => "-D",
Level::Forbid => "-F",
Level::Allow => panic!(),
};
let hyphen_case_lint_name = name.replace("_", "-");
if lint_flag_val.as_str() == name {
sess.diag_note_once(
&mut err,
DiagnosticMessageId::from(lint),
&format!(
"requested on the command line with `{} {}`",
flag, hyphen_case_lint_name
),
);
} else {
let hyphen_case_flag_val = lint_flag_val.as_str().replace("_", "-");
sess.diag_note_once(
&mut err,
DiagnosticMessageId::from(lint),
&format!(
"`{} {}` implied by `{} {}`",
flag, hyphen_case_lint_name, flag, hyphen_case_flag_val
),
);
}
}
sess.diag_span_note_once(
&mut err,
DiagnosticMessageId::from(lint),
src,
"the lint level is defined here",
);
if lint_attr_name.as_str() != name {
let level_str = level.as_str();
sess.diag_note_once(
LintSource::Node(lint_attr_name, src, reason) => {
if let Some(rationale) = reason {
err.note(&rationale.as_str());
}
sess.diag_span_note_once(
&mut err,
DiagnosticMessageId::from(lint),
&format!(
"`#[{}({})]` implied by `#[{}({})]`",
level_str, name, level_str, lint_attr_name
),
src,
"the lint level is defined here",
);
if lint_attr_name.as_str() != name {
let level_str = level.as_str();
sess.diag_note_once(
&mut err,
DiagnosticMessageId::from(lint),
&format!(
"`#[{}({})]` implied by `#[{}({})]`",
level_str, name, level_str, lint_attr_name
),
);
}
}
}
}

err.code(DiagnosticId::Lint(name));

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";

let explanation = if lint_id == LintId::of(builtin::UNSTABLE_NAME_COLLISIONS) {
"once this method is added to the standard library, \
the ambiguity may cause an error or change in behavior!"
.to_owned()
} else if lint_id == LintId::of(builtin::MUTABLE_BORROW_RESERVATION_CONFLICT) {
"this borrowing pattern was not meant to be accepted, \
and may become a hard error in the future"
.to_owned()
} else if let Some(edition) = future_incompatible.edition {
format!("{} in the {} edition!", STANDARD_MESSAGE, edition)
} else {
format!("{} in a future release!", STANDARD_MESSAGE)
};
let citation = format!("for more information, see {}", future_incompatible.reference);
err.warn(&explanation);
err.note(&citation);
}
err.code(DiagnosticId::Lint(name));

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";

let explanation = if lint_id == LintId::of(builtin::UNSTABLE_NAME_COLLISIONS) {
"once this method is added to the standard library, \
the ambiguity may cause an error or change in behavior!"
.to_owned()
} else if lint_id == LintId::of(builtin::MUTABLE_BORROW_RESERVATION_CONFLICT) {
"this borrowing pattern was not meant to be accepted, \
and may become a hard error in the future"
.to_owned()
} else if let Some(edition) = future_incompatible.edition {
format!("{} in the {} edition!", STANDARD_MESSAGE, edition)
} else {
format!("{} in a future release!", STANDARD_MESSAGE)
};
let citation = format!("for more information, see {}", future_incompatible.reference);
err.warn(&explanation);
err.note(&citation);
}

return err;
// Finally, run `decorate`. This function is also responsible for emitting the diagnostic.
decorate(LintDiagnosticBuilder::new(err));
}
struct_lint_level_impl(sess, lint, level, src, span, Box::new(decorate))
}

/// Returns whether `span` originates in a foreign crate's external macro.
Expand Down
19 changes: 12 additions & 7 deletions src/librustc/middle/stability.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,13 @@ fn late_report_deprecation(
return;
}

let mut diag = tcx.struct_span_lint_hir(lint, hir_id, span, message);
if let hir::Node::Expr(_) = tcx.hir().get(hir_id) {
deprecation_suggestion(&mut diag, suggestion, span);
}
diag.emit();
tcx.struct_span_lint_hir(lint, hir_id, span, |lint| {
let mut diag = lint.build(message);
if let hir::Node::Expr(_) = tcx.hir().get(hir_id) {
deprecation_suggestion(&mut diag, suggestion, span);
}
diag.emit()
});
if hir_id == hir::DUMMY_HIR_ID {
span_bug!(span, "emitted a {} lint with dummy HIR id: {:?}", lint.name, def_id);
}
Expand Down Expand Up @@ -387,8 +389,11 @@ impl<'tcx> TyCtxt<'tcx> {
/// Additionally, this function will also check if the item is deprecated. If so, and `id` is
/// not `None`, a deprecated lint attached to `id` will be emitted.
pub fn check_stability(self, def_id: DefId, id: Option<HirId>, span: Span) {
let soft_handler =
|lint, span, msg: &_| self.lint_hir(lint, id.unwrap_or(hir::CRATE_HIR_ID), span, msg);
let soft_handler = |lint, span, msg: &_| {
self.struct_span_lint_hir(lint, id.unwrap_or(hir::CRATE_HIR_ID), span, |lint| {
lint.build(msg).emit()
})
};
match self.eval_stability(def_id, id, span) {
EvalResult::Allow => {}
EvalResult::Deny { feature, reason, issue, is_soft } => {
Expand Down
Loading

0 comments on commit 95e0a2c

Please sign in to comment.