Skip to content

Commit

Permalink
Make DiagnosticBuilder::emit consuming.
Browse files Browse the repository at this point in the history
This works for most of its call sites. This is nice, because `emit` very
much makes sense as a consuming operation -- indeed,
`DiagnosticBuilderState` exists to ensure no diagnostic is emitted
twice, but it uses runtime checks.

For the small number of call sites where a consuming emit doesn't work,
the commit adds `DiagnosticBuilder::emit_without_consuming`. (This will
be removed in subsequent commits.)

Likewise, `emit_unless` becomes consuming. And `delay_as_bug` becomes
consuming, while `delay_as_bug_without_consuming` is added (which will
also be removed in subsequent commits.)

All this requires significant changes to `DiagnosticBuilder`'s chaining
methods. Currently `DiagnosticBuilder` method chaining uses a
non-consuming `&mut self -> &mut Self` style, which allows chaining to
be used when the chain ends in `emit()`, like so:
```
    struct_err(msg).span(span).emit();
```
But it doesn't work when producing a `DiagnosticBuilder` value,
requiring this:
```
    let mut err = self.struct_err(msg);
    err.span(span);
    err
```
This style of chaining won't work with consuming `emit` though. For
that, we need to use to a `self -> Self` style. That also would allow
`DiagnosticBuilder` production to be chained, e.g.:
```
    self.struct_err(msg).span(span)
```
However, removing the `&mut self -> &mut Self` style would require that
individual modifications of a `DiagnosticBuilder` go from this:
```
    err.span(span);
```
to this:
```
    err = err.span(span);
```
There are *many* such places. I have a high tolerance for tedious
refactorings, but even I gave up after a long time trying to convert
them all.

Instead, this commit has it both ways: the existing `&mut self -> Self`
chaining methods are kept, and new `self -> Self` chaining methods are
added, all of which have a `_mv` suffix (short for "move"). Changes to
the existing `forward!` macro lets this happen with very little
additional boilerplate code. I chose to add the suffix to the new
chaining methods rather than the existing ones, because the number of
changes required is much smaller that way.

This doubled chainging is a bit clumsy, but I think it is worthwhile
because it allows a *lot* of good things to subsequently happen. In this
commit, there are many `mut` qualifiers removed in places where
diagnostics are emitted without being modified. In subsequent commits:
- chaining can be used more, making the code more concise;
- more use of chaining also permits the removal of redundant diagnostic
  APIs like `struct_err_with_code`, which can be replaced easily with
  `struct_err` + `code_mv`;
- `emit_without_diagnostic` can be removed, which simplifies a lot of
  machinery, removing the need for `DiagnosticBuilderState`.
  • Loading branch information
nnethercote committed Jan 8, 2024
1 parent ca2fc42 commit b1b9278
Show file tree
Hide file tree
Showing 86 changed files with 329 additions and 312 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ macro_rules! gate {
($visitor:expr, $feature:ident, $span:expr, $explain:expr, $help:expr) => {{
if !$visitor.features.$feature && !$span.allows_unstable(sym::$feature) {
feature_err(&$visitor.sess.parse_sess, sym::$feature, $span, $explain)
.help($help)
.help_mv($help)
.emit();
}
}};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let named_ty = self.regioncx.name_regions(self.infcx.tcx, hidden_ty);
let named_key = self.regioncx.name_regions(self.infcx.tcx, key);
let named_region = self.regioncx.name_regions(self.infcx.tcx, member_region);
let mut diag = unexpected_hidden_region_diagnostic(
let diag = unexpected_hidden_region_diagnostic(
self.infcx.tcx,
span,
named_ty,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ fn check_opaque_type_parameter_valid(
return Err(tcx
.dcx()
.struct_span_err(span, "non-defining opaque type use in defining scope")
.span_note(spans, format!("{descr} used multiple times"))
.span_note_mv(spans, format!("{descr} used multiple times"))
.emit());
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_builtin_macros/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl
match expr_to_spanned_string(ecx, template_expr, msg) {
Ok(template_part) => template_part,
Err(err) => {
if let Some((mut err, _)) = err {
if let Some((err, _)) = err {
err.emit();
}
return None;
Expand Down Expand Up @@ -747,7 +747,7 @@ pub(super) fn expand_asm<'cx>(
};
MacEager::expr(expr)
}
Err(mut err) => {
Err(err) => {
err.emit();
DummyResult::any(sp)
}
Expand Down Expand Up @@ -779,7 +779,7 @@ pub(super) fn expand_global_asm<'cx>(
DummyResult::any(sp)
}
}
Err(mut err) => {
Err(err) => {
err.emit();
DummyResult::any(sp)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/assert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub fn expand_assert<'cx>(
) -> Box<dyn MacResult + 'cx> {
let Assert { cond_expr, custom_message } = match parse_assert(cx, span, tts) {
Ok(assert) => assert,
Err(mut err) => {
Err(err) => {
err.emit();
return DummyResult::any(span);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub fn expand_cfg(
);
MacEager::expr(cx.expr_bool(sp, matches_cfg))
}
Err(mut err) => {
Err(err) => {
err.emit();
DummyResult::any(sp)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/cfg_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl CfgEval<'_, '_> {
parser.capture_cfg = true;
match parse_annotatable_with(&mut parser) {
Ok(a) => annotatable = a,
Err(mut err) => {
Err(err) => {
err.emit();
return Some(annotatable);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/cmdline_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn inject(krate: &mut ast::Crate, parse_sess: &ParseSess, attrs: &[String])
let start_span = parser.token.span;
let AttrItem { path, args, tokens: _ } = match parser.parse_attr_item(false) {
Ok(ai) => ai,
Err(mut err) => {
Err(err) => {
err.emit();
continue;
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn parse_args<'a>(ecx: &mut ExtCtxt<'a>, sp: Span, tts: TokenStream) -> PResult<
}

match p.expect(&token::Comma) {
Err(mut err) => {
Err(err) => {
match token::TokenKind::Comma.similar_tokens() {
Some(tks) if tks.contains(&p.token.kind) => {
// If a similar token is found, then it may be a typo. We
Expand Down Expand Up @@ -630,8 +630,7 @@ fn report_missing_placeholders(
.collect::<Vec<_>>();

if !placeholders.is_empty() {
if let Some(mut new_diag) = report_redundant_format_arguments(ecx, args, used, placeholders)
{
if let Some(new_diag) = report_redundant_format_arguments(ecx, args, used, placeholders) {
diag.cancel();
new_diag.emit();
return;
Expand Down Expand Up @@ -976,7 +975,7 @@ fn expand_format_args_impl<'cx>(
MacEager::expr(DummyResult::raw_expr(sp, true))
}
}
Err(mut err) => {
Err(err) => {
err.emit();
DummyResult::any(sp)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/proc_macro_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {

self.dcx
.struct_span_err(attr.span, msg)
.span_label(prev_attr.span, "previous attribute here")
.span_label_mv(prev_attr.span, "previous attribute here")
.emit();

return;
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_builtin_macros/src/source_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub fn expand_include<'cx>(
// The file will be added to the code map by the parser
let file = match resolve_path(&cx.sess.parse_sess, file.as_str(), sp) {
Ok(f) => f,
Err(mut err) => {
Err(err) => {
err.emit();
return DummyResult::any(sp);
}
Expand Down Expand Up @@ -146,7 +146,7 @@ pub fn expand_include<'cx>(
let mut ret = SmallVec::new();
loop {
match self.p.parse_item(ForceCollect::No) {
Err(mut err) => {
Err(err) => {
err.emit();
break;
}
Expand Down Expand Up @@ -181,7 +181,7 @@ pub fn expand_include_str(
};
let file = match resolve_path(&cx.sess.parse_sess, file.as_str(), sp) {
Ok(f) => f,
Err(mut err) => {
Err(err) => {
err.emit();
return DummyResult::any(sp);
}
Expand Down Expand Up @@ -215,7 +215,7 @@ pub fn expand_include_bytes(
};
let file = match resolve_path(&cx.sess.parse_sess, file.as_str(), sp) {
Ok(f) => f,
Err(mut err) => {
Err(err) => {
err.emit();
return DummyResult::any(sp);
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,8 @@ fn not_testable_error(cx: &ExtCtxt<'_>, attr_sp: Span, item: Option<&ast::Item>)
),
);
}
err.span_label(attr_sp, "the `#[test]` macro causes a function to be run as a test and has no effect on non-functions")
.span_suggestion(attr_sp,
err.span_label_mv(attr_sp, "the `#[test]` macro causes a function to be run as a test and has no effect on non-functions")
.span_suggestion_mv(attr_sp,
"replace with conditional compilation to make the item only exist when tests are being run",
"#[cfg(test)]",
Applicability::MaybeIncorrect)
Expand Down Expand Up @@ -480,7 +480,7 @@ fn should_panic(cx: &ExtCtxt<'_>, i: &ast::Item) -> ShouldPanic {
"argument must be of the form: \
`expected = \"error message\"`",
)
.note(
.note_mv(
"errors in this attribute were erroneously \
allowed and will become a hard error in a \
future release",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/type_ascribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub fn expand_type_ascribe(
) -> Box<dyn base::MacResult + 'static> {
let (expr, ty) = match parse_ascribe(cx, tts) {
Ok(parsed) => parsed,
Err(mut err) => {
Err(err) => {
err.emit();
return DummyResult::any(span);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
InlineAttr::Never
} else {
struct_span_err!(tcx.dcx(), items[0].span(), E0535, "invalid argument")
.help("valid inline arguments are `always` and `never`")
.help_mv("valid inline arguments are `always` and `never`")
.emit();

InlineAttr::None
Expand Down Expand Up @@ -662,7 +662,7 @@ fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option<u16> {
let msg = format!("ordinal value in `link_ordinal` is too large: `{}`", &ordinal);
tcx.dcx()
.struct_span_err(attr.span, msg)
.note("the value may not exceed `u16::MAX`")
.note_mv("the value may not exceed `u16::MAX`")
.emit();
None
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub fn from_target_feature(
let code = "enable = \"..\"";
tcx.dcx()
.struct_span_err(span, msg)
.span_suggestion(span, "must be of the form", code, Applicability::HasPlaceholders)
.span_suggestion_mv(span, "must be of the form", code, Applicability::HasPlaceholders)
.emit();
};
let rust_features = tcx.features();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
return;
}

let mut err = op.build_error(self.ccx, span);
let err = op.build_error(self.ccx, span);
assert!(err.is_error());

match op.importance() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ fn print_crate_info(
let result = parse_crate_attrs(sess);
match result {
Ok(attrs) => Some(attrs),
Err(mut parse_error) => {
Err(parse_error) => {
parse_error.emit();
return Compilation::Stop;
}
Expand Down
Loading

0 comments on commit b1b9278

Please sign in to comment.