From f0a3684c1e792b6c16e32dc154de34328beef619 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 10 Jan 2024 14:27:02 +1100 Subject: [PATCH 01/12] Inline and remove `DiagCtxtInner::bump_{lint_err,err}_count`. They have one and two call sites respectively, and they just make the code harder to read. --- compiler/rustc_errors/src/lib.rs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 8fb539fc3582f..b2b74c7195a99 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1168,7 +1168,8 @@ impl DiagCtxt { let mut inner = self.inner.borrow_mut(); if loud && lint_level.is_error() { - inner.bump_err_count(); + inner.err_count += 1; + inner.panic_if_treat_err_as_bug(); } inner.emitter.emit_unused_externs(lint_level, unused_externs) @@ -1353,10 +1354,11 @@ impl DiagCtxtInner { } if diagnostic.is_error() { if diagnostic.is_lint { - self.bump_lint_err_count(); + self.lint_err_count += 1; } else { - self.bump_err_count(); + self.err_count += 1; } + self.panic_if_treat_err_as_bug(); #[allow(deprecated)] { @@ -1447,16 +1449,6 @@ impl DiagCtxtInner { panic::panic_any(DelayedBugPanic); } - fn bump_lint_err_count(&mut self) { - self.lint_err_count += 1; - self.panic_if_treat_err_as_bug(); - } - - fn bump_err_count(&mut self) { - self.err_count += 1; - self.panic_if_treat_err_as_bug(); - } - fn panic_if_treat_err_as_bug(&self) { if self.treat_err_as_bug() { match ( From 2aac288c18f96c55677d90262e068820caf310f3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 10 Jan 2024 14:29:01 +1100 Subject: [PATCH 02/12] Use the right level with `-Ztreat-err-as-bug`. Errors in `DiagCtxtInner::emit_diagnostic` are never set to `Level::Bug`, because the condition never succeeds, because `self.treat_err_as_bug()` is called *before* the error counts are incremented. This commit switches to `self.treat_next_err_as_bug()`, fixing the problem. This changes the error message output to actually say "internal compiler error". --- compiler/rustc_errors/src/lib.rs | 2 +- tests/rustdoc-ui/ice-bug-report-url.stderr | 2 +- tests/ui/consts/const-eval/const-eval-query-stack.stderr | 2 +- tests/ui/impl-trait/issues/issue-86800.stderr | 2 +- tests/ui/panics/default-backtrace-ice.stderr | 2 +- tests/ui/treat-err-as-bug/err.stderr | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index b2b74c7195a99..e60300bc3ebf4 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1256,7 +1256,7 @@ impl DiagCtxtInner { } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { - if matches!(diagnostic.level, Error | Fatal) && self.treat_err_as_bug() { + if matches!(diagnostic.level, Error | Fatal) && self.treat_next_err_as_bug() { diagnostic.level = Bug; } diff --git a/tests/rustdoc-ui/ice-bug-report-url.stderr b/tests/rustdoc-ui/ice-bug-report-url.stderr index 869fcd20fac8e..06a5269131083 100644 --- a/tests/rustdoc-ui/ice-bug-report-url.stderr +++ b/tests/rustdoc-ui/ice-bug-report-url.stderr @@ -1,4 +1,4 @@ -error: expected one of `->`, `where`, or `{`, found `` +error: internal compiler error: expected one of `->`, `where`, or `{`, found `` --> $DIR/ice-bug-report-url.rs:14:10 | LL | fn wrong() diff --git a/tests/ui/consts/const-eval/const-eval-query-stack.stderr b/tests/ui/consts/const-eval/const-eval-query-stack.stderr index 01fb8153cf384..c748af608d1e6 100644 --- a/tests/ui/consts/const-eval/const-eval-query-stack.stderr +++ b/tests/ui/consts/const-eval/const-eval-query-stack.stderr @@ -1,4 +1,4 @@ -error[E0080]: evaluation of constant value failed +error: internal compiler error[E0080]: evaluation of constant value failed --> $DIR/const-eval-query-stack.rs:16:16 | LL | const X: i32 = 1 / 0; diff --git a/tests/ui/impl-trait/issues/issue-86800.stderr b/tests/ui/impl-trait/issues/issue-86800.stderr index 07ba8eb021b31..7af4846a9593f 100644 --- a/tests/ui/impl-trait/issues/issue-86800.stderr +++ b/tests/ui/impl-trait/issues/issue-86800.stderr @@ -4,7 +4,7 @@ error: unconstrained opaque type LL | type TransactionFuture<'__, O> = impl '__ + Future>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -error[E0792]: expected generic lifetime parameter, found `'_` +error: internal compiler error[E0792]: expected generic lifetime parameter, found `'_` --> $DIR/issue-86800.rs:39:5 | LL | type TransactionFuture<'__, O> = impl '__ + Future>; diff --git a/tests/ui/panics/default-backtrace-ice.stderr b/tests/ui/panics/default-backtrace-ice.stderr index 4b00f13504758..9d27cb22ae942 100644 --- a/tests/ui/panics/default-backtrace-ice.stderr +++ b/tests/ui/panics/default-backtrace-ice.stderr @@ -1,4 +1,4 @@ -error[E0425]: cannot find value `missing_ident` in this scope +error: internal compiler error[E0425]: cannot find value `missing_ident` in this scope --> $DIR/default-backtrace-ice.rs:21:13 | LL | fn main() { missing_ident; } diff --git a/tests/ui/treat-err-as-bug/err.stderr b/tests/ui/treat-err-as-bug/err.stderr index 3a56445a26b58..4c5d0e5ae7987 100644 --- a/tests/ui/treat-err-as-bug/err.stderr +++ b/tests/ui/treat-err-as-bug/err.stderr @@ -1,4 +1,4 @@ -error[E0080]: could not evaluate static initializer +error: internal compiler error[E0080]: could not evaluate static initializer --> $DIR/err.rs:11:21 | LL | pub static C: u32 = 0 - 1; From a0f5431e2385c08e40eb8c549249f4e94b32318c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 10 Jan 2024 14:45:38 +1100 Subject: [PATCH 03/12] Move code around. No point computing `warnings` and `errors` if we're going to return early before they're used. --- compiler/rustc_errors/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index e60300bc3ebf4..404c89ea01b64 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -981,6 +981,10 @@ impl DiagCtxt { inner.emit_stashed_diagnostics(); + if inner.treat_err_as_bug() { + return; + } + let warnings = match inner.deduplicated_warn_count { 0 => Cow::from(""), 1 => Cow::from("1 warning emitted"), @@ -991,9 +995,6 @@ impl DiagCtxt { 1 => Cow::from("aborting due to 1 previous error"), count => Cow::from(format!("aborting due to {count} previous errors")), }; - if inner.treat_err_as_bug() { - return; - } match (errors.len(), warnings.len()) { (0, 0) => return, From 552bed8048d09fc313450f57430da29be94945f4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 10 Jan 2024 15:00:20 +1100 Subject: [PATCH 04/12] Remove `DiagnosticBuilder::into_diagnostic` from `-Ztreat-err-as-bug` consideration. It seems very wrong to have a `-Ztreat-err-as-bug` check here before the error is even emitted. Once that's done: - `into_diagnostic` is infallible, so its return type doesn't need the `Option`; - the `&'a DiagCtxt` also isn't needed, because only one callsite uses it, and it already have access to it via `self.dcx`; - the comments about dcx disabling buffering are no longer true, this is unconditional now; - and the `debug!` seems unnecessary... the comment greatly overstates its importance because few diagnostics come through `into_diagnostic`, and `-Ztrack-diagnostics` exists anyway. --- .../rustc_errors/src/diagnostic_builder.rs | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index a02909f29c45d..eda2f384fad34 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -255,35 +255,18 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { /// Stashes diagnostic for possible later improvement in a different, /// later stage of the compiler. The diagnostic can be accessed with /// the provided `span` and `key` through [`DiagCtxt::steal_diagnostic()`]. - /// - /// As with `buffer`, this is unless the dcx has disabled such buffering. pub fn stash(self, span: Span, key: StashKey) { - if let Some((diag, dcx)) = self.into_diagnostic() { - dcx.stash_diagnostic(span, key, diag); - } + self.dcx.stash_diagnostic(span, key, self.into_diagnostic()); } - /// Converts the builder to a `Diagnostic` for later emission, - /// unless dcx has disabled such buffering. - fn into_diagnostic(mut self) -> Option<(Diagnostic, &'a DiagCtxt)> { - if self.dcx.inner.lock().flags.treat_err_as_bug.is_some() { - self.emit(); - return None; - } - - let diag = self.take_diag(); - - // Logging here is useful to help track down where in logs an error was - // actually emitted. - debug!("buffer: diag={:?}", diag); - - Some((diag, self.dcx)) + /// Converts the builder to a `Diagnostic` for later emission. + fn into_diagnostic(mut self) -> Diagnostic { + self.take_diag() } - /// Buffers the diagnostic for later emission, - /// unless dcx has disabled such buffering. + /// Buffers the diagnostic for later emission. pub fn buffer(self, buffered_diagnostics: &mut Vec) { - buffered_diagnostics.extend(self.into_diagnostic().map(|(diag, _)| diag)); + buffered_diagnostics.push(self.into_diagnostic()); } /// Delay emission of this diagnostic as a bug. From f5c0cd0bd1c564985788fb72d73ee3c8d8beb1d7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 13:50:00 +1100 Subject: [PATCH 05/12] Inline and remove three functions. Each of these has a single call site: `source_file_to_parser`, `try_file_to_source_file`, `file_to_source_file`. Having them separate just makes the code longer and harder to read. Also, `maybe_file_to_stream` doesn't need to be `pub`. --- compiler/rustc_parse/src/lib.rs | 53 +++++++++------------------------ 1 file changed, 14 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs index b93f08a21e311..106137299b400 100644 --- a/compiler/rustc_parse/src/lib.rs +++ b/compiler/rustc_parse/src/lib.rs @@ -113,19 +113,24 @@ pub fn maybe_new_parser_from_source_str( maybe_source_file_to_parser(sess, sess.source_map().new_source_file(name, source)) } -/// Creates a new parser, handling errors as appropriate if the file doesn't exist. -/// If a span is given, that is used on an error as the source of the problem. +/// Creates a new parser, aborting if the file doesn't exist. If a span is given, that is used on +/// an error as the source of the problem. pub fn new_parser_from_file<'a>(sess: &'a ParseSess, path: &Path, sp: Option) -> Parser<'a> { - source_file_to_parser(sess, file_to_source_file(sess, path, sp)) -} + let source_file = sess.source_map().load_file(path).unwrap_or_else(|e| { + let msg = format!("couldn't read {}: {}", path.display(), e); + let mut diag = Diagnostic::new(Level::Fatal, msg); + if let Some(sp) = sp { + diag.span(sp); + } + sess.dcx.emit_diagnostic(diag); + FatalError.raise(); + }); -/// Given a session and a `source_file`, returns a parser. -fn source_file_to_parser(sess: &ParseSess, source_file: Lrc) -> Parser<'_> { panictry_buffer!(&sess.dcx, maybe_source_file_to_parser(sess, source_file)) } -/// Given a session and a `source_file`, return a parser. Returns any buffered errors from lexing the -/// initial token stream. +/// Given a session and a `source_file`, return a parser. Returns any buffered errors from lexing +/// the initial token stream. fn maybe_source_file_to_parser( sess: &ParseSess, source_file: Lrc, @@ -142,36 +147,6 @@ fn maybe_source_file_to_parser( // Base abstractions -/// Given a session and a path and an optional span (for error reporting), -/// add the path to the session's source_map and return the new source_file or -/// error when a file can't be read. -fn try_file_to_source_file( - sess: &ParseSess, - path: &Path, - spanopt: Option, -) -> Result, Diagnostic> { - sess.source_map().load_file(path).map_err(|e| { - let msg = format!("couldn't read {}: {}", path.display(), e); - let mut diag = Diagnostic::new(Level::Fatal, msg); - if let Some(sp) = spanopt { - diag.span(sp); - } - diag - }) -} - -/// Given a session and a path and an optional span (for error reporting), -/// adds the path to the session's `source_map` and returns the new `source_file`. -fn file_to_source_file(sess: &ParseSess, path: &Path, spanopt: Option) -> Lrc { - match try_file_to_source_file(sess, path, spanopt) { - Ok(source_file) => source_file, - Err(d) => { - sess.dcx.emit_diagnostic(d); - FatalError.raise(); - } - } -} - /// Given a `source_file`, produces a sequence of token trees. pub fn source_file_to_stream( sess: &ParseSess, @@ -183,7 +158,7 @@ pub fn source_file_to_stream( /// Given a source file, produces a sequence of token trees. Returns any buffered errors from /// parsing the token stream. -pub fn maybe_file_to_stream( +fn maybe_file_to_stream( sess: &ParseSess, source_file: Lrc, override_span: Option, From d5aafb846b7d77aee03511059ea21cb1041d62f9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 16:24:31 +1100 Subject: [PATCH 06/12] Use `struct_fatal` in `new_parser_from_file`. It's a little more concise, and the standard way to do it. --- compiler/rustc_parse/src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs index 106137299b400..55437de72b5dc 100644 --- a/compiler/rustc_parse/src/lib.rs +++ b/compiler/rustc_parse/src/lib.rs @@ -19,7 +19,7 @@ use rustc_ast::tokenstream::TokenStream; use rustc_ast::{AttrItem, Attribute, MetaItem}; use rustc_ast_pretty::pprust; use rustc_data_structures::sync::Lrc; -use rustc_errors::{Diagnostic, FatalError, Level, PResult}; +use rustc_errors::{Diagnostic, PResult}; use rustc_session::parse::ParseSess; use rustc_span::{FileName, SourceFile, Span}; @@ -118,12 +118,11 @@ pub fn maybe_new_parser_from_source_str( pub fn new_parser_from_file<'a>(sess: &'a ParseSess, path: &Path, sp: Option) -> Parser<'a> { let source_file = sess.source_map().load_file(path).unwrap_or_else(|e| { let msg = format!("couldn't read {}: {}", path.display(), e); - let mut diag = Diagnostic::new(Level::Fatal, msg); + let mut err = sess.dcx.struct_fatal(msg); if let Some(sp) = sp { - diag.span(sp); + err.span(sp); } - sess.dcx.emit_diagnostic(diag); - FatalError.raise(); + err.emit(); }); panictry_buffer!(&sess.dcx, maybe_source_file_to_parser(sess, source_file)) From 2668270dfb52c1241a10294bc61fa9b6da0d7933 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 10:35:56 +1100 Subject: [PATCH 07/12] Stop using `DiagnosticBuilder::buffer` in `WritebackCx`. --- compiler/rustc_hir_typeck/src/writeback.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index c56a028321aef..3430a5fb00dc9 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -498,14 +498,14 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { // order when emitting them. let err = self.tcx().dcx().struct_span_err(span, format!("user args: {user_args:?}")); - err.buffer(&mut errors_buffer); + errors_buffer.push(err); } } if !errors_buffer.is_empty() { errors_buffer.sort_by_key(|diag| diag.span.primary_span()); - for diag in errors_buffer { - self.tcx().dcx().emit_diagnostic(diag); + for err in errors_buffer { + err.emit(); } } } From 29c601aa0b6a1661f22da160be3ff58b6d79fe67 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 10:45:56 +1100 Subject: [PATCH 08/12] Stop using `DiagnosticBuilder::buffer` in `Checker`. This requires cancelling the "secondary" errors when they're not emitted, to prevent panics due to unconsumed `DiagnosticBuilder`s. --- .../src/transform/check_consts/check.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 738c532964a2c..ae9595d7e6444 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -1,6 +1,6 @@ //! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations. -use rustc_errors::{Diagnostic, ErrorGuaranteed}; +use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_index::bit_set::BitSet; @@ -214,7 +214,7 @@ pub struct Checker<'mir, 'tcx> { local_has_storage_dead: Option>, error_emitted: Option, - secondary_errors: Vec, + secondary_errors: Vec>, } impl<'mir, 'tcx> Deref for Checker<'mir, 'tcx> { @@ -272,14 +272,17 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> { } // If we got through const-checking without emitting any "primary" errors, emit any - // "secondary" errors if they occurred. + // "secondary" errors if they occurred. Otherwise, cancel the "secondary" errors. let secondary_errors = mem::take(&mut self.secondary_errors); if self.error_emitted.is_none() { for error in secondary_errors { - self.tcx.dcx().emit_diagnostic(error); + error.emit(); } } else { assert!(self.tcx.dcx().has_errors().is_some()); + for error in secondary_errors { + error.cancel(); + } } } @@ -347,7 +350,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> { self.error_emitted = Some(reported); } - ops::DiagnosticImportance::Secondary => err.buffer(&mut self.secondary_errors), + ops::DiagnosticImportance::Secondary => self.secondary_errors.push(err), } } From fbe68bc40c6e49efa3a6ec4aa2640786853f2479 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 11:22:10 +1100 Subject: [PATCH 09/12] Stop using `DiagnosticBuilder::buffer` in `BorrowckErrors`. But we can't easily switch from `Vec` to `Vec>` because there's a mix of errors and warnings which result in different `G` types. So we must make `DiagnosticBuilder::into_diagnostic` public, but that's ok, and it will get more use in subsequent commits. --- compiler/rustc_borrowck/src/lib.rs | 19 ++++++++++--------- .../rustc_errors/src/diagnostic_builder.rs | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 495b255583c2a..0457b4e6ddc78 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2399,10 +2399,10 @@ mod error { /// and we want only the best of those errors. /// /// The `report_use_of_moved_or_uninitialized` function checks this map and replaces the - /// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of the - /// `Place` of the previous most diagnostic. This happens instead of buffering the error. Once - /// all move errors have been reported, any diagnostics in this map are added to the buffer - /// to be emitted. + /// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of + /// the `Place` of the previous most diagnostic. This happens instead of buffering the + /// error. Once all move errors have been reported, any diagnostics in this map are added + /// to the buffer to be emitted. /// /// `BTreeMap` is used to preserve the order of insertions when iterating. This is necessary /// when errors in the map are being re-added to the error buffer so that errors with the @@ -2410,7 +2410,8 @@ mod error { buffered_move_errors: BTreeMap, (PlaceRef<'tcx>, DiagnosticBuilder<'tcx>)>, buffered_mut_errors: FxIndexMap, usize)>, - /// Diagnostics to be reported buffer. + /// Buffer of diagnostics to be reported. Uses `Diagnostic` rather than `DiagnosticBuilder` + /// because it has a mixture of error diagnostics and non-error diagnostics. buffered: Vec, /// Set to Some if we emit an error during borrowck tainted_by_errors: Option, @@ -2434,11 +2435,11 @@ mod error { "diagnostic buffered but not emitted", )) } - t.buffer(&mut self.buffered); + self.buffered.push(t.into_diagnostic()); } pub fn buffer_non_error_diag(&mut self, t: DiagnosticBuilder<'_, ()>) { - t.buffer(&mut self.buffered); + self.buffered.push(t.into_diagnostic()); } pub fn set_tainted_by_errors(&mut self, e: ErrorGuaranteed) { @@ -2486,13 +2487,13 @@ mod error { // Buffer any move errors that we collected and de-duplicated. for (_, (_, diag)) in std::mem::take(&mut self.errors.buffered_move_errors) { // We have already set tainted for this error, so just buffer it. - diag.buffer(&mut self.errors.buffered); + self.errors.buffered.push(diag.into_diagnostic()); } for (_, (mut diag, count)) in std::mem::take(&mut self.errors.buffered_mut_errors) { if count > 10 { diag.note(format!("...and {} other attempted mutable borrows", count - 10)); } - diag.buffer(&mut self.errors.buffered); + self.errors.buffered.push(diag.into_diagnostic()); } if !self.errors.buffered.is_empty() { diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index eda2f384fad34..932b8793807f1 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -260,7 +260,7 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { } /// Converts the builder to a `Diagnostic` for later emission. - fn into_diagnostic(mut self) -> Diagnostic { + pub fn into_diagnostic(mut self) -> Diagnostic { self.take_diag() } From d02150fd45b4e63007a446f497ce032f48b1166e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 14:21:07 +1100 Subject: [PATCH 10/12] Fix lifetimes in `StringReader`. Two different lifetimes are conflated. This doesn't matter right now, but needs to be fixed for the next commit to work. And the more descriptive lifetime names make the code easier to read. --- compiler/rustc_parse/src/lexer/mod.rs | 24 +++++++++---------- compiler/rustc_parse/src/lexer/tokentrees.rs | 24 +++++++++++-------- .../rustc_parse/src/lexer/unicode_chars.rs | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/mod.rs b/compiler/rustc_parse/src/lexer/mod.rs index 7db9291921f0e..9626d4e817158 100644 --- a/compiler/rustc_parse/src/lexer/mod.rs +++ b/compiler/rustc_parse/src/lexer/mod.rs @@ -42,9 +42,9 @@ pub struct UnmatchedDelim { pub candidate_span: Option, } -pub(crate) fn parse_token_trees<'a>( - sess: &'a ParseSess, - mut src: &'a str, +pub(crate) fn parse_token_trees<'sess, 'src>( + sess: &'sess ParseSess, + mut src: &'src str, mut start_pos: BytePos, override_span: Option, ) -> Result> { @@ -90,16 +90,16 @@ pub(crate) fn parse_token_trees<'a>( } } -struct StringReader<'a> { - sess: &'a ParseSess, +struct StringReader<'sess, 'src> { + sess: &'sess ParseSess, /// Initial position, read-only. start_pos: BytePos, /// The absolute offset within the source_map of the current character. pos: BytePos, /// Source text to tokenize. - src: &'a str, + src: &'src str, /// Cursor for getting lexer tokens. - cursor: Cursor<'a>, + cursor: Cursor<'src>, override_span: Option, /// When a "unknown start of token: \u{a0}" has already been emitted earlier /// in this file, it's safe to treat further occurrences of the non-breaking @@ -107,8 +107,8 @@ struct StringReader<'a> { nbsp_is_whitespace: bool, } -impl<'a> StringReader<'a> { - pub fn dcx(&self) -> &'a DiagCtxt { +impl<'sess, 'src> StringReader<'sess, 'src> { + pub fn dcx(&self) -> &'sess DiagCtxt { &self.sess.dcx } @@ -526,7 +526,7 @@ impl<'a> StringReader<'a> { /// Slice of the source text from `start` up to but excluding `self.pos`, /// meaning the slice does not include the character `self.ch`. - fn str_from(&self, start: BytePos) -> &'a str { + fn str_from(&self, start: BytePos) -> &'src str { self.str_from_to(start, self.pos) } @@ -537,12 +537,12 @@ impl<'a> StringReader<'a> { } /// Slice of the source text spanning from `start` up to but excluding `end`. - fn str_from_to(&self, start: BytePos, end: BytePos) -> &'a str { + fn str_from_to(&self, start: BytePos, end: BytePos) -> &'src str { &self.src[self.src_index(start)..self.src_index(end)] } /// Slice of the source text spanning from `start` until the end - fn str_from_to_end(&self, start: BytePos) -> &'a str { + fn str_from_to_end(&self, start: BytePos) -> &'src str { &self.src[self.src_index(start)..] } diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index 64b3928689afe..c9ff2d58e2c56 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -8,18 +8,18 @@ use rustc_ast_pretty::pprust::token_to_string; use rustc_errors::{Applicability, PErr}; use rustc_span::symbol::kw; -pub(super) struct TokenTreesReader<'a> { - string_reader: StringReader<'a>, +pub(super) struct TokenTreesReader<'sess, 'src> { + string_reader: StringReader<'sess, 'src>, /// The "next" token, which has been obtained from the `StringReader` but /// not yet handled by the `TokenTreesReader`. token: Token, diag_info: TokenTreeDiagInfo, } -impl<'a> TokenTreesReader<'a> { +impl<'sess, 'src> TokenTreesReader<'sess, 'src> { pub(super) fn parse_all_token_trees( - string_reader: StringReader<'a>, - ) -> (TokenStream, Result<(), Vec>>, Vec) { + string_reader: StringReader<'sess, 'src>, + ) -> (TokenStream, Result<(), Vec>>, Vec) { let mut tt_reader = TokenTreesReader { string_reader, token: Token::dummy(), @@ -35,7 +35,7 @@ impl<'a> TokenTreesReader<'a> { fn parse_token_trees( &mut self, is_delimited: bool, - ) -> (Spacing, TokenStream, Result<(), Vec>>) { + ) -> (Spacing, TokenStream, Result<(), Vec>>) { // Move past the opening delimiter. let (_, open_spacing) = self.bump(false); @@ -71,7 +71,7 @@ impl<'a> TokenTreesReader<'a> { } } - fn eof_err(&mut self) -> PErr<'a> { + fn eof_err(&mut self) -> PErr<'sess> { let msg = "this file contains an unclosed delimiter"; let mut err = self.string_reader.sess.dcx.struct_span_err(self.token.span, msg); for &(_, sp) in &self.diag_info.open_braces { @@ -99,7 +99,7 @@ impl<'a> TokenTreesReader<'a> { fn parse_token_tree_open_delim( &mut self, open_delim: Delimiter, - ) -> Result>> { + ) -> Result>> { // The span for beginning of the delimited section let pre_span = self.token.span; @@ -229,7 +229,11 @@ impl<'a> TokenTreesReader<'a> { (this_tok, this_spacing) } - fn unclosed_delim_err(&mut self, tts: TokenStream, mut errs: Vec>) -> Vec> { + fn unclosed_delim_err( + &mut self, + tts: TokenStream, + mut errs: Vec>, + ) -> Vec> { // If there are unclosed delims, see if there are diff markers and if so, point them // out instead of complaining about the unclosed delims. let mut parser = crate::stream_to_parser(self.string_reader.sess, tts, None); @@ -285,7 +289,7 @@ impl<'a> TokenTreesReader<'a> { return errs; } - fn close_delim_err(&mut self, delim: Delimiter) -> PErr<'a> { + fn close_delim_err(&mut self, delim: Delimiter) -> PErr<'sess> { // An unexpected closing delimiter (i.e., there is no // matching opening delimiter). let token_str = token_to_string(&self.token); diff --git a/compiler/rustc_parse/src/lexer/unicode_chars.rs b/compiler/rustc_parse/src/lexer/unicode_chars.rs index dac7569e385e7..a136abaa28bb8 100644 --- a/compiler/rustc_parse/src/lexer/unicode_chars.rs +++ b/compiler/rustc_parse/src/lexer/unicode_chars.rs @@ -337,7 +337,7 @@ const ASCII_ARRAY: &[(&str, &str, Option)] = &[ ]; pub(super) fn check_for_substitution( - reader: &StringReader<'_>, + reader: &StringReader<'_, '_>, pos: BytePos, ch: char, count: usize, From 6656413a5c9beb774dfde16804aa523cf9dbf1b5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 14:22:44 +1100 Subject: [PATCH 11/12] Stop using `DiagnosticBuilder::buffer` in the parser. One consequence is that errors returned by `maybe_new_parser_from_source_str` now must be consumed, so a bunch of places that previously ignored those errors now cancel them. (Most of them explicitly dropped the errors before. I guess that was to indicate "we are explicitly ignoring these", though I'm not 100% sure.) --- compiler/rustc_interface/src/interface.rs | 11 +++++--- compiler/rustc_parse/src/lexer/mod.rs | 8 +++--- compiler/rustc_parse/src/lib.rs | 26 +++++++++---------- src/librustdoc/clean/render_macro_matchers.rs | 4 +-- src/librustdoc/doctest.rs | 8 +++--- .../src/doc/needless_doctest_main.rs | 2 +- src/tools/rustfmt/src/parse/parser.rs | 4 +-- src/tools/rustfmt/src/parse/session.rs | 8 +++--- 8 files changed, 39 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 6c000380e713a..03335996c0354 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -82,7 +82,7 @@ pub(crate) fn parse_cfg(dcx: &DiagCtxt, cfgs: Vec) -> Cfg { Ok(..) => {} Err(err) => err.cancel(), }, - Err(errs) => drop(errs), + Err(errs) => errs.into_iter().for_each(|err| err.cancel()), } // If the user tried to use a key="value" flag, but is missing the quotes, provide @@ -129,9 +129,12 @@ pub(crate) fn parse_check_cfg(dcx: &DiagCtxt, specs: Vec) -> CheckCfg { error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`") }; - let Ok(mut parser) = maybe_new_parser_from_source_str(&sess, filename, s.to_string()) - else { - expected_error(); + let mut parser = match maybe_new_parser_from_source_str(&sess, filename, s.to_string()) { + Ok(parser) => parser, + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + expected_error(); + } }; let meta_item = match parser.parse_meta_item() { diff --git a/compiler/rustc_parse/src/lexer/mod.rs b/compiler/rustc_parse/src/lexer/mod.rs index 9626d4e817158..d7ecf577ed676 100644 --- a/compiler/rustc_parse/src/lexer/mod.rs +++ b/compiler/rustc_parse/src/lexer/mod.rs @@ -7,7 +7,7 @@ use rustc_ast::ast::{self, AttrStyle}; use rustc_ast::token::{self, CommentKind, Delimiter, Token, TokenKind}; use rustc_ast::tokenstream::TokenStream; use rustc_ast::util::unicode::contains_text_flow_control_chars; -use rustc_errors::{error_code, Applicability, DiagCtxt, Diagnostic, StashKey}; +use rustc_errors::{error_code, Applicability, DiagCtxt, DiagnosticBuilder, StashKey}; use rustc_lexer::unescape::{self, EscapeError, Mode}; use rustc_lexer::{Base, DocStyle, RawStrError}; use rustc_lexer::{Cursor, LiteralKind}; @@ -47,7 +47,7 @@ pub(crate) fn parse_token_trees<'sess, 'src>( mut src: &'src str, mut start_pos: BytePos, override_span: Option, -) -> Result> { +) -> Result>> { // Skip `#!`, if present. if let Some(shebang_len) = rustc_lexer::strip_shebang(src) { src = &src[shebang_len..]; @@ -76,13 +76,13 @@ pub(crate) fn parse_token_trees<'sess, 'src>( let mut buffer = Vec::with_capacity(1); for unmatched in unmatched_delims { if let Some(err) = make_unclosed_delims_error(unmatched, sess) { - err.buffer(&mut buffer); + buffer.push(err); } } if let Err(errs) = res { // Add unclosing delimiter or diff marker errors for err in errs { - err.buffer(&mut buffer); + buffer.push(err); } } Err(buffer) diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs index 55437de72b5dc..c00e318f22709 100644 --- a/compiler/rustc_parse/src/lib.rs +++ b/compiler/rustc_parse/src/lib.rs @@ -19,7 +19,7 @@ use rustc_ast::tokenstream::TokenStream; use rustc_ast::{AttrItem, Attribute, MetaItem}; use rustc_ast_pretty::pprust; use rustc_data_structures::sync::Lrc; -use rustc_errors::{Diagnostic, PResult}; +use rustc_errors::{DiagnosticBuilder, FatalError, PResult}; use rustc_session::parse::ParseSess; use rustc_span::{FileName, SourceFile, Span}; @@ -45,14 +45,13 @@ rustc_fluent_macro::fluent_messages! { "../messages.ftl" } /// A variant of 'panictry!' that works on a `Vec` instead of a single /// `DiagnosticBuilder`. macro_rules! panictry_buffer { - ($handler:expr, $e:expr) => {{ - use rustc_errors::FatalError; + ($e:expr) => {{ use std::result::Result::{Err, Ok}; match $e { Ok(e) => e, Err(errs) => { for e in errs { - $handler.emit_diagnostic(e); + e.emit(); } FatalError.raise() } @@ -100,16 +99,17 @@ pub fn parse_stream_from_source_str( /// Creates a new parser from a source string. pub fn new_parser_from_source_str(sess: &ParseSess, name: FileName, source: String) -> Parser<'_> { - panictry_buffer!(&sess.dcx, maybe_new_parser_from_source_str(sess, name, source)) + panictry_buffer!(maybe_new_parser_from_source_str(sess, name, source)) } /// Creates a new parser from a source string. Returns any buffered errors from lexing the initial -/// token stream. +/// token stream; these must be consumed via `emit`, `cancel`, etc., otherwise a panic will occur +/// when they are dropped. pub fn maybe_new_parser_from_source_str( sess: &ParseSess, name: FileName, source: String, -) -> Result, Vec> { +) -> Result, Vec>> { maybe_source_file_to_parser(sess, sess.source_map().new_source_file(name, source)) } @@ -125,7 +125,7 @@ pub fn new_parser_from_file<'a>(sess: &'a ParseSess, path: &Path, sp: Option(sess: &'a ParseSess, path: &Path, sp: Option, -) -> Result, Vec> { +) -> Result, Vec>> { let end_pos = source_file.end_position(); let stream = maybe_file_to_stream(sess, source_file, None)?; let mut parser = stream_to_parser(sess, stream, None); @@ -152,16 +152,16 @@ pub fn source_file_to_stream( source_file: Lrc, override_span: Option, ) -> TokenStream { - panictry_buffer!(&sess.dcx, maybe_file_to_stream(sess, source_file, override_span)) + panictry_buffer!(maybe_file_to_stream(sess, source_file, override_span)) } /// Given a source file, produces a sequence of token trees. Returns any buffered errors from /// parsing the token stream. -fn maybe_file_to_stream( - sess: &ParseSess, +fn maybe_file_to_stream<'sess>( + sess: &'sess ParseSess, source_file: Lrc, override_span: Option, -) -> Result> { +) -> Result>> { let src = source_file.src.as_ref().unwrap_or_else(|| { sess.dcx.bug(format!( "cannot lex `source_file` without source: {}", diff --git a/src/librustdoc/clean/render_macro_matchers.rs b/src/librustdoc/clean/render_macro_matchers.rs index 605f9e496c769..b736f4a795614 100644 --- a/src/librustdoc/clean/render_macro_matchers.rs +++ b/src/librustdoc/clean/render_macro_matchers.rs @@ -69,8 +69,8 @@ fn snippet_equal_to_token(tcx: TyCtxt<'_>, matcher: &TokenTree) -> Option parser, - Err(diagnostics) => { - drop(diagnostics); + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); return None; } }; diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 9a9006252684d..82746a7ab0343 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -589,7 +589,7 @@ pub(crate) fn make_test( let mut parser = match maybe_new_parser_from_source_str(&sess, filename, source) { Ok(p) => p, Err(errs) => { - drop(errs); + errs.into_iter().for_each(|err| err.cancel()); return (found_main, found_extern_crate, found_macro); } }; @@ -759,8 +759,10 @@ fn check_if_attr_is_complete(source: &str, edition: Edition) -> bool { let mut parser = match maybe_new_parser_from_source_str(&sess, filename, source.to_owned()) { Ok(p) => p, - Err(_) => { - // If there is an unclosed delimiter, an error will be returned by the tokentrees. + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + // If there is an unclosed delimiter, an error will be returned by the + // tokentrees. return false; } }; diff --git a/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs b/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs index a744b69ecb478..8b018220c1715 100644 --- a/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs +++ b/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs @@ -53,7 +53,7 @@ pub fn check( let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code) { Ok(p) => p, Err(errs) => { - drop(errs); + errs.into_iter().for_each(|err| err.cancel()); return (false, test_attr_spans); }, }; diff --git a/src/tools/rustfmt/src/parse/parser.rs b/src/tools/rustfmt/src/parse/parser.rs index 7045a7dd9ce35..31226cf8c307d 100644 --- a/src/tools/rustfmt/src/parse/parser.rs +++ b/src/tools/rustfmt/src/parse/parser.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use rustc_ast::token::TokenKind; use rustc_ast::{ast, attr, ptr}; -use rustc_errors::Diagnostic; +use rustc_errors::DiagnosticBuilder; use rustc_parse::{new_parser_from_file, parser::Parser as RawParser}; use rustc_span::{sym, Span}; use thin_vec::ThinVec; @@ -65,7 +65,7 @@ impl<'a> ParserBuilder<'a> { fn parser( sess: &'a rustc_session::parse::ParseSess, input: Input, - ) -> Result, Option>> { + ) -> Result, Option>>> { match input { Input::File(ref file) => catch_unwind(AssertUnwindSafe(move || { new_parser_from_file(sess, file, None) diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index f4fb5073dfdcd..6dc3eac44d43d 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -4,7 +4,9 @@ use std::sync::atomic::{AtomicBool, Ordering}; use rustc_data_structures::sync::{IntoDynSyncSend, Lrc}; use rustc_errors::emitter::{DynEmitter, Emitter, HumanEmitter}; use rustc_errors::translation::Translate; -use rustc_errors::{ColorConfig, DiagCtxt, Diagnostic, Level as DiagnosticLevel}; +use rustc_errors::{ + ColorConfig, DiagCtxt, Diagnostic, DiagnosticBuilder, Level as DiagnosticLevel, +}; use rustc_session::parse::ParseSess as RawParseSess; use rustc_span::{ source_map::{FilePathMapping, SourceMap}, @@ -283,9 +285,9 @@ impl ParseSess { // Methods that should be restricted within the parse module. impl ParseSess { - pub(super) fn emit_diagnostics(&self, diagnostics: Vec) { + pub(super) fn emit_diagnostics(&self, diagnostics: Vec>) { for diagnostic in diagnostics { - self.parse_sess.dcx.emit_diagnostic(diagnostic); + diagnostic.emit(); } } From 4fd1db1aa56fba038fd11f70aa6d5c56e0059930 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 14:05:10 +1100 Subject: [PATCH 12/12] Remove `DiagnosticBuilder::buffer`. All its uses have been removed. --- compiler/rustc_errors/src/diagnostic_builder.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index 932b8793807f1..bd7c58d904e70 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -264,11 +264,6 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { self.take_diag() } - /// Buffers the diagnostic for later emission. - pub fn buffer(self, buffered_diagnostics: &mut Vec) { - buffered_diagnostics.push(self.into_diagnostic()); - } - /// Delay emission of this diagnostic as a bug. /// /// This can be useful in contexts where an error indicates a bug but