Skip to content

Commit

Permalink
Rework how diagnostic lints are stored.
Browse files Browse the repository at this point in the history
`Diagnostic::code` has the type `DiagnosticId`, which has `Error` and
`Lint` variants. Plus `Diagnostic::is_lint` is a bool, which should be
redundant w.r.t. `Diagnostic::code`.

Seems simple. Except it's possible for a lint to have an error code, in
which case its `code` field is recorded as `Error`, and `is_lint` is
required to indicate that it's a lint. This is what happens with
`derive(LintDiagnostic)` lints. Which means those lints don't have a
lint name or a `has_future_breakage` field because those are stored in
the `DiagnosticId::Lint`.

It's all a bit messy and confused and seems unintentional.

This commit:
- removes `DiagnosticId`;
- changes `Diagnostic::code` to `Option<String>`, which means both
  errors and lints can straightforwardly have an error code;
- changes `Diagnostic::is_lint` to `Option<IsLint>`, where `IsLint` is a
  new type containing a lint name and a `has_future_breakage` bool, so
  all lints can have those, error code or not.
  • Loading branch information
nnethercote committed Jan 14, 2024
1 parent 2de99ec commit 6748d95
Show file tree
Hide file tree
Showing 23 changed files with 108 additions and 128 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_data_structures::memmap::Mmap;
use rustc_data_structures::profiling::{SelfProfilerRef, VerboseTimingGuard};
use rustc_data_structures::sync::Lrc;
use rustc_errors::emitter::Emitter;
use rustc_errors::{translation::Translate, DiagCtxt, DiagnosticId, FatalError, Level};
use rustc_errors::{translation::Translate, DiagCtxt, FatalError, Level};
use rustc_errors::{DiagnosticBuilder, DiagnosticMessage, Style};
use rustc_fs_util::link_or_copy;
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
Expand Down Expand Up @@ -1000,7 +1000,7 @@ type DiagnosticArgName<'source> = Cow<'source, str>;
struct Diagnostic {
msgs: Vec<(DiagnosticMessage, Style)>,
args: FxHashMap<DiagnosticArgName<'static>, rustc_errors::DiagnosticArgValue<'static>>,
code: Option<DiagnosticId>,
code: Option<String>,
lvl: Level,
}

Expand Down
12 changes: 4 additions & 8 deletions compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use crate::emitter::FileWithAnnotatedLines;
use crate::snippet::Line;
use crate::translation::{to_fluent_args, Translate};
use crate::{
CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, Emitter, FluentBundle,
LazyFallbackBundle, Level, MultiSpan, Style, SubDiagnostic,
CodeSuggestion, Diagnostic, DiagnosticMessage, Emitter, FluentBundle, LazyFallbackBundle,
Level, MultiSpan, Style, SubDiagnostic,
};
use annotate_snippets::{Annotation, AnnotationType, Renderer, Slice, Snippet, SourceAnnotation};
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -127,7 +127,7 @@ impl AnnotateSnippetEmitter {
level: &Level,
messages: &[(DiagnosticMessage, Style)],
args: &FluentArgs<'_>,
code: &Option<DiagnosticId>,
code: &Option<String>,
msp: &MultiSpan,
_children: &[SubDiagnostic],
_suggestions: &[CodeSuggestion],
Expand Down Expand Up @@ -181,11 +181,7 @@ impl AnnotateSnippetEmitter {
let snippet = Snippet {
title: Some(Annotation {
label: Some(&message),
id: code.as_ref().map(|c| match c {
DiagnosticId::Error(val) | DiagnosticId::Lint { name: val, .. } => {
val.as_str()
}
}),
id: code.as_deref(),
annotation_type: annotation_type_for_level(*level),
}),
footer: vec![],
Expand Down
45 changes: 21 additions & 24 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub struct Diagnostic {
pub(crate) level: Level,

pub messages: Vec<(DiagnosticMessage, Style)>,
pub code: Option<DiagnosticId>,
pub code: Option<String>,
pub span: MultiSpan,
pub children: Vec<SubDiagnostic>,
pub suggestions: Result<Vec<CodeSuggestion>, SuggestionsDisabled>,
Expand All @@ -115,9 +115,9 @@ pub struct Diagnostic {
/// `span` if there is one. Otherwise, it is `DUMMY_SP`.
pub sort_span: Span,

/// If diagnostic is from Lint, custom hash function ignores notes
/// otherwise hash is based on the all the fields
pub is_lint: bool,
/// If diagnostic is from Lint, custom hash function ignores children.
/// Otherwise hash is based on the all the fields.
pub is_lint: Option<IsLint>,

/// With `-Ztrack_diagnostics` enabled,
/// we print where in rustc this error was emitted.
Expand Down Expand Up @@ -146,13 +146,11 @@ impl fmt::Display for DiagnosticLocation {
}

#[derive(Clone, Debug, PartialEq, Eq, Hash, Encodable, Decodable)]
pub enum DiagnosticId {
Error(String),
Lint {
name: String,
/// Indicates whether this lint should show up in cargo's future breakage report.
has_future_breakage: bool,
},
pub struct IsLint {
/// The lint name.
pub(crate) name: String,
/// Indicates whether this lint should show up in cargo's future breakage report.
has_future_breakage: bool,
}

/// A "sub"-diagnostic attached to a parent diagnostic.
Expand Down Expand Up @@ -231,7 +229,7 @@ impl Diagnostic {
suggestions: Ok(vec![]),
args: Default::default(),
sort_span: DUMMY_SP,
is_lint: false,
is_lint: None,
emitted_at: DiagnosticLocation::caller(),
}
}
Expand Down Expand Up @@ -288,16 +286,13 @@ impl Diagnostic {

/// Indicates whether this diagnostic should show up in cargo's future breakage report.
pub(crate) fn has_future_breakage(&self) -> bool {
match self.code {
Some(DiagnosticId::Lint { has_future_breakage, .. }) => has_future_breakage,
_ => false,
}
matches!(self.is_lint, Some(IsLint { has_future_breakage: true, .. }))
}

pub(crate) fn is_force_warn(&self) -> bool {
match self.level {
Level::ForceWarning(_) => {
assert!(self.is_lint);
assert!(self.is_lint.is_some());
true
}
_ => false,
Expand Down Expand Up @@ -893,12 +888,12 @@ impl Diagnostic {
self
}

pub fn is_lint(&mut self) -> &mut Self {
self.is_lint = true;
pub fn is_lint(&mut self, name: String, has_future_breakage: bool) -> &mut Self {
self.is_lint = Some(IsLint { name, has_future_breakage });
self
}

pub fn code(&mut self, s: DiagnosticId) -> &mut Self {
pub fn code(&mut self, s: String) -> &mut Self {
self.code = Some(s);
self
}
Expand All @@ -908,8 +903,8 @@ impl Diagnostic {
self
}

pub fn get_code(&self) -> Option<DiagnosticId> {
self.code.clone()
pub fn get_code(&self) -> Option<&str> {
self.code.as_deref()
}

pub fn primary_message(&mut self, msg: impl Into<DiagnosticMessage>) -> &mut Self {
Expand Down Expand Up @@ -995,7 +990,8 @@ impl Diagnostic {
&Level,
&[(DiagnosticMessage, Style)],
Vec<(&Cow<'static, str>, &DiagnosticArgValue<'static>)>,
&Option<DiagnosticId>,
&Option<String>,
&Option<IsLint>,
&MultiSpan,
&Result<Vec<CodeSuggestion>, SuggestionsDisabled>,
Option<&[SubDiagnostic]>,
Expand All @@ -1005,9 +1001,10 @@ impl Diagnostic {
&self.messages,
self.args().collect(),
&self.code,
&self.is_lint,
&self.span,
&self.suggestions,
(if self.is_lint { None } else { Some(&self.children) }),
(if self.is_lint.is_some() { None } else { Some(&self.children) }),
)
}
}
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::diagnostic::IntoDiagnosticArg;
use crate::{DiagCtxt, Level, MultiSpan, StashKey};
use crate::{
Diagnostic, DiagnosticId, DiagnosticMessage, DiagnosticStyledString, ErrorGuaranteed,
ExplicitBug, SubdiagnosticMessage,
Diagnostic, DiagnosticMessage, DiagnosticStyledString, ErrorGuaranteed, ExplicitBug,
SubdiagnosticMessage,
};
use rustc_lint_defs::Applicability;
use rustc_span::source_map::Spanned;
Expand Down Expand Up @@ -395,8 +395,11 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
forward!((span, with_span)(
sp: impl Into<MultiSpan>,
));
forward!((is_lint, with_is_lint)(
name: String, has_future_breakage: bool,
));
forward!((code, with_code)(
s: DiagnosticId,
s: String,
));
forward!((arg, with_arg)(
name: impl Into<Cow<'static, str>>, arg: impl IntoDiagnosticArg,
Expand Down Expand Up @@ -443,5 +446,5 @@ macro_rules! struct_span_code_err {

#[macro_export]
macro_rules! error_code {
($code:ident) => {{ $crate::DiagnosticId::Error(stringify!($code).to_owned()) }};
($code:ident) => {{ stringify!($code).to_owned() }};
}
17 changes: 8 additions & 9 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ use crate::snippet::{
use crate::styled_buffer::StyledBuffer;
use crate::translation::{to_fluent_args, Translate};
use crate::{
diagnostic::DiagnosticLocation, CodeSuggestion, DiagCtxt, Diagnostic, DiagnosticId,
DiagnosticMessage, FluentBundle, LazyFallbackBundle, Level, MultiSpan, SubDiagnostic,
SubstitutionHighlight, SuggestionStyle, TerminalUrl,
diagnostic::DiagnosticLocation, CodeSuggestion, DiagCtxt, Diagnostic, DiagnosticMessage,
FluentBundle, LazyFallbackBundle, Level, MultiSpan, SubDiagnostic, SubstitutionHighlight,
SuggestionStyle, TerminalUrl,
};
use rustc_lint_defs::pluralize;

Expand Down Expand Up @@ -1309,7 +1309,7 @@ impl HumanEmitter {
msp: &MultiSpan,
msgs: &[(DiagnosticMessage, Style)],
args: &FluentArgs<'_>,
code: &Option<DiagnosticId>,
code: &Option<String>,
level: &Level,
max_line_num_len: usize,
is_secondary: bool,
Expand All @@ -1336,14 +1336,13 @@ impl HumanEmitter {
buffer.append(0, level.to_str(), Style::Level(*level));
label_width += level.to_str().len();
}
// only render error codes, not lint codes
if let Some(DiagnosticId::Error(ref code)) = *code {
if let Some(code) = code {
buffer.append(0, "[", Style::Level(*level));
let code = if let TerminalUrl::Yes = self.terminal_url {
let path = "https://doc.rust-lang.org/error_codes";
format!("\x1b]8;;{path}/{code}.html\x07{code}\x1b]8;;\x07")
Cow::Owned(format!("\x1b]8;;{path}/{code}.html\x07{code}\x1b]8;;\x07"))
} else {
code.clone()
Cow::Borrowed(code)
};
buffer.append(0, &code, Style::Level(*level));
buffer.append(0, "]", Style::Level(*level));
Expand Down Expand Up @@ -2077,7 +2076,7 @@ impl HumanEmitter {
level: &Level,
messages: &[(DiagnosticMessage, Style)],
args: &FluentArgs<'_>,
code: &Option<DiagnosticId>,
code: &Option<String>,
span: &MultiSpan,
children: &[SubDiagnostic],
suggestions: &[CodeSuggestion],
Expand Down
37 changes: 17 additions & 20 deletions compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ use termcolor::{ColorSpec, WriteColor};
use crate::emitter::{should_show_source_code, Emitter, HumanReadableErrorType};
use crate::registry::Registry;
use crate::translation::{to_fluent_args, Translate};
use crate::DiagnosticId;
use crate::{
CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, SubDiagnostic,
TerminalUrl,
diagnostic::IsLint, CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel,
SubDiagnostic, TerminalUrl,
};
use rustc_lint_defs::Applicability;

Expand Down Expand Up @@ -301,7 +300,8 @@ struct DiagnosticSpanMacroExpansion {

#[derive(Serialize)]
struct DiagnosticCode {
/// The code itself.
/// The error code (e.g. "E1234"), if the diagnostic has one. Or the lint
/// name, if it's a lint without an error code.
code: String,
/// An explanation for the code.
explanation: Option<&'static str>,
Expand Down Expand Up @@ -399,9 +399,21 @@ impl Diagnostic {
let output = String::from_utf8(output).unwrap();

let translated_message = je.translate_messages(&diag.messages, &args);

let code = if let Some(code) = &diag.code {
Some(DiagnosticCode {
code: code.to_string(),
explanation: je.registry.as_ref().unwrap().try_find_description(&code).ok(),
})
} else if let Some(IsLint { name, .. }) = &diag.is_lint {
Some(DiagnosticCode { code: name.to_string(), explanation: None })
} else {
None
};

Diagnostic {
message: translated_message.to_string(),
code: DiagnosticCode::map_opt_string(diag.code.clone(), je),
code,
level: diag.level.to_str(),
spans: DiagnosticSpan::from_multispan(&diag.span, &args, je),
children: diag
Expand Down Expand Up @@ -592,18 +604,3 @@ impl DiagnosticSpanLine {
.unwrap_or_else(|_| vec![])
}
}

impl DiagnosticCode {
fn map_opt_string(s: Option<DiagnosticId>, je: &JsonEmitter) -> Option<DiagnosticCode> {
s.map(|s| {
let s = match s {
DiagnosticId::Error(s) => s,
DiagnosticId::Lint { name, .. } => name,
};
let je_result =
je.registry.as_ref().map(|registry| registry.try_find_description(&s)).unwrap();

DiagnosticCode { code: s, explanation: je_result.ok() }
})
}
}
Loading

0 comments on commit 6748d95

Please sign in to comment.