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
@@ -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
11 changes: 11 additions & 0 deletions compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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
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;
@@ -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]
24 changes: 0 additions & 24 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
@@ -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
@@ -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,
};
@@ -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.
5 changes: 2 additions & 3 deletions compiler/rustc_hir_analysis/src/astconv/generics.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}

55 changes: 30 additions & 25 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
@@ -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))
}
@@ -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
},
);
}
77 changes: 41 additions & 36 deletions compiler/rustc_hir_analysis/src/check/cast.rs
Original file line number Diff line number Diff line change
@@ -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;
@@ -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")]
@@ -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
},
);
}
@@ -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();
@@ -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
},
);
}
@@ -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
},
);
}
18 changes: 11 additions & 7 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}
}

@@ -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();
},
);
}
@@ -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();
},
)
}
Loading