Skip to content

Commit

Permalink
Auto merge of #75534 - Aaron1011:feature/new-future-breakage, r=pnkfelix
Browse files Browse the repository at this point in the history
Implement rustc side of report-future-incompat

cc #71249

This is an alternative to `@pnkfelix's` initial implementation in https://github.com/pnkfelix/rust/commits/prototype-rustc-side-of-report-future-incompat (mainly because I started working before seeing that branch 😄 ).

My approach outputs the entire original `Diagnostic`, in a way that is compatible with incremental compilation. This is not yet integrated with compiletest, but can be used manually by passing `-Z emit-future-incompat-report` to `rustc`.

Several changes are made to support this feature:
* The `librustc_session/lint` module is moved to a new crate `librustc_lint_defs` (name bikesheddable). This allows accessing lint definitions from `librustc_errors`.
* The `Lint` struct is extended with an `Option<FutureBreakage>`. When present, it indicates that we should display a lint in the future-compat report. `FutureBreakage` contains additional information that we may want to display in the report (currently, a `date` field indicating when the crate will stop compiling).
* A new variant `rustc_error::Level::Allow` is added. This is used when constructing a diagnostic for a future-breakage lint that is marked as allowed (via `#[allow]` or `--cap-lints`). This allows us to capture any future-breakage diagnostics in one place, while still discarding them before they are passed to the `Emitter`.
* `DiagnosticId::Lint` is extended with a `has_future_breakage` field, indicating whether or not the `Lint` has future breakage information (and should therefore show up in the report).
* `Session` is given access to the `LintStore` via a new `SessionLintStore` trait (since `librustc_session` cannot directly reference `LintStore` without a cyclic dependency). We use this to turn a string `DiagnosticId::Lint` back into a `Lint`, to retrieve the `FutureBreakage` data.

Currently, `FutureBreakage.date` is always set to `None`. However, this could potentially be interpreted by Cargo in the future.

I've enabled the future-breakage report for the `ARRAY_INTO_ITER` lint, which can be used to test out this PR. The intent is to use the field to allow Cargo to determine the date of future breakage (as described in [RFC 2834](https://github.com/rust-lang/rfcs/blob/master/text/2834-cargo-report-future-incompat.md)) without needing to parse the diagnostic itself.

cc `@pnkfelix`
  • Loading branch information
bors committed Nov 1, 2020
2 parents 1899c48 + 6db00a2 commit b202532
Show file tree
Hide file tree
Showing 28 changed files with 539 additions and 123 deletions.
14 changes: 14 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3638,6 +3638,7 @@ dependencies = [
"annotate-snippets 0.8.0",
"atty",
"rustc_data_structures",
"rustc_lint_defs",
"rustc_macros",
"rustc_serialize",
"rustc_span",
Expand Down Expand Up @@ -3830,6 +3831,18 @@ dependencies = [
"unicode-security",
]

[[package]]
name = "rustc_lint_defs"
version = "0.0.0"
dependencies = [
"rustc_ast",
"rustc_data_structures",
"rustc_macros",
"rustc_serialize",
"rustc_span",
"tracing",
]

[[package]]
name = "rustc_llvm"
version = "0.0.0"
Expand Down Expand Up @@ -4112,6 +4125,7 @@ dependencies = [
"rustc_errors",
"rustc_feature",
"rustc_fs_util",
"rustc_lint_defs",
"rustc_macros",
"rustc_serialize",
"rustc_span",
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
E0726,
"implicit elided lifetime not allowed here"
);
rustc_session::lint::add_elided_lifetime_in_path_suggestion(
&self.sess,
rustc_errors::add_elided_lifetime_in_path_suggestion(
&self.sess.source_map(),
&mut err,
expected_lifetimes,
path_span,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_errors/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ rustc_serialize = { path = "../rustc_serialize" }
rustc_span = { path = "../rustc_span" }
rustc_macros = { path = "../rustc_macros" }
rustc_data_structures = { path = "../rustc_data_structures" }
rustc_lint_defs = { path = "../rustc_lint_defs" }
unicode-width = "0.1.4"
atty = "0.2"
termcolor = "1.0"
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ fn annotation_type_for_level(level: Level) -> AnnotationType {
Level::Help => AnnotationType::Help,
// FIXME(#59346): Not sure how to map these two levels
Level::Cancelled | Level::FailureNote => AnnotationType::Error,
Level::Allow => panic!("Should not call with Allow"),
}
}

Expand Down Expand Up @@ -143,7 +144,8 @@ impl AnnotateSnippetEmitterWriter {
title: Some(Annotation {
label: Some(&message),
id: code.as_ref().map(|c| match c {
DiagnosticId::Error(val) | DiagnosticId::Lint(val) => val.as_str(),
DiagnosticId::Error(val)
| DiagnosticId::Lint { name: val, has_future_breakage: _ } => val.as_str(),
}),
annotation_type: annotation_type_for_level(*level),
}),
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::snippet::Style;
use crate::Applicability;
use crate::CodeSuggestion;
use crate::Level;
use crate::Substitution;
use crate::SubstitutionPart;
use crate::SuggestionStyle;
use rustc_lint_defs::Applicability;
use rustc_span::{MultiSpan, Span, DUMMY_SP};
use std::fmt;

Expand All @@ -27,7 +27,7 @@ pub struct Diagnostic {
#[derive(Clone, Debug, PartialEq, Eq, Hash, Encodable, Decodable)]
pub enum DiagnosticId {
Error(String),
Lint(String),
Lint { name: String, has_future_breakage: bool },
}

/// For example a note attached to an error.
Expand Down Expand Up @@ -107,7 +107,14 @@ impl Diagnostic {
match self.level {
Level::Bug | Level::Fatal | Level::Error | Level::FailureNote => true,

Level::Warning | Level::Note | Level::Help | Level::Cancelled => false,
Level::Warning | Level::Note | Level::Help | Level::Cancelled | Level::Allow => false,
}
}

pub fn has_future_breakage(&self) -> bool {
match self.code {
Some(DiagnosticId::Lint { has_future_breakage, .. }) => has_future_breakage,
_ => false,
}
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{Applicability, Handler, Level, StashKey};
use crate::{Diagnostic, DiagnosticId, DiagnosticStyledString};
use crate::{Handler, Level, StashKey};
use rustc_lint_defs::Applicability;

use rustc_span::{MultiSpan, Span};
use std::fmt::{self, Debug};
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@

use Destination::*;

use rustc_lint_defs::FutureBreakage;
use rustc_span::source_map::SourceMap;
use rustc_span::{MultiSpan, SourceFile, Span};

use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString};
use crate::styled_buffer::StyledBuffer;
use crate::{
pluralize, CodeSuggestion, Diagnostic, DiagnosticId, Level, SubDiagnostic, SuggestionStyle,
};
use crate::{CodeSuggestion, Diagnostic, DiagnosticId, Level, SubDiagnostic, SuggestionStyle};

use rustc_lint_defs::pluralize;

use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -192,6 +193,8 @@ pub trait Emitter {
/// other formats can, and will, simply ignore it.
fn emit_artifact_notification(&mut self, _path: &Path, _artifact_type: &str) {}

fn emit_future_breakage_report(&mut self, _diags: Vec<(FutureBreakage, Diagnostic)>) {}

/// Checks if should show explanations about "rustc --explain"
fn should_show_explain(&self) -> bool {
true
Expand Down
41 changes: 39 additions & 2 deletions compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ use rustc_span::source_map::{FilePathMapping, SourceMap};

use crate::emitter::{Emitter, HumanReadableErrorType};
use crate::registry::Registry;
use crate::{Applicability, DiagnosticId};
use crate::DiagnosticId;
use crate::{CodeSuggestion, SubDiagnostic};
use rustc_lint_defs::{Applicability, FutureBreakage};

use rustc_data_structures::sync::Lrc;
use rustc_span::hygiene::ExpnData;
Expand Down Expand Up @@ -131,6 +132,31 @@ impl Emitter for JsonEmitter {
}
}

fn emit_future_breakage_report(&mut self, diags: Vec<(FutureBreakage, crate::Diagnostic)>) {
let data: Vec<FutureBreakageItem> = diags
.into_iter()
.map(|(breakage, mut diag)| {
if diag.level == crate::Level::Allow {
diag.level = crate::Level::Warning;
}
FutureBreakageItem {
future_breakage_date: breakage.date,
diagnostic: Diagnostic::from_errors_diagnostic(&diag, self),
}
})
.collect();
let report = FutureIncompatReport { future_incompat_report: data };
let result = if self.pretty {
writeln!(&mut self.dst, "{}", as_pretty_json(&report))
} else {
writeln!(&mut self.dst, "{}", as_json(&report))
}
.and_then(|_| self.dst.flush());
if let Err(e) = result {
panic!("failed to print future breakage report: {:?}", e);
}
}

fn source_map(&self) -> Option<&Lrc<SourceMap>> {
Some(&self.sm)
}
Expand Down Expand Up @@ -223,6 +249,17 @@ struct ArtifactNotification<'a> {
emit: &'a str,
}

#[derive(Encodable)]
struct FutureBreakageItem {
future_breakage_date: Option<&'static str>,
diagnostic: Diagnostic,
}

#[derive(Encodable)]
struct FutureIncompatReport {
future_incompat_report: Vec<FutureBreakageItem>,
}

impl Diagnostic {
fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic {
let sugg = diag.suggestions.iter().map(|sugg| Diagnostic {
Expand Down Expand Up @@ -432,7 +469,7 @@ impl DiagnosticCode {
s.map(|s| {
let s = match s {
DiagnosticId::Error(s) => s,
DiagnosticId::Lint(s) => s,
DiagnosticId::Lint { name, has_future_breakage: _ } => name,
};
let je_result =
je.registry.as_ref().map(|registry| registry.try_find_description(&s)).unwrap();
Expand Down
Loading

0 comments on commit b202532

Please sign in to comment.