Skip to content

Commit cd9e72e

Browse files
committed
Auto merge of #119146 - nnethercote:rm-DiagCtxt-api-duplication, r=compiler-errors
Remove `DiagCtxt` API duplication `DiagCtxt` defines the internal API for creating and emitting diagnostics: methods like `struct_err`, `struct_span_warn`, `note`, `create_fatal`, `emit_bug`. There are over 50 methods. Some of these methods are then duplicated across several other types: `Session`, `ParseSess`, `Parser`, `ExtCtxt`, and `MirBorrowckCtxt`. `Session` duplicates the most, though half the ones it does are unused. Each duplicated method just calls forward to the corresponding method in `DiagCtxt`. So this duplication exists to (in the best case) shorten chains like `ecx.tcx.sess.parse_sess.dcx.emit_err()` to `ecx.emit_err()`. This API duplication is ugly and has been bugging me for a while. And it's inconsistent: there's no real logic about which methods are duplicated, and the use of `#[rustc_lint_diagnostic]` and `#[track_caller]` attributes vary across the duplicates. This PR removes the duplicated API methods and makes all diagnostic creation and emission go through `DiagCtxt`. It also adds `dcx` getter methods to several types to shorten chains. This approach scales *much* better than API duplication; indeed, the PR adds `dcx()` to numerous types that didn't have API duplication: `TyCtxt`, `LoweringCtxt`, `ConstCx`, `FnCtxt`, `TypeErrCtxt`, `InferCtxt`, `CrateLoader`, `CheckAttrVisitor`, and `Resolver`. These result in a lot of changes from `foo.tcx.sess.emit_err()` to `foo.dcx().emit_err()`. (You could do this with more types, but it gets into diminishing returns territory for types that don't emit many diagnostics.) After all these changes, some call sites are more verbose, some are less verbose, and many are the same. The total number of lines is reduced, mostly because of the removed API duplication. And consistency is increased, because calls to `emit_err` and friends are always preceded with `.dcx()` or `.dcx`. r? `@compiler-errors`
2 parents fb96fb1 + b949a32 commit cd9e72e

File tree

4 files changed

+15
-15
lines changed

4 files changed

+15
-15
lines changed

clippy_config/src/conf.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -636,11 +636,11 @@ impl Conf {
636636
match path {
637637
Ok((_, warnings)) => {
638638
for warning in warnings {
639-
sess.warn(warning.clone());
639+
sess.dcx().warn(warning.clone());
640640
}
641641
},
642642
Err(error) => {
643-
sess.err(format!("error finding Clippy's configuration file: {error}"));
643+
sess.dcx().err(format!("error finding Clippy's configuration file: {error}"));
644644
},
645645
}
646646

@@ -652,7 +652,7 @@ impl Conf {
652652
Ok((Some(path), _)) => match sess.source_map().load_file(path) {
653653
Ok(file) => deserialize(&file),
654654
Err(error) => {
655-
sess.err(format!("failed to read `{}`: {error}", path.display()));
655+
sess.dcx().err(format!("failed to read `{}`: {error}", path.display()));
656656
TryConf::default()
657657
},
658658
},
@@ -663,14 +663,14 @@ impl Conf {
663663

664664
// all conf errors are non-fatal, we just use the default conf in case of error
665665
for error in errors {
666-
sess.span_err(
666+
sess.dcx().span_err(
667667
error.span,
668668
format!("error reading Clippy's configuration file: {}", error.message),
669669
);
670670
}
671671

672672
for warning in warnings {
673-
sess.span_warn(
673+
sess.dcx().span_warn(
674674
warning.span,
675675
format!("error reading Clippy's configuration file: {}", warning.message),
676676
);

clippy_config/src/msrvs.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ impl Msrv {
8383
(None, Some(cargo_msrv)) => self.stack = vec![cargo_msrv],
8484
(Some(clippy_msrv), Some(cargo_msrv)) => {
8585
if clippy_msrv != cargo_msrv {
86-
sess.warn(format!(
86+
sess.dcx().warn(format!(
8787
"the MSRV in `clippy.toml` and `Cargo.toml` differ; using `{clippy_msrv}` from `clippy.toml`"
8888
));
8989
}
@@ -106,7 +106,7 @@ impl Msrv {
106106

107107
if let Some(msrv_attr) = msrv_attrs.next() {
108108
if let Some(duplicate) = msrv_attrs.last() {
109-
sess.struct_span_err(duplicate.span, "`clippy::msrv` is defined multiple times")
109+
sess.dcx().struct_span_err(duplicate.span, "`clippy::msrv` is defined multiple times")
110110
.span_note(msrv_attr.span, "first definition found here")
111111
.emit();
112112
}
@@ -116,9 +116,9 @@ impl Msrv {
116116
return Some(version);
117117
}
118118

119-
sess.span_err(msrv_attr.span, format!("`{msrv}` is not a valid Rust version"));
119+
sess.dcx().span_err(msrv_attr.span, format!("`{msrv}` is not a valid Rust version"));
120120
} else {
121-
sess.span_err(msrv_attr.span, "bad clippy attribute");
121+
sess.dcx().span_err(msrv_attr.span, "bad clippy attribute");
122122
}
123123
}
124124

clippy_lints/src/missing_const_for_fn.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn {
153153

154154
if let Err((span, err)) = is_min_const_fn(cx.tcx, mir, &self.msrv) {
155155
if cx.tcx.is_const_fn_raw(def_id.to_def_id()) {
156-
cx.tcx.sess.span_err(span, err);
156+
cx.tcx.dcx().span_err(span, err);
157157
}
158158
} else {
159159
span_lint(cx, MISSING_CONST_FOR_FN, span, "this could be a `const fn`");

clippy_utils/src/attrs.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,12 @@ pub fn get_attr<'a>(
7676
})
7777
.map_or_else(
7878
|| {
79-
sess.span_err(attr_segments[1].ident.span, "usage of unknown attribute");
79+
sess.dcx().span_err(attr_segments[1].ident.span, "usage of unknown attribute");
8080
false
8181
},
8282
|deprecation_status| {
8383
let mut diag =
84-
sess.struct_span_err(attr_segments[1].ident.span, "usage of deprecated attribute");
84+
sess.dcx().struct_span_err(attr_segments[1].ident.span, "usage of deprecated attribute");
8585
match *deprecation_status {
8686
DeprecationStatus::Deprecated => {
8787
diag.emit();
@@ -116,10 +116,10 @@ fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'
116116
if let Ok(value) = FromStr::from_str(value.as_str()) {
117117
f(value);
118118
} else {
119-
sess.span_err(attr.span, "not a number");
119+
sess.dcx().span_err(attr.span, "not a number");
120120
}
121121
} else {
122-
sess.span_err(attr.span, "bad clippy attribute");
122+
sess.dcx().span_err(attr.span, "bad clippy attribute");
123123
}
124124
}
125125
}
@@ -132,7 +132,7 @@ pub fn get_unique_attr<'a>(
132132
let mut unique_attr: Option<&ast::Attribute> = None;
133133
for attr in get_attr(sess, attrs, name) {
134134
if let Some(duplicate) = unique_attr {
135-
sess.struct_span_err(attr.span, format!("`{name}` is defined multiple times"))
135+
sess.dcx().struct_span_err(attr.span, format!("`{name}` is defined multiple times"))
136136
.span_note(duplicate.span, "first definition found here")
137137
.emit();
138138
} else {

0 commit comments

Comments
 (0)