mirror of
https://github.com/rust-lang/rust.git
synced 2024-10-30 05:51:58 +00:00
Remove DiagnosticBuilderState.
Currently it's used for two dynamic checks: - When a diagnostic is emitted, has it been emitted before? - When a diagnostic is dropped, has it been emitted/cancelled? The first check is no longer need, because `emit` is consuming, so it's impossible to emit a `DiagnosticBuilder` twice. The second check is still needed. This commit replaces `DiagnosticBuilderState` with a simpler `Option<Box<Diagnostic>>`, which is enough for the second check: functions like `emit` and `cancel` can take the `Diagnostic` and then `drop` can check that the `Diagnostic` was taken. The `DiagCtxt` reference from `DiagnosticBuilderState` is now stored as its own field, removing the need for the `dcx` method. As well as making the code shorter and simpler, the commit removes: - One (deprecated) `ErrorGuaranteed::unchecked_claim_error_was_emitted` call. - Two `FIXME(eddyb)` comments that are no longer relevant. - The use of a dummy `Diagnostic` in `into_diagnostic`. Nice!
This commit is contained in:
Nicholas Nethercote
parent
0cb486bc5b
commit
2d91c6d1bf
@ -35,19 +35,28 @@ where
|
||||
}
|
||||
|
||||
/// Used for emitting structured error messages and other diagnostic information.
|
||||
/// Each constructed `DiagnosticBuilder` must be consumed by a function such as
|
||||
/// `emit`, `cancel`, `delay_as_bug`, or `into_diagnostic`. A panic occurrs if a
|
||||
/// `DiagnosticBuilder` is dropped without being consumed by one of these
|
||||
/// functions.
|
||||
///
|
||||
/// If there is some state in a downstream crate you would like to
|
||||
/// access in the methods of `DiagnosticBuilder` here, consider
|
||||
/// extending `DiagCtxtFlags`.
|
||||
#[must_use]
|
||||
pub struct DiagnosticBuilder<'a, G: EmissionGuarantee = ErrorGuaranteed> {
|
||||
state: DiagnosticBuilderState<'a>,
|
||||
pub dcx: &'a DiagCtxt,
|
||||
|
||||
/// `Diagnostic` is a large type, and `DiagnosticBuilder` is often used as a
|
||||
/// return value, especially within the frequently-used `PResult` type.
|
||||
/// In theory, return value optimization (RVO) should avoid unnecessary
|
||||
/// copying. In practice, it does not (at the time of writing).
|
||||
diagnostic: Box<Diagnostic>,
|
||||
/// Why the `Option`? It is always `Some` until the `DiagnosticBuilder` is
|
||||
/// consumed via `emit`, `cancel`, etc. At that point it is consumed and
|
||||
/// replaced with `None`. Then `drop` checks that it is `None`; if not, it
|
||||
/// panics because a diagnostic was built but not used.
|
||||
///
|
||||
/// Why the Box? `Diagnostic` is a large type, and `DiagnosticBuilder` is
|
||||
/// often used as a return value, especially within the frequently-used
|
||||
/// `PResult` type. In theory, return value optimization (RVO) should avoid
|
||||
/// unnecessary copying. In practice, it does not (at the time of writing).
|
||||
diag: Option<Box<Diagnostic>>,
|
||||
|
||||
_marker: PhantomData<G>,
|
||||
}
|
||||
@ -56,32 +65,9 @@ pub struct DiagnosticBuilder<'a, G: EmissionGuarantee = ErrorGuaranteed> {
|
||||
// twice, which would be bad.
|
||||
impl<G> !Clone for DiagnosticBuilder<'_, G> {}
|
||||
|
||||
#[derive(Clone)]
|
||||
enum DiagnosticBuilderState<'a> {
|
||||
/// Initial state of a `DiagnosticBuilder`, before `.emit()` or `.cancel()`.
|
||||
///
|
||||
/// The `Diagnostic` will be emitted through this `DiagCtxt`.
|
||||
Emittable(&'a DiagCtxt),
|
||||
|
||||
/// State of a `DiagnosticBuilder`, after `.emit()` or *during* `.cancel()`.
|
||||
///
|
||||
/// The `Diagnostic` will be ignored when calling `.emit()`, and it can be
|
||||
/// assumed that `.emit()` was previously called, to end up in this state.
|
||||
///
|
||||
/// While this is also used by `.cancel()`, this state is only observed by
|
||||
/// the `Drop` `impl` of `DiagnosticBuilder`, because `.cancel()` takes
|
||||
/// `self` by-value specifically to prevent any attempts to `.emit()`.
|
||||
///
|
||||
// FIXME(eddyb) currently this doesn't prevent extending the `Diagnostic`,
|
||||
// despite that being potentially lossy, if important information is added
|
||||
// *after* the original `.emit()` call.
|
||||
AlreadyEmittedOrDuringCancellation,
|
||||
}
|
||||
|
||||
// `DiagnosticBuilderState` should be pointer-sized.
|
||||
rustc_data_structures::static_assert_size!(
|
||||
DiagnosticBuilderState<'_>,
|
||||
std::mem::size_of::<&DiagCtxt>()
|
||||
DiagnosticBuilder<'_, ()>,
|
||||
2 * std::mem::size_of::<usize>()
|
||||
);
|
||||
|
||||
/// Trait for types that `DiagnosticBuilder::emit` can return as a "guarantee"
|
||||
@ -99,62 +85,44 @@ pub trait EmissionGuarantee: Sized {
|
||||
}
|
||||
|
||||
impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
|
||||
/// Takes the diagnostic. For use by methods that consume the
|
||||
/// DiagnosticBuilder: `emit`, `cancel`, etc. Afterwards, `drop` is the
|
||||
/// only code that will be run on `self`.
|
||||
fn take_diag(&mut self) -> Diagnostic {
|
||||
Box::into_inner(self.diag.take().unwrap())
|
||||
}
|
||||
|
||||
/// Most `emit_producing_guarantee` functions use this as a starting point.
|
||||
fn emit_producing_nothing(mut self) {
|
||||
match self.state {
|
||||
// First `.emit()` call, the `&DiagCtxt` is still available.
|
||||
DiagnosticBuilderState::Emittable(dcx) => {
|
||||
self.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
|
||||
dcx.emit_diagnostic_without_consuming(&mut self.diagnostic);
|
||||
}
|
||||
// `.emit()` was previously called, disallowed from repeating it.
|
||||
DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
|
||||
}
|
||||
let diag = self.take_diag();
|
||||
self.dcx.emit_diagnostic(diag);
|
||||
}
|
||||
|
||||
/// `ErrorGuaranteed::emit_producing_guarantee` uses this.
|
||||
// FIXME(eddyb) make `ErrorGuaranteed` impossible to create outside `.emit()`.
|
||||
fn emit_producing_error_guaranteed(mut self) -> ErrorGuaranteed {
|
||||
let diag = self.take_diag();
|
||||
|
||||
// Only allow a guarantee if the `level` wasn't switched to a
|
||||
// non-error. The field isn't `pub`, but the whole `Diagnostic` can be
|
||||
// overwritten with a new one, thanks to `DerefMut`.
|
||||
assert!(
|
||||
diag.is_error(),
|
||||
"emitted non-error ({:?}) diagnostic from `DiagnosticBuilder<ErrorGuaranteed>`",
|
||||
diag.level,
|
||||
);
|
||||
|
||||
let guar = self.dcx.emit_diagnostic(diag);
|
||||
guar.unwrap()
|
||||
}
|
||||
}
|
||||
|
||||
// FIXME(eddyb) make `ErrorGuaranteed` impossible to create outside `.emit()`.
|
||||
impl EmissionGuarantee for ErrorGuaranteed {
|
||||
fn emit_producing_guarantee(mut db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
|
||||
// Contrast this with `emit_producing_nothing`.
|
||||
match db.state {
|
||||
// First `.emit()` call, the `&DiagCtxt` is still available.
|
||||
DiagnosticBuilderState::Emittable(dcx) => {
|
||||
db.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
|
||||
let guar = dcx.emit_diagnostic_without_consuming(&mut db.diagnostic);
|
||||
|
||||
// Only allow a guarantee if the `level` wasn't switched to a
|
||||
// non-error - the field isn't `pub`, but the whole `Diagnostic`
|
||||
// can be overwritten with a new one, thanks to `DerefMut`.
|
||||
assert!(
|
||||
db.diagnostic.is_error(),
|
||||
"emitted non-error ({:?}) diagnostic \
|
||||
from `DiagnosticBuilder<ErrorGuaranteed>`",
|
||||
db.diagnostic.level,
|
||||
);
|
||||
guar.unwrap()
|
||||
}
|
||||
// `.emit()` was previously called, disallowed from repeating it,
|
||||
// but can take advantage of the previous `.emit()`'s guarantee
|
||||
// still being applicable (i.e. as a form of idempotency).
|
||||
DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {
|
||||
// Only allow a guarantee if the `level` wasn't switched to a
|
||||
// non-error - the field isn't `pub`, but the whole `Diagnostic`
|
||||
// can be overwritten with a new one, thanks to `DerefMut`.
|
||||
assert!(
|
||||
db.diagnostic.is_error(),
|
||||
"`DiagnosticBuilder<ErrorGuaranteed>`'s diagnostic \
|
||||
became non-error ({:?}), after original `.emit()`",
|
||||
db.diagnostic.level,
|
||||
);
|
||||
#[allow(deprecated)]
|
||||
ErrorGuaranteed::unchecked_claim_error_was_emitted()
|
||||
}
|
||||
}
|
||||
fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
|
||||
db.emit_producing_error_guaranteed()
|
||||
}
|
||||
}
|
||||
|
||||
// FIXME(eddyb) should there be a `Option<ErrorGuaranteed>` impl as well?
|
||||
impl EmissionGuarantee for () {
|
||||
fn emit_producing_guarantee(db: DiagnosticBuilder<'_, Self>) -> Self::EmitResult {
|
||||
db.emit_producing_nothing();
|
||||
@ -219,12 +187,12 @@ macro_rules! forward {
|
||||
) => {
|
||||
#[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")]
|
||||
pub fn $n(&mut self, $($name: $ty),*) -> &mut Self {
|
||||
self.diagnostic.$n($($name),*);
|
||||
self.diag.as_mut().unwrap().$n($($name),*);
|
||||
self
|
||||
}
|
||||
#[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")]
|
||||
pub fn $n_mv(mut self, $($name: $ty),*) -> Self {
|
||||
self.diagnostic.$n($($name),*);
|
||||
self.diag.as_mut().unwrap().$n($($name),*);
|
||||
self
|
||||
}
|
||||
};
|
||||
@ -234,13 +202,13 @@ impl<G: EmissionGuarantee> Deref for DiagnosticBuilder<'_, G> {
|
||||
type Target = Diagnostic;
|
||||
|
||||
fn deref(&self) -> &Diagnostic {
|
||||
&self.diagnostic
|
||||
self.diag.as_ref().unwrap()
|
||||
}
|
||||
}
|
||||
|
||||
impl<G: EmissionGuarantee> DerefMut for DiagnosticBuilder<'_, G> {
|
||||
fn deref_mut(&mut self) -> &mut Diagnostic {
|
||||
&mut self.diagnostic
|
||||
self.diag.as_mut().unwrap()
|
||||
}
|
||||
}
|
||||
|
||||
@ -254,13 +222,9 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
|
||||
/// Creates a new `DiagnosticBuilder` with an already constructed
|
||||
/// diagnostic.
|
||||
#[track_caller]
|
||||
pub(crate) fn new_diagnostic(dcx: &'a DiagCtxt, diagnostic: Diagnostic) -> Self {
|
||||
pub(crate) fn new_diagnostic(dcx: &'a DiagCtxt, diag: Diagnostic) -> Self {
|
||||
debug!("Created new diagnostic");
|
||||
Self {
|
||||
state: DiagnosticBuilderState::Emittable(dcx),
|
||||
diagnostic: Box::new(diagnostic),
|
||||
_marker: PhantomData,
|
||||
}
|
||||
Self { dcx, diag: Some(Box::new(diag)), _marker: PhantomData }
|
||||
}
|
||||
|
||||
/// Emit and consume the diagnostic.
|
||||
@ -281,14 +245,10 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
|
||||
self.emit()
|
||||
}
|
||||
|
||||
/// Cancel the diagnostic (a structured diagnostic must either be emitted or
|
||||
/// Cancel and consume the diagnostic. (A diagnostic must either be emitted or
|
||||
/// cancelled or it will panic when dropped).
|
||||
///
|
||||
/// This method takes `self` by-value to disallow calling `.emit()` on it,
|
||||
/// which may be expected to *guarantee* the emission of an error, either
|
||||
/// at the time of the call, or through a prior `.emit()` call.
|
||||
pub fn cancel(mut self) {
|
||||
self.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;
|
||||
self.diag = None;
|
||||
drop(self);
|
||||
}
|
||||
|
||||
@ -304,44 +264,21 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
|
||||
}
|
||||
|
||||
/// Converts the builder to a `Diagnostic` for later emission,
|
||||
/// unless dcx has disabled such buffering, or `.emit()` was called.
|
||||
/// unless dcx has disabled such buffering.
|
||||
pub fn into_diagnostic(mut self) -> Option<(Diagnostic, &'a DiagCtxt)> {
|
||||
let dcx = match self.state {
|
||||
// No `.emit()` calls, the `&DiagCtxt` is still available.
|
||||
DiagnosticBuilderState::Emittable(dcx) => dcx,
|
||||
// `.emit()` was previously called, nothing we can do.
|
||||
DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {
|
||||
return None;
|
||||
}
|
||||
};
|
||||
|
||||
if dcx.inner.lock().flags.dont_buffer_diagnostics
|
||||
|| dcx.inner.lock().flags.treat_err_as_bug.is_some()
|
||||
{
|
||||
let flags = self.dcx.inner.lock().flags;
|
||||
if flags.dont_buffer_diagnostics || flags.treat_err_as_bug.is_some() {
|
||||
self.emit();
|
||||
return None;
|
||||
}
|
||||
|
||||
// Take the `Diagnostic` by replacing it with a dummy.
|
||||
let dummy = Diagnostic::new(Level::Allow, DiagnosticMessage::from(""));
|
||||
let diagnostic = std::mem::replace(&mut *self.diagnostic, dummy);
|
||||
|
||||
// Disable the ICE on `Drop`.
|
||||
self.cancel();
|
||||
let diag = self.take_diag();
|
||||
|
||||
// Logging here is useful to help track down where in logs an error was
|
||||
// actually emitted.
|
||||
debug!("buffer: diagnostic={:?}", diagnostic);
|
||||
debug!("buffer: diag={:?}", diag);
|
||||
|
||||
Some((diagnostic, dcx))
|
||||
}
|
||||
|
||||
/// Retrieves the [`DiagCtxt`] if available
|
||||
pub fn dcx(&self) -> Option<&DiagCtxt> {
|
||||
match self.state {
|
||||
DiagnosticBuilderState::Emittable(dcx) => Some(dcx),
|
||||
DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => None,
|
||||
}
|
||||
Some((diag, self.dcx))
|
||||
}
|
||||
|
||||
/// Buffers the diagnostic for later emission,
|
||||
@ -494,30 +431,24 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
|
||||
|
||||
impl<G: EmissionGuarantee> Debug for DiagnosticBuilder<'_, G> {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
self.diagnostic.fmt(f)
|
||||
self.diag.fmt(f)
|
||||
}
|
||||
}
|
||||
|
||||
/// Destructor bomb - a `DiagnosticBuilder` must be either emitted or cancelled
|
||||
/// or we emit a bug.
|
||||
/// Destructor bomb: every `DiagnosticBuilder` must be consumed (emitted,
|
||||
/// cancelled, etc.) or we emit a bug.
|
||||
impl<G: EmissionGuarantee> Drop for DiagnosticBuilder<'_, G> {
|
||||
fn drop(&mut self) {
|
||||
match self.state {
|
||||
// No `.emit()` or `.cancel()` calls.
|
||||
DiagnosticBuilderState::Emittable(dcx) => {
|
||||
if !panicking() {
|
||||
dcx.emit_diagnostic(Diagnostic::new(
|
||||
Level::Bug,
|
||||
DiagnosticMessage::from(
|
||||
"the following error was constructed but not emitted",
|
||||
),
|
||||
));
|
||||
dcx.emit_diagnostic_without_consuming(&mut self.diagnostic);
|
||||
panic!("error was constructed but not emitted");
|
||||
}
|
||||
match self.diag.take() {
|
||||
Some(diag) if !panicking() => {
|
||||
self.dcx.emit_diagnostic(Diagnostic::new(
|
||||
Level::Bug,
|
||||
DiagnosticMessage::from("the following error was constructed but not emitted"),
|
||||
));
|
||||
self.dcx.emit_diagnostic(*diag);
|
||||
panic!("error was constructed but not emitted");
|
||||
}
|
||||
// `.emit()` was previously called, or maybe we're during `.cancel()`.
|
||||
DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -7,6 +7,7 @@
|
||||
#![feature(rustdoc_internals)]
|
||||
#![feature(array_windows)]
|
||||
#![feature(associated_type_defaults)]
|
||||
#![feature(box_into_inner)]
|
||||
#![feature(extract_if)]
|
||||
#![feature(if_let_guard)]
|
||||
#![feature(let_chains)]
|
||||
|
||||
@ -181,8 +181,7 @@ pub(crate) struct UnsafeOpInUnsafeFn {
|
||||
impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn {
|
||||
#[track_caller]
|
||||
fn decorate_lint<'b>(self, diag: &'b mut DiagnosticBuilder<'a, ()>) {
|
||||
let dcx = diag.dcx().expect("lint should not yet be emitted");
|
||||
let desc = dcx.eagerly_translate_to_string(self.details.label(), [].into_iter());
|
||||
let desc = diag.dcx.eagerly_translate_to_string(self.details.label(), [].into_iter());
|
||||
diag.arg("details", desc);
|
||||
diag.span_label(self.details.span, self.details.label());
|
||||
self.details.add_subdiagnostics(diag);
|
||||
|
||||
Loading…
Reference in New Issue
Block a user