Skip to content

Commit

Permalink
Auto merge of rust-lang#125208 - fmease:lint-diags-store-their-span, …
Browse files Browse the repository at this point in the history
…r=<try>

Make lint diagnostics responsible for providing their primary span

### Summary and Rustc-Dev-Facing Changes

Instead of passing the primary span of lint diagnostics separately — namely to the functions `tcx.emit_span_lint` and `tcx.emit_node_span_lint` —, make lint diagnostics responsible for “storing” (providing) their span. I've removed the two aforementioned methods. You are now expected to use `tcx.emit_lint` and `tcx.emit_node_lint` respectively and to provide the primary span when deriving `LintDiagnostic` (via `#[primary_span]`) / implementing `LintDiagnostic` manually (via `LintDiagnostic::span`).

### Motivation

Making the general format of `#[derive(LintDiagnostic)]` identical to `#[derive(Diagnostic)]`. I bet this has lead to slight confusion or annoyances before. Moreover, it enables deriving both traits at once (see rust-lang#125169 for the motivation).

### Approach

As outlined in rust-lang#125169 (comment), the naïve approach of doing the same as `Diagnostic` doesn't work for `LintDiagnostic`, i.e., setting the primary span with `Diag::span` inside `decorate_lint` (that's `into_diag` for `Diagnostic`) because `lint_level` (`lint_level_impl`) needs access to the primary spans before decorating the `Diag` to be able to filter out some lints (namely if they come from an external macro) etc.

Therefore, I had to introduce a new method to `LintDiagnostic`: `fn span(&self) -> Option<MultiSpan>` (similar to the existing method `LintDiagnostic::msg`).

### Commits

1. The 1st commit addresses [rust-lang#125169 (comment)](rust-lang#125169 (comment)). It contains the necessary changes to the linting APIs. It doesn't build on its own due to the removed APIs. Split out for easier reviewing.
2. The 2nd commit updates all existing lint diagnostics to use the new APIs. Most of them are mindless, monotonous, obvious changes. Some changes are *slightly* more involved out of necessity. I've verified (partly programmatically, partly manually) that all lint diags that used to have a primary span still have a primary span.
3. The 3rd commit updates the fulldeps UI tests that exercise the (lint) diagnostic API
4. The 4th commit fixes [rust-lang#125169](rust-lang#125169). I've only deduplicated the lint diagnostic structs mentioned in the issue and haven't gone looking for other structs that may benefit from deriving both `Diagnostic` and `LintDiagnostic` because I didn't have the time to do so yet.

### Meta

Fixes rust-lang#125169.

I'll submit a rustc-dev-guide PR later.
I hope this doesn't need an MCP, it's pretty straight forward.

cc `@davidtwco`
r? `@nnethercote` or anyone on compiler who has the energy to review 82 files
  • Loading branch information
bors committed Oct 24, 2024
2 parents f61306d + b22eadf commit 9de008a
Show file tree
Hide file tree
Showing 96 changed files with 1,654 additions and 1,250 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ fn do_mir_borrowck<'tcx>(

let mut_span = tcx.sess.source_map().span_until_non_whitespace(span);

tcx.emit_node_span_lint(UNUSED_MUT, lint_root, span, VarNeedNotMut { span: mut_span })
tcx.emit_node_lint(UNUSED_MUT, lint_root, VarNeedNotMut { span, mut_span })
}

let tainted_by_errors = mbcx.emit_errors();
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_borrowck/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ pub(crate) struct GenericDoesNotLiveLongEnough {
#[derive(LintDiagnostic)]
#[diag(borrowck_var_does_not_need_mut)]
pub(crate) struct VarNeedNotMut {
#[suggestion(style = "short", applicability = "machine-applicable", code = "")]
#[primary_span]
pub span: Span,
#[suggestion(style = "short", applicability = "machine-applicable", code = "")]
pub mut_span: Span,
}
#[derive(Diagnostic)]
#[diag(borrowck_var_cannot_escape_closure)]
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,11 @@ pub(super) fn lint<'tcx, L>(
tcx: TyCtxtAt<'tcx>,
machine: &CompileTimeMachine<'tcx>,
lint: &'static rustc_session::lint::Lint,
decorator: impl FnOnce(Vec<errors::FrameNote>) -> L,
decorator: impl FnOnce(Span, Vec<errors::FrameNote>) -> L,
) where
L: for<'a> rustc_errors::LintDiagnostic<'a, ()>,
{
let (span, frames) = get_span_and_frames(tcx, &machine.stack);

tcx.emit_node_span_lint(lint, machine.best_lint_scope(*tcx), span, decorator(frames));
tcx.emit_node_lint(lint, machine.best_lint_scope(*tcx), decorator(span, frames));
}
5 changes: 2 additions & 3 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,11 +622,10 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
.0
.is_error();
let span = ecx.cur_span();
ecx.tcx.emit_node_span_lint(
ecx.tcx.emit_node_lint(
rustc_session::lint::builtin::LONG_RUNNING_CONST_EVAL,
hir_id,
span,
LongRunning { item_span: ecx.tcx.span },
LongRunning { span, item_span: ecx.tcx.span },
);
// If this was a hard error, don't bother continuing evaluation.
if is_error {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ pub(crate) struct InteriorMutableRefEscaping {
#[diag(const_eval_long_running)]
#[note]
pub struct LongRunning {
#[primary_span]
pub span: Span,
#[help]
pub item_span: Span,
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,10 @@ pub trait SubdiagMessageOp<G: EmissionGuarantee> =
/// `#[derive(LintDiagnostic)]` -- see [rustc_macros::LintDiagnostic].
#[rustc_diagnostic_item = "LintDiagnostic"]
pub trait LintDiagnostic<'a, G: EmissionGuarantee> {
/// Decorate and emit a lint.
/// Decorate a lint.
fn decorate_lint<'b>(self, diag: &'b mut Diag<'a, G>);

fn span(&self) -> Option<MultiSpan>;
}

#[derive(Clone, Debug, Encodable, Decodable)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,11 @@ fn report_mismatched_rpitit_signature<'tcx>(
});

let span = unmatched_bound.unwrap_or(span);
tcx.emit_node_span_lint(
tcx.emit_node_lint(
if is_internal { REFINING_IMPL_TRAIT_INTERNAL } else { REFINING_IMPL_TRAIT_REACHABLE },
tcx.local_def_id_to_hir_id(impl_m_def_id.expect_local()),
span,
crate::errors::ReturnPositionImplTraitInTraitRefined {
span,
impl_return_span,
trait_return_span,
pre,
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_hir_analysis/src/check/errs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ fn handle_static_mut_ref(
} else {
(errors::MutRefSugg::Shared { lo, hi }, "shared")
};
tcx.emit_node_span_lint(STATIC_MUT_REFS, hir_id, span, errors::RefOfMutStatic {
span,
sugg,
shared,
});
tcx.emit_node_lint(STATIC_MUT_REFS, hir_id, errors::RefOfMutStatic { span, sugg, shared });
}
}
7 changes: 4 additions & 3 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2334,11 +2334,10 @@ fn lint_redundant_lifetimes<'tcx>(
&& outlives_env.free_region_map().sub_free_regions(tcx, victim, candidate)
{
shadowed.insert(victim);
tcx.emit_node_span_lint(
tcx.emit_node_lint(
rustc_lint_defs::builtin::REDUNDANT_LIFETIMES,
tcx.local_def_id_to_hir_id(def_id.expect_local()),
tcx.def_span(def_id),
RedundantLifetimeArgsLint { candidate, victim },
RedundantLifetimeArgsLint { span: tcx.def_span(def_id), candidate, victim },
);
}
}
Expand All @@ -2349,6 +2348,8 @@ fn lint_redundant_lifetimes<'tcx>(
#[diag(hir_analysis_redundant_lifetime_args)]
#[note]
struct RedundantLifetimeArgsLint<'tcx> {
#[primary_span]
pub span: Span,
/// The lifetime we have found to be redundant.
victim: ty::Region<'tcx>,
// The lifetime we can replace the victim with.
Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_hir_analysis/src/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,18 +494,18 @@ fn lint_uncovered_ty_params<'tcx>(
let name = tcx.item_name(param_def_id);

match local_ty {
Some(local_type) => tcx.emit_node_span_lint(
Some(local_type) => tcx.emit_node_lint(
UNCOVERED_PARAM_IN_PROJECTION,
hir_id,
span,
errors::TyParamFirstLocalLint { span, note: (), param: name, local_type },
),
None => tcx.emit_node_span_lint(
UNCOVERED_PARAM_IN_PROJECTION,
hir_id,
span,
errors::TyParamSomeLint { span, note: (), param: name },
errors::TyParamFirstLocal { span, note: (), param: name, local_type },
),
None => {
tcx.emit_node_lint(UNCOVERED_PARAM_IN_PROJECTION, hir_id, errors::TyParamSome {
span,
note: (),
param: name,
})
}
};
}
}
Expand Down
32 changes: 5 additions & 27 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,7 @@ pub(crate) enum LateBoundInApit {
#[diag(hir_analysis_unused_associated_type_bounds)]
#[note]
pub(crate) struct UnusedAssociatedTypeBounds {
#[primary_span]
#[suggestion(code = "")]
pub span: Span,
}
Expand All @@ -1141,6 +1142,8 @@ pub(crate) struct UnusedAssociatedTypeBounds {
#[note]
#[note(hir_analysis_feedback_note)]
pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> {
#[primary_span]
pub span: Span,
#[suggestion(applicability = "maybe-incorrect", code = "{pre}{return_ty}{post}")]
pub impl_return_span: Span,
#[label]
Expand Down Expand Up @@ -1379,9 +1382,7 @@ pub(crate) struct CrossCrateTraitsDefined {
pub traits: String,
}

// FIXME(fmease): Deduplicate:

#[derive(Diagnostic)]
#[derive(Diagnostic, LintDiagnostic)]
#[diag(hir_analysis_ty_param_first_local, code = E0210)]
#[note]
pub(crate) struct TyParamFirstLocal<'tcx> {
Expand All @@ -1394,19 +1395,7 @@ pub(crate) struct TyParamFirstLocal<'tcx> {
pub local_type: Ty<'tcx>,
}

#[derive(LintDiagnostic)]
#[diag(hir_analysis_ty_param_first_local, code = E0210)]
#[note]
pub(crate) struct TyParamFirstLocalLint<'tcx> {
#[label]
pub span: Span,
#[note(hir_analysis_case_note)]
pub note: (),
pub param: Symbol,
pub local_type: Ty<'tcx>,
}

#[derive(Diagnostic)]
#[derive(Diagnostic, LintDiagnostic)]
#[diag(hir_analysis_ty_param_some, code = E0210)]
#[note]
pub(crate) struct TyParamSome {
Expand All @@ -1418,17 +1407,6 @@ pub(crate) struct TyParamSome {
pub param: Symbol,
}

#[derive(LintDiagnostic)]
#[diag(hir_analysis_ty_param_some, code = E0210)]
#[note]
pub(crate) struct TyParamSomeLint {
#[label]
pub span: Span,
#[note(hir_analysis_only_note)]
pub note: (),
pub param: Symbol,
}

#[derive(Diagnostic)]
pub(crate) enum OnlyCurrentTraits {
#[diag(hir_analysis_only_current_traits_outside, code = E0117)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a
// corresponding `Projection` clause
for def_ids in associated_types.values_mut() {
for (projection_bound, span) in &projection_bounds {
for &(projection_bound, span) in &projection_bounds {
let def_id = projection_bound.projection_def_id();
// FIXME(#120456) - is `swap_remove` correct?
def_ids.swap_remove(&def_id);
if tcx.generics_require_sized_self(def_id) {
tcx.emit_node_span_lint(
tcx.emit_node_lint(
UNUSED_ASSOCIATED_TYPE_BOUNDS,
hir_id,
*span,
crate::errors::UnusedAssociatedTypeBounds { span: *span },
crate::errors::UnusedAssociatedTypeBounds { span },
);
}
}
Expand Down
24 changes: 10 additions & 14 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,8 @@ impl<'a, 'tcx> CastCheck<'tcx> {
};
let expr_ty = fcx.resolve_vars_if_possible(self.expr_ty);
let cast_ty = fcx.resolve_vars_if_possible(self.cast_ty);
fcx.tcx.emit_node_span_lint(lint, self.expr.hir_id, self.span, errors::TrivialCast {
fcx.tcx.emit_node_lint(lint, self.expr.hir_id, errors::TrivialCast {
span: self.span,
numeric,
expr_ty,
cast_ty,
Expand Down Expand Up @@ -934,11 +935,11 @@ impl<'a, 'tcx> CastCheck<'tcx> {
.collect::<Vec<_>>();

if !added.is_empty() {
tcx.emit_node_span_lint(
tcx.emit_node_lint(
lint::builtin::PTR_CAST_ADD_AUTO_TO_OBJECT,
self.expr.hir_id,
self.span,
errors::PtrCastAddAutoToObject {
span: self.span,
traits_len: added.len(),
traits: {
let mut traits: Vec<_> = added
Expand Down Expand Up @@ -1097,11 +1098,10 @@ impl<'a, 'tcx> CastCheck<'tcx> {
let expr_ty = fcx.resolve_vars_if_possible(self.expr_ty);
let cast_ty = fcx.resolve_vars_if_possible(self.cast_ty);

fcx.tcx.emit_node_span_lint(
fcx.tcx.emit_node_lint(
lint::builtin::CENUM_IMPL_DROP_CAST,
self.expr.hir_id,
self.span,
errors::CastEnumDrop { expr_ty, cast_ty },
errors::CastEnumDrop { span: self.span, expr_ty, cast_ty },
);
}
}
Expand Down Expand Up @@ -1130,12 +1130,10 @@ impl<'a, 'tcx> CastCheck<'tcx> {
(false, false) => errors::LossyProvenancePtr2IntSuggestion::Other { cast_span },
};

let lint = errors::LossyProvenancePtr2Int { expr_ty, cast_ty, sugg };
fcx.tcx.emit_node_span_lint(
fcx.tcx.emit_node_lint(
lint::builtin::LOSSY_PROVENANCE_CASTS,
self.expr.hir_id,
self.span,
lint,
errors::LossyProvenancePtr2Int { span: self.span, expr_ty, cast_ty, sugg },
);
}

Expand All @@ -1146,12 +1144,10 @@ impl<'a, 'tcx> CastCheck<'tcx> {
};
let expr_ty = fcx.resolve_vars_if_possible(self.expr_ty);
let cast_ty = fcx.resolve_vars_if_possible(self.cast_ty);
let lint = errors::LossyProvenanceInt2Ptr { expr_ty, cast_ty, sugg };
fcx.tcx.emit_node_span_lint(
fcx.tcx.emit_node_lint(
lint::builtin::FUZZY_PROVENANCE_CASTS,
self.expr.hir_id,
self.span,
lint,
errors::LossyProvenanceInt2Ptr { span: self.span, expr_ty, cast_ty, sugg },
);
}

Expand Down
Loading

0 comments on commit 9de008a

Please sign in to comment.