Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/librustdoc/errors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use crate::fluent_generated as fluent;
use rustc_errors::IntoDiagnostic;
use rustc_macros::Diagnostic;
use rustc_span::ErrorGuaranteed;

#[derive(Diagnostic)]
#[diag(rustdoc_main_error)]
pub(crate) struct MainError {
pub(crate) error: String,
}

pub(crate) struct CouldntGenerateDocumentation {
pub(crate) error: String,
pub(crate) file: String,
}

impl IntoDiagnostic<'_> for CouldntGenerateDocumentation {
fn into_diagnostic(
self,
handler: &'_ rustc_errors::Handler,
) -> rustc_errors::DiagnosticBuilder<'_, ErrorGuaranteed> {
let mut diag = handler.struct_err(fluent::rustdoc_couldnt_generate_documentation);
diag.set_arg("error", self.error);
if !self.file.is_empty() {
diag.note(fluent::_subdiag::note);
}

diag
}
}

#[derive(Diagnostic)]
#[diag(rustdoc_compilation_failed)]
pub(crate) struct CompilationFailed;
31 changes: 16 additions & 15 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#![warn(rustc::internal)]
#![allow(clippy::collapsible_if, clippy::collapsible_else_if)]
#![allow(rustc::potential_query_instability)]
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]

extern crate thin_vec;
#[macro_use]
Expand All @@ -43,6 +45,7 @@ extern crate rustc_driver;
extern crate rustc_errors;
extern crate rustc_expand;
extern crate rustc_feature;
extern crate rustc_fluent_macro;
extern crate rustc_hir;
extern crate rustc_hir_analysis;
extern crate rustc_hir_pretty;
Expand Down Expand Up @@ -74,15 +77,18 @@ use std::env::{self, VarError};
use std::io::{self, IsTerminal};
use std::process;

use errors::{CouldntGenerateDocumentation, MainError};
use rustc_driver::abort_on_err;
use rustc_errors::ErrorGuaranteed;
use rustc_errors::{DiagnosticMessage, ErrorGuaranteed, SubdiagnosticMessage};
use rustc_fluent_macro::fluent_messages;
use rustc_interface::interface;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::{make_crate_type_option, ErrorOutputType, RustcOptGroup};
use rustc_session::getopts;
use rustc_session::{early_error, early_warn};

use crate::clean::utils::DOC_RUST_LANG_ORG_CHANNEL;
use crate::errors::CompilationFailed;

/// A macro to create a FxHashMap.
///
Expand All @@ -108,6 +114,7 @@ mod core;
mod docfs;
mod doctest;
mod error;
mod errors;
mod externalfiles;
mod fold;
mod formats;
Expand All @@ -123,6 +130,8 @@ mod visit;
mod visit_ast;
mod visit_lib;

fluent_messages! { "./messages.ftl" }

pub fn main() {
// See docs in https://github.com/rust-lang/rust/blob/master/compiler/rustc/src/main.rs
// about jemalloc.
Expand Down Expand Up @@ -683,10 +692,7 @@ type MainResult = Result<(), ErrorGuaranteed>;
fn wrap_return(diag: &rustc_errors::Handler, res: Result<(), String>) -> MainResult {
match res {
Ok(()) => diag.has_errors().map_or(Ok(()), Err),
Err(err) => {
let reported = diag.struct_err(err).emit();
Err(reported)
}
Err(error) => Err(diag.emit_err(MainError { error })),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it makes sense to "translate" this -- it's not any better in this current state, IMO. Can you just add diagnostic_outside_of_impl for this function and remove MainError?

Ideally we'd change Result<(), String> into something that's like Result<(), T> where T: IntoDiagnostic or something...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some thought, realized that the error string that we got here comes from Result, which is in English.

If we would like to make this section translatable, then we need to support translations of Result::Err. Is this even feasible?

}
}

Expand All @@ -698,15 +704,10 @@ fn run_renderer<'tcx, T: formats::FormatRenderer<'tcx>>(
) -> MainResult {
match formats::run_format::<T>(krate, renderopts, cache, tcx) {
Ok(_) => tcx.sess.has_errors().map_or(Ok(()), Err),
Err(e) => {
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.note(format!("failed to create or modify \"{}\"", file));
}
Err(msg.emit())
}
Err(e) => Err(tcx.sess.emit_err(CouldntGenerateDocumentation {
error: e.error,
file: e.file.display().to_string(),
Copy link
Contributor

@compiler-errors compiler-errors Jun 19, 2023

Choose a reason for hiding this comment

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

Is file ever actually empty in practice? If so, I think it may be better to represent this as Option<String> so you can just annotate it with #[note] instead of having a custom impl IntoDiagnostic<'_> for CouldntGenerateDocumentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is another alternative that I found, it requires adding another struct for a Subdiagnostic note.

#[derive(Diagnostic)]
#[diag(rustdoc_couldnt_generate_documentation)]
pub(crate) struct CouldntGenerateDocumentation {
    pub(crate) error: String,
    #[subdiagnostic]
    pub(crate) file: Option<FailedToCreateOrModifyFile>,
}

#[derive(Subdiagnostic)]
#[note(rustdoc_failed_to_create_or_modify_file)]
pub(crate) struct FailedToCreateOrModifyFile {
    pub(crate) file: String,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having Option<String> for the file field (in the version specified in this review comment) and adding #[note] to the struct causes compilation failures.

This is because Option<String> doesn't implement IntoDiagnosticArgs. Implementing IntoDiagnosticArgs for Option<String> is also not allowed due to the orphan rules. Creating a new (wrapped) type for Option<String>, and then implementing the trait wouldn't work because the Option is "gone" from the struct field.

CMIIW here :)

})),
}
}

Expand Down Expand Up @@ -817,7 +818,7 @@ fn main_args(at_args: &[String]) -> MainResult {
compiler.enter(|queries| {
let mut gcx = abort_on_err(queries.global_ctxt(), sess);
if sess.diagnostic().has_errors_or_lint_errors().is_some() {
sess.fatal("Compilation failed, aborting rustdoc");
sess.emit_fatal(CompilationFailed);
}

gcx.enter(|tcx| {
Expand Down
7 changes: 7 additions & 0 deletions src/librustdoc/messages.ftl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
rustdoc_main_error = {$error}
rustdoc_couldnt_generate_documentation =
couldn't generate documentation: {$error}
.note = failed to create or modify "{$file}"
rustdoc_compilation_failed = Compilation failed, aborting rustdoc
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is pre-existing, but it's conventional for diagnostics to be lowercase. Not sure if rustdoc is special in this regard or if this was just an oversight.

Suggested change
rustdoc_compilation_failed = Compilation failed, aborting rustdoc
rustdoc_compilation_failed = compilation failed, aborting rustdoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the diagnostics in rustdoc are in lowercase (around < 5 that are not).

Shall we set all them to lowercase? @jsha