Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move lint level source explanation to the bottom #101986

Merged
merged 10 commits into from
Oct 1, 2022
  •  
  •  
  •  
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,10 @@ impl<'tcx> ConstEvalErr<'tcx> {
rustc_session::lint::builtin::CONST_ERR,
hir_id,
tcx.span,
message,
|lint| {
let mut lint = lint.build(message);
finish(&mut lint, Some(err_msg));
lint.emit();
finish(lint, Some(err_msg));
lint
},
);
ErrorHandled::Linted
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,17 @@ impl<S: Into<String>> From<S> for DiagnosticMessage {
}
}

/// A workaround for "good path" ICEs when formatting types in disables lints.
///
/// Delays formatting until `.into(): DiagnosticMessage` is used.
pub struct DelayDm<F>(pub F);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this go away after the migration to translatable messages?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the intention.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a rustc lint to detect when we're using Ty: Display without wrapping it in this? (Maybe not to address in this PR, but I know I'm bound to make that mistake myself at some point in the future.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a lint, yes.


impl<F: FnOnce() -> String> From<DelayDm<F>> for DiagnosticMessage {
fn from(DelayDm(f): DelayDm<F>) -> Self {
DiagnosticMessage::from(f())
}
}

/// Translating *into* a subdiagnostic message from a diagnostic message is a little strange - but
/// the subdiagnostic functions (e.g. `span_label`) take a `SubdiagnosticMessage` and the
/// subdiagnostic derive refers to typed identifiers that are `DiagnosticMessage`s, so need to be
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::snippet::Style;
use crate::{
CodeSuggestion, DiagnosticMessage, EmissionGuarantee, Level, LintDiagnosticBuilder, MultiSpan,
CodeSuggestion, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, Level, MultiSpan,
SubdiagnosticMessage, Substitution, SubstitutionPart, SuggestionStyle,
};
use rustc_ast as ast;
Expand Down Expand Up @@ -209,7 +209,12 @@ pub trait AddToDiagnostic {
#[rustc_diagnostic_item = "DecorateLint"]
pub trait DecorateLint<'a, G: EmissionGuarantee> {
/// Decorate and emit a lint.
fn decorate_lint(self, diag: LintDiagnosticBuilder<'a, G>);
fn decorate_lint<'b>(
self,
diag: &'b mut DiagnosticBuilder<'a, G>,
) -> &'b mut DiagnosticBuilder<'a, G>;

fn msg(&self) -> DiagnosticMessage;
}

#[must_use]
Expand Down
24 changes: 0 additions & 24 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,27 +629,3 @@ macro_rules! struct_span_err {
macro_rules! error_code {
($code:ident) => {{ $crate::DiagnosticId::Error(stringify!($code).to_owned()) }};
}

/// Wrapper around a `DiagnosticBuilder` for creating lints.
pub struct LintDiagnosticBuilder<'a, G: EmissionGuarantee>(DiagnosticBuilder<'a, G>);

impl<'a, G: EmissionGuarantee> LintDiagnosticBuilder<'a, G> {
#[rustc_lint_diagnostics]
/// Return the inner `DiagnosticBuilder`, first setting the primary message to `msg`.
pub fn build(mut self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'a, G> {
self.0.set_primary_message(msg);
self.0.set_is_lint();
self.0
}

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

impl<'a> LintDiagnosticBuilder<'a, ErrorGuaranteed> {
pub fn forget_guarantee(self) -> LintDiagnosticBuilder<'a, ()> {
LintDiagnosticBuilder(self.0.forget_guarantee())
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use rustc_data_structures::stable_hasher::StableHasher;
use rustc_data_structures::sync::{self, Lock, Lrc};
use rustc_data_structures::AtomicRef;
pub use rustc_error_messages::{
fallback_fluent_bundle, fluent, fluent_bundle, DiagnosticMessage, FluentBundle,
fallback_fluent_bundle, fluent, fluent_bundle, DelayDm, DiagnosticMessage, FluentBundle,
LanguageIdentifier, LazyFallbackBundle, MultiSpan, SpanLabel, SubdiagnosticMessage,
DEFAULT_LOCALE_RESOURCES,
};
Expand Down Expand Up @@ -374,7 +374,7 @@ pub use diagnostic::{
AddToDiagnostic, DecorateLint, Diagnostic, DiagnosticArg, DiagnosticArgFromDisplay,
DiagnosticArgValue, DiagnosticId, DiagnosticStyledString, IntoDiagnosticArg, SubDiagnostic,
};
pub use diagnostic_builder::{DiagnosticBuilder, EmissionGuarantee, LintDiagnosticBuilder};
pub use diagnostic_builder::{DiagnosticBuilder, EmissionGuarantee};
use std::backtrace::Backtrace;

/// A handler deals with errors and other compiler output.
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_hir_analysis/src/astconv/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,9 +649,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
LATE_BOUND_LIFETIME_ARGUMENTS,
args.args[0].hir_id(),
multispan,
|lint| {
lint.build(msg).emit();
},
msg,
|lint| lint,
);
}

Expand Down
55 changes: 30 additions & 25 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2015,30 +2015,35 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
tcx.check_stability(item.def_id, Some(hir_ref_id), span, None);

if let Some(variant_def_id) = variant_resolution {
tcx.struct_span_lint_hir(AMBIGUOUS_ASSOCIATED_ITEMS, hir_ref_id, span, |lint| {
let mut err = lint.build("ambiguous associated item");
let mut could_refer_to = |kind: DefKind, def_id, also| {
let note_msg = format!(
"`{}` could{} refer to the {} defined here",
assoc_ident,
also,
kind.descr(def_id)
);
err.span_note(tcx.def_span(def_id), &note_msg);
};
tcx.struct_span_lint_hir(
AMBIGUOUS_ASSOCIATED_ITEMS,
hir_ref_id,
span,
"ambiguous associated item",
|lint| {
let mut could_refer_to = |kind: DefKind, def_id, also| {
let note_msg = format!(
"`{}` could{} refer to the {} defined here",
assoc_ident,
also,
kind.descr(def_id)
);
lint.span_note(tcx.def_span(def_id), &note_msg);
};

could_refer_to(DefKind::Variant, variant_def_id, "");
could_refer_to(kind, item.def_id, " also");
could_refer_to(DefKind::Variant, variant_def_id, "");
could_refer_to(kind, item.def_id, " also");

err.span_suggestion(
span,
"use fully-qualified syntax",
format!("<{} as {}>::{}", qself_ty, tcx.item_name(trait_did), assoc_ident),
Applicability::MachineApplicable,
);
lint.span_suggestion(
span,
"use fully-qualified syntax",
format!("<{} as {}>::{}", qself_ty, tcx.item_name(trait_did), assoc_ident),
Applicability::MachineApplicable,
);

err.emit();
});
lint
},
);
}
Ok((ty, kind, item.def_id))
}
Expand Down Expand Up @@ -3084,15 +3089,15 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
BARE_TRAIT_OBJECTS,
self_ty.hir_id,
self_ty.span,
msg,
|lint| {
let mut diag = lint.build(msg);
diag.multipart_suggestion_verbose(
lint.multipart_suggestion_verbose(
"use `dyn`",
sugg,
Applicability::MachineApplicable,
);
self.maybe_lint_blanket_trait_impl(&self_ty, &mut diag);
diag.emit();
self.maybe_lint_blanket_trait_impl(&self_ty, lint);
lint
},
);
}
Expand Down
77 changes: 41 additions & 36 deletions compiler/rustc_hir_analysis/src/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use super::FnCtxt;
use crate::hir::def_id::DefId;
use crate::type_error_struct;
use hir::def_id::LOCAL_CRATE;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed};
use rustc_errors::{struct_span_err, Applicability, DelayDm, DiagnosticBuilder, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_infer::traits::{Obligation, ObligationCause, ObligationCauseCode};
use rustc_middle::mir::Mutability;
Expand Down Expand Up @@ -754,19 +754,25 @@ impl<'a, 'tcx> CastCheck<'tcx> {
} else {
("", lint::builtin::TRIVIAL_CASTS)
};
fcx.tcx.struct_span_lint_hir(lint, self.expr.hir_id, self.span, |err| {
err.build(&format!(
"trivial {}cast: `{}` as `{}`",
adjective,
fcx.ty_to_string(t_expr),
fcx.ty_to_string(t_cast)
))
.help(&format!(
"cast can be replaced by coercion; this might \
require {type_asc_or}a temporary variable"
))
.emit();
});
fcx.tcx.struct_span_lint_hir(
lint,
self.expr.hir_id,
self.span,
DelayDm(|| {
format!(
"trivial {}cast: `{}` as `{}`",
adjective,
fcx.ty_to_string(t_expr),
fcx.ty_to_string(t_cast)
)
}),
|lint| {
lint.help(format!(
"cast can be replaced by coercion; this might \
require {type_asc_or}a temporary variable"
))
},
);
}

#[instrument(skip(fcx), level = "debug")]
Expand Down Expand Up @@ -1074,12 +1080,12 @@ impl<'a, 'tcx> CastCheck<'tcx> {
lint::builtin::CENUM_IMPL_DROP_CAST,
self.expr.hir_id,
self.span,
|err| {
err.build(&format!(
"cannot cast enum `{}` into integer `{}` because it implements `Drop`",
self.expr_ty, self.cast_ty
))
.emit();
DelayDm(|| format!(
"cannot cast enum `{}` into integer `{}` because it implements `Drop`",
self.expr_ty, self.cast_ty
)),
|lint| {
lint
},
);
}
Expand All @@ -1090,12 +1096,11 @@ impl<'a, 'tcx> CastCheck<'tcx> {
lint::builtin::LOSSY_PROVENANCE_CASTS,
self.expr.hir_id,
self.span,
|err| {
let mut err = err.build(&format!(
DelayDm(|| format!(
"under strict provenance it is considered bad style to cast pointer `{}` to integer `{}`",
self.expr_ty, self.cast_ty
));

)),
|lint| {
let msg = "use `.addr()` to obtain the address of a pointer";

let expr_prec = self.expr.precedence().order();
Expand All @@ -1114,22 +1119,22 @@ impl<'a, 'tcx> CastCheck<'tcx> {
(cast_span, format!(").addr(){scalar_cast}")),
];

err.multipart_suggestion(msg, suggestions, Applicability::MaybeIncorrect);
lint.multipart_suggestion(msg, suggestions, Applicability::MaybeIncorrect);
} else {
err.span_suggestion(
lint.span_suggestion(
cast_span,
msg,
format!(".addr(){scalar_cast}"),
Applicability::MaybeIncorrect,
);
}

err.help(
lint.help(
"if you can't comply with strict provenance and need to expose the pointer \
provenance you can use `.expose_addr()` instead"
);

err.emit();
lint
},
);
}
Expand All @@ -1139,24 +1144,24 @@ impl<'a, 'tcx> CastCheck<'tcx> {
lint::builtin::FUZZY_PROVENANCE_CASTS,
self.expr.hir_id,
self.span,
|err| {
let mut err = err.build(&format!(
"strict provenance disallows casting integer `{}` to pointer `{}`",
self.expr_ty, self.cast_ty
));
DelayDm(|| format!(
"strict provenance disallows casting integer `{}` to pointer `{}`",
self.expr_ty, self.cast_ty
)),
|lint| {
let msg = "use `.with_addr()` to adjust a valid pointer in the same allocation, to this address";
let suggestions = vec![
(self.expr_span.shrink_to_lo(), String::from("(...).with_addr(")),
(self.expr_span.shrink_to_hi().to(self.cast_span), String::from(")")),
];

err.multipart_suggestion(msg, suggestions, Applicability::MaybeIncorrect);
err.help(
lint.multipart_suggestion(msg, suggestions, Applicability::MaybeIncorrect);
lint.help(
"if you can't comply with strict provenance and don't have a pointer with \
the correct provenance you can use `std::ptr::from_exposed_addr()` instead"
);

err.emit();
lint
},
);
}
Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,13 @@ pub(super) fn check_abi(tcx: TyCtxt<'_>, hir_id: hir::HirId, span: Span, abi: Ab
.emit();
}
None => {
tcx.struct_span_lint_hir(UNSUPPORTED_CALLING_CONVENTIONS, hir_id, span, |lint| {
lint.build("use of calling convention not supported on this target").emit();
});
tcx.struct_span_lint_hir(
UNSUPPORTED_CALLING_CONVENTIONS,
hir_id,
span,
"use of calling convention not supported on this target",
|lint| lint,
);
}
}

Expand Down Expand Up @@ -510,10 +514,10 @@ fn check_static_inhabited<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) {
UNINHABITED_STATIC,
tcx.hir().local_def_id_to_hir_id(def_id),
span,
"static of uninhabited type",
|lint| {
lint.build("static of uninhabited type")
lint
.note("uninhabited statics cannot be initialized, and any access would be an immediate error")
.emit();
},
);
}
Expand Down Expand Up @@ -1434,17 +1438,17 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
tcx.hir().local_def_id_to_hir_id(adt.did().expect_local()),
span,
"zero-sized fields in `repr(transparent)` cannot contain external non-exhaustive types",
|lint| {
let note = if non_exhaustive {
"is marked with `#[non_exhaustive]`"
} else {
"contains private fields"
};
let field_ty = tcx.def_path_str_with_substs(def_id, substs);
lint.build("zero-sized fields in repr(transparent) cannot contain external non-exhaustive types")
lint
.note(format!("this {descr} contains `{field_ty}`, which {note}, \
and makes it not a breaking change to become non-zero-sized in the future."))
.emit();
},
)
}
Expand Down
Loading