diff options
| author | bors <bors@rust-lang.org> | 2022-02-25 00:46:04 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-02-25 00:46:04 +0000 |
| commit | d4de1f230ca30b7ce08fbf453daebf8b2e7ffcc9 (patch) | |
| tree | 922a1d8bf2850b2f35dda5bc6cc14e9194c375c1 /src/librustdoc | |
| parent | 4e82f35492ea5c78e19609bf4468f0a686d9a756 (diff) | |
| parent | b7e95dee65c35db8f8e07046d445b12d92cbae12 (diff) | |
Auto merge of #93368 - eddyb:diagbld-guarantee, r=estebank
rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission".
That is, `DiagnosticBuilder` is now generic over the return type of `.emit()`, so we'll now have:
* `DiagnosticBuilder<ErrorReported>` for error (incl. fatal/bug) diagnostics
* can only be created via a `const L: Level`-generic constructor, that limits allowed variants via a `where` clause, so not even `rustc_errors` can accidentally bypass this limitation
* asserts `diagnostic.is_error()` on emission, just in case the construction restriction was bypassed (e.g. by replacing the whole `Diagnostic` inside `DiagnosticBuilder`)
* `.emit()` returns `ErrorReported`, as a "proof" token that `.emit()` was called
(though note that this isn't a real guarantee until after completing the work on
#69426)
* `DiagnosticBuilder<()>` for everything else (warnings, notes, etc.)
* can also be obtained from other `DiagnosticBuilder`s by calling `.forget_guarantee()`
This PR is a companion to other ongoing work, namely:
* #69426
and it's ongoing implementation:
#93222
the API changes in this PR are needed to get statically-checked "only errors produce `ErrorReported` from `.emit()`", but doesn't itself provide any really strong guarantees without those other `ErrorReported` changes
* #93244
would make the choices of API changes (esp. naming) in this PR fit better overall
In order to be able to let `.emit()` return anything trustable, several changes had to be made:
* `Diagnostic`'s `level` field is now private to `rustc_errors`, to disallow arbitrary "downgrade"s from "some kind of error" to "warning" (or anything else that doesn't cause compilation to fail)
* it's still possible to replace the whole `Diagnostic` inside the `DiagnosticBuilder`, sadly, that's harder to fix, but it's unlikely enough that we can paper over it with asserts on `.emit()`
* `.cancel()` now consumes `DiagnosticBuilder`, preventing `.emit()` calls on a cancelled diagnostic
* it's also now done internally, through `DiagnosticBuilder`-private state, instead of having a `Level::Cancelled` variant that can be read (or worse, written) by the user
* this removes a hazard of calling `.cancel()` on an error then continuing to attach details to it, and even expect to be able to `.emit()` it
* warnings were switched to *only* `can_emit_warnings` on emission (instead of pre-cancelling early)
* `struct_dummy` was removed (as it relied on a pre-`Cancelled` `Diagnostic`)
* since `.emit()` doesn't consume the `DiagnosticBuilder` <sub>(I tried and gave up, it's much more work than this PR)</sub>,
we have to make `.emit()` idempotent wrt the guarantees it returns
* thankfully, `err.emit(); err.emit();` can return `ErrorReported` both times, as the second `.emit()` call has no side-effects *only* because the first one did do the appropriate emission
* `&mut Diagnostic` is now used in a lot of function signatures, which used to take `&mut DiagnosticBuilder` (in the interest of not having to make those functions generic)
* the APIs were already mostly identical, allowing for low-effort porting to this new setup
* only some of the suggestion methods needed some rework, to have the extra `DiagnosticBuilder` functionality on the `Diagnostic` methods themselves (that change is also present in #93259)
* `.emit()`/`.cancel()` aren't available, but IMO calling them from an "error decorator/annotator" function isn't a good practice, and can lead to strange behavior (from the caller's perspective)
* `.downgrade_to_delayed_bug()` was added, letting you convert any `.is_error()` diagnostic into a `delay_span_bug` one (which works because in both cases the guarantees available are the same)
This PR should ideally be reviewed commit-by-commit, since there is a lot of fallout in each.
r? `@estebank` cc `@Manishearth` `@nikomatsakis` `@mark-i-m`
Diffstat (limited to 'src/librustdoc')
| -rw-r--r-- | src/librustdoc/clean/render_macro_matchers.rs | 6 | ||||
| -rw-r--r-- | src/librustdoc/doctest.rs | 7 | ||||
| -rw-r--r-- | src/librustdoc/lib.rs | 8 | ||||
| -rw-r--r-- | src/librustdoc/passes/collect_intra_doc_links.rs | 19 |
4 files changed, 20 insertions, 20 deletions
diff --git a/src/librustdoc/clean/render_macro_matchers.rs b/src/librustdoc/clean/render_macro_matchers.rs index dff370ab750..58ca8869ea9 100644 --- a/src/librustdoc/clean/render_macro_matchers.rs +++ b/src/librustdoc/clean/render_macro_matchers.rs @@ -69,9 +69,7 @@ fn snippet_equal_to_token(tcx: TyCtxt<'_>, matcher: &TokenTree) -> Option<String match rustc_parse::maybe_new_parser_from_source_str(&sess, file_name, snippet.clone()) { Ok(parser) => parser, Err(diagnostics) => { - for mut diagnostic in diagnostics { - diagnostic.cancel(); - } + drop(diagnostics); return None; } }; @@ -79,7 +77,7 @@ fn snippet_equal_to_token(tcx: TyCtxt<'_>, matcher: &TokenTree) -> Option<String // Reparse a single token tree. let mut reparsed_trees = match parser.parse_all_token_trees() { Ok(reparsed_trees) => reparsed_trees, - Err(mut diagnostic) => { + Err(diagnostic) => { diagnostic.cancel(); return None; } diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 5ccc3dabe83..a08732be1c5 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -551,10 +551,7 @@ crate fn make_test( let mut parser = match maybe_new_parser_from_source_str(&sess, filename, source) { Ok(p) => p, Err(errs) => { - for mut err in errs { - err.cancel(); - } - + drop(errs); return (found_main, found_extern_crate, found_macro); } }; @@ -594,7 +591,7 @@ crate fn make_test( } } Ok(None) => break, - Err(mut e) => { + Err(e) => { e.cancel(); break; } diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 7eff725989c..9d3e58a3a66 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -720,12 +720,10 @@ fn run_renderer<'tcx, T: formats::FormatRenderer<'tcx>>( let mut msg = tcx.sess.struct_err(&format!("couldn't generate documentation: {}", e.error)); let file = e.file.display().to_string(); - if file.is_empty() { - msg.emit() - } else { - msg.note(&format!("failed to create or modify \"{}\"", file)).emit() + if !file.is_empty() { + msg.note(&format!("failed to create or modify \"{}\"", file)); } - Err(ErrorReported) + Err(msg.emit()) } } } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 33a1530d588..3ebd28e83b1 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -3,7 +3,7 @@ //! [RFC 1946]: https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md use rustc_data_structures::{fx::FxHashMap, stable_set::FxHashSet}; -use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc_errors::{Applicability, Diagnostic}; use rustc_hir::def::{ DefKind, Namespace::{self, *}, @@ -1434,7 +1434,7 @@ impl LinkCollector<'_, '_> { ) { // The resolved item did not match the disambiguator; give a better error than 'not found' let msg = format!("incompatible link kind for `{}`", path_str); - let callback = |diag: &mut DiagnosticBuilder<'_>, sp: Option<rustc_span::Span>| { + let callback = |diag: &mut Diagnostic, sp: Option<rustc_span::Span>| { let note = format!( "this link resolved to {} {}, which is not {} {}", resolved.article(), @@ -1866,7 +1866,7 @@ fn report_diagnostic( lint: &'static Lint, msg: &str, DiagnosticInfo { item, ori_link: _, dox, link_range }: &DiagnosticInfo<'_>, - decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option<rustc_span::Span>), + decorate: impl FnOnce(&mut Diagnostic, Option<rustc_span::Span>), ) { let hir_id = match DocContext::as_local_hir_id(tcx, item.def_id) { Some(hir_id) => hir_id, @@ -2098,8 +2098,15 @@ fn resolution_failure( ) } ResolutionFailure::NoParentItem => { - diag.level = rustc_errors::Level::Bug; - "all intra-doc links should have a parent item".to_owned() + // FIXME(eddyb) this doesn't belong here, whatever made + // the `ResolutionFailure::NoParentItem` should emit an + // immediate or delayed `span_bug` about the issue. + tcx.sess.delay_span_bug( + sp.unwrap_or(DUMMY_SP), + "intra-doc link missing parent item", + ); + + "BUG: all intra-doc links should have a parent item".to_owned() } ResolutionFailure::MalformedGenerics(variant) => match variant { MalformedGenerics::UnbalancedAngleBrackets => { @@ -2233,7 +2240,7 @@ fn ambiguity_error( /// disambiguator. fn suggest_disambiguator( res: Res, - diag: &mut DiagnosticBuilder<'_>, + diag: &mut Diagnostic, path_str: &str, ori_link: &str, sp: Option<rustc_span::Span>, |
