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`

Author: bors

src/abi/mod.rs

@@ -47,12 +47,12 @@ pub(crate) fn conv_to_call_conv(sess: &Session, c: Conv, default_call_conv: Call
         }
 
         Conv::X86Intr | Conv::RiscvInterrupt { .. } => {
-            sess.fatal(format!("interrupt call conv {c:?} not yet implemented"))
+            sess.dcx().fatal(format!("interrupt call conv {c:?} not yet implemented"))
         }
 
-        Conv::ArmAapcs => sess.fatal("aapcs call conv not yet implemented"),
+        Conv::ArmAapcs => sess.dcx().fatal("aapcs call conv not yet implemented"),
         Conv::CCmseNonSecureCall => {
-            sess.fatal("C-cmse-nonsecure-call call conv is not yet implemented");
+            sess.dcx().fatal("C-cmse-nonsecure-call call conv is not yet implemented");
         }
 
         Conv::Msp430Intr
@@ -88,10 +88,10 @@ pub(crate) fn import_function<'tcx>(
     let sig = get_function_sig(tcx, module.target_config().default_call_conv, inst);
     match module.declare_function(name, Linkage::Import, &sig) {
         Ok(func_id) => func_id,
-        Err(ModuleError::IncompatibleDeclaration(_)) => tcx.sess.fatal(format!(
+        Err(ModuleError::IncompatibleDeclaration(_)) => tcx.dcx().fatal(format!(
             "attempt to declare `{name}` as function, but it was already declared as static"
         )),
-        Err(ModuleError::IncompatibleSignature(_, prev_sig, new_sig)) => tcx.sess.fatal(format!(
+        Err(ModuleError::IncompatibleSignature(_, prev_sig, new_sig)) => tcx.dcx().fatal(format!(
             "attempt to declare `{name}` with signature {new_sig:?}, \
              but it was already declared with signature {prev_sig:?}"
         )),
@@ -181,7 +181,7 @@ fn make_local_place<'tcx>(
     is_ssa: bool,
 ) -> CPlace<'tcx> {
     if layout.is_unsized() {
-        fx.tcx.sess.span_fatal(
+        fx.tcx.dcx().span_fatal(
             fx.mir.local_decls[local].source_info.span,
             "unsized locals are not yet supported",
         );
@@ -226,7 +226,7 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
 
     // FIXME implement variadics in cranelift
     if fn_abi.c_variadic {
-        fx.tcx.sess.span_fatal(
+        fx.tcx.dcx().span_fatal(
             fx.mir.span,
             "Defining variadic functions is not yet supported by Cranelift",
         );
@@ -543,7 +543,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
         // FIXME find a cleaner way to support varargs
         if fn_sig.c_variadic() {
             if !matches!(fn_sig.abi(), Abi::C { .. }) {
-                fx.tcx.sess.span_fatal(
+                fx.tcx.dcx().span_fatal(
                     source_info.span,
                     format!("Variadic call for non-C abi {:?}", fn_sig.abi()),
                 );
@@ -555,7 +555,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
                     let ty = fx.bcx.func.dfg.value_type(arg);
                     if !ty.is_int() {
                         // FIXME set %al to upperbound on float args once floats are supported
-                        fx.tcx.sess.span_fatal(
+                        fx.tcx.dcx().span_fatal(
                             source_info.span,
                             format!("Non int ty {:?} for variadic call", ty),
                         );

src/base.rs

@@ -236,13 +236,13 @@ pub(crate) fn verify_func(
         match cranelift_codegen::verify_function(&func, &flags) {
             Ok(_) => {}
             Err(err) => {
-                tcx.sess.err(format!("{:?}", err));
+                tcx.dcx().err(format!("{:?}", err));
                 let pretty_error = cranelift_codegen::print_errors::pretty_verifier_error(
                     &func,
                     Some(Box::new(writer)),
                     err,
                 );
-                tcx.sess.fatal(format!("cranelift verify error:\n{}", pretty_error));
+                tcx.dcx().fatal(format!("cranelift verify error:\n{}", pretty_error));
             }
         }
     });
@@ -450,7 +450,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
                 unwind: _,
             } => {
                 if options.contains(InlineAsmOptions::MAY_UNWIND) {
-                    fx.tcx.sess.span_fatal(
+                    fx.tcx.dcx().span_fatal(
                         source_info.span,
                         "cranelift doesn't support unwinding from inline assembly.",
                     );
@@ -812,7 +812,7 @@ fn codegen_stmt<'tcx>(
         | StatementKind::PlaceMention(..)
         | StatementKind::AscribeUserType(..) => {}
 
-        StatementKind::Coverage { .. } => fx.tcx.sess.fatal("-Zcoverage is unimplemented"),
+        StatementKind::Coverage { .. } => fx.tcx.dcx().fatal("-Zcoverage is unimplemented"),
         StatementKind::Intrinsic(ref intrinsic) => match &**intrinsic {
             // We ignore `assume` intrinsics, they are only useful for optimizations
             NonDivergingIntrinsic::Assume(_) => {}

src/common.rs

@@ -465,9 +465,12 @@ impl<'tcx> LayoutOfHelpers<'tcx> for RevealAllLayoutCx<'tcx> {
     #[inline]
     fn handle_layout_err(&self, err: LayoutError<'tcx>, span: Span, ty: Ty<'tcx>) -> ! {
         if let LayoutError::SizeOverflow(_) | LayoutError::ReferencesError(_) = err {
-            self.0.sess.span_fatal(span, err.to_string())
+            self.0.sess.dcx().span_fatal(span, err.to_string())
         } else {
-            self.0.sess.span_fatal(span, format!("failed to get layout for `{}`: {}", ty, err))
+            self.0
+                .sess
+                .dcx()
+                .span_fatal(span, format!("failed to get layout for `{}`: {}", ty, err))
         }
     }
 }
@@ -483,7 +486,7 @@ impl<'tcx> FnAbiOfHelpers<'tcx> for RevealAllLayoutCx<'tcx> {
         fn_abi_request: FnAbiRequest<'tcx>,
     ) -> ! {
         if let FnAbiError::Layout(LayoutError::SizeOverflow(_)) = err {
-            self.0.sess.emit_fatal(Spanned { span, node: err })
+            self.0.sess.dcx().emit_fatal(Spanned { span, node: err })
         } else {
             match fn_abi_request {
                 FnAbiRequest::OfFnPtr { sig, extra_args } => {

src/constant.rs

@@ -263,7 +263,7 @@ fn data_id_for_static(
             attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL),
         ) {
             Ok(data_id) => data_id,
-            Err(ModuleError::IncompatibleDeclaration(_)) => tcx.sess.fatal(format!(
+            Err(ModuleError::IncompatibleDeclaration(_)) => tcx.dcx().fatal(format!(
                 "attempt to declare `{symbol_name}` as static, but it was already declared as function"
             )),
             Err(err) => Err::<_, _>(err).unwrap(),
@@ -311,7 +311,7 @@ fn data_id_for_static(
         attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL),
     ) {
         Ok(data_id) => data_id,
-        Err(ModuleError::IncompatibleDeclaration(_)) => tcx.sess.fatal(format!(
+        Err(ModuleError::IncompatibleDeclaration(_)) => tcx.dcx().fatal(format!(
             "attempt to declare `{symbol_name}` as static, but it was already declared as function"
         )),
         Err(err) => Err::<_, _>(err).unwrap(),
@@ -360,7 +360,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
                 if let Some(names) = section_name.split_once(',') {
                     names
                 } else {
-                    tcx.sess.fatal(format!(
+                    tcx.dcx().fatal(format!(
                         "#[link_section = \"{}\"] is not valid for macos target: must be segment and section separated by comma",
                         section_name
                     ));
@@ -406,7 +406,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
                 GlobalAlloc::Static(def_id) => {
                     if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::THREAD_LOCAL)
                     {
-                        tcx.sess.fatal(format!(
+                        tcx.dcx().fatal(format!(
                             "Allocation {:?} contains reference to TLS value {:?}",
                             alloc_id, def_id
                         ));

src/driver/aot.rs

@@ -69,7 +69,7 @@ impl OngoingCodegen {
 
             let module_codegen_result = match module_codegen_result {
                 Ok(module_codegen_result) => module_codegen_result,
-                Err(err) => sess.fatal(err),
+                Err(err) => sess.dcx().fatal(err),
             };
             let ModuleCodegenResult { module_regular, module_global_asm, existing_work_product } =
                 module_codegen_result;
@@ -108,7 +108,7 @@ impl OngoingCodegen {
 
         self.concurrency_limiter.finished();
 
-        sess.abort_if_errors();
+        sess.dcx().abort_if_errors();
 
         (
             CodegenResults {
@@ -422,7 +422,7 @@ pub(crate) fn run_aot(
                                     backend_config.clone(),
                                     global_asm_config.clone(),
                                     cgu.name(),
-                                    concurrency_limiter.acquire(tcx.sess.dcx()),
+                                    concurrency_limiter.acquire(tcx.dcx()),
                                 ),
                                 module_codegen,
                                 Some(rustc_middle::dep_graph::hash_result),
@@ -455,7 +455,7 @@ pub(crate) fn run_aot(
             "allocator_shim".to_owned(),
         ) {
             Ok(allocator_module) => Some(allocator_module),
-            Err(err) => tcx.sess.fatal(err),
+            Err(err) => tcx.dcx().fatal(err),
         }
     } else {
         None
@@ -478,7 +478,7 @@ pub(crate) fn run_aot(
             let obj = create_compressed_metadata_file(tcx.sess, &metadata, &symbol_name);
 
             if let Err(err) = std::fs::write(&tmp_file, obj) {
-                tcx.sess.fatal(format!("error writing metadata object file: {}", err));
+                tcx.dcx().fatal(format!("error writing metadata object file: {}", err));
             }
 
             (metadata_cgu_name, tmp_file)

src/driver/jit.rs

@@ -94,11 +94,11 @@ fn create_jit_module(
 
 pub(crate) fn run_jit(tcx: TyCtxt<'_>, backend_config: BackendConfig) -> ! {
     if !tcx.sess.opts.output_types.should_codegen() {
-        tcx.sess.fatal("JIT mode doesn't work with `cargo check`");
+        tcx.dcx().fatal("JIT mode doesn't work with `cargo check`");
     }
 
     if !tcx.crate_types().contains(&rustc_session::config::CrateType::Executable) {
-        tcx.sess.fatal("can't jit non-executable crate");
+        tcx.dcx().fatal("can't jit non-executable crate");
     }
 
     let (mut jit_module, mut cx) = create_jit_module(
@@ -141,17 +141,17 @@ pub(crate) fn run_jit(tcx: TyCtxt<'_>, backend_config: BackendConfig) -> ! {
                 }
                 MonoItem::GlobalAsm(item_id) => {
                     let item = tcx.hir().item(item_id);
-                    tcx.sess.span_fatal(item.span, "Global asm is not supported in JIT mode");
+                    tcx.dcx().span_fatal(item.span, "Global asm is not supported in JIT mode");
                 }
             }
         }
     });
 
     if !cx.global_asm.is_empty() {
-        tcx.sess.fatal("Inline asm is not supported in JIT mode");
+        tcx.dcx().fatal("Inline asm is not supported in JIT mode");
     }
 
-    tcx.sess.abort_if_errors();
+    tcx.dcx().abort_if_errors();
 
     jit_module.finalize_definitions().unwrap();
     unsafe { cx.unwind_context.register_jit(&jit_module) };
@@ -338,7 +338,7 @@ fn dep_symbol_lookup_fn(
             .collect::<Box<[_]>>(),
     );
 
-    sess.abort_if_errors();
+    sess.dcx().abort_if_errors();
 
     Box::new(move |sym_name| {
         for dylib in &*imported_dylibs {

src/global_asm.rs

@@ -47,7 +47,7 @@ pub(crate) fn codegen_global_asm_item(tcx: TyCtxt<'_>, global_asm: &mut String,
                         }
                         InlineAsmOperand::SymFn { anon_const } => {
                             if cfg!(not(feature = "inline_asm_sym")) {
-                                tcx.sess.span_err(
+                                tcx.dcx().span_err(
                                     item.span,
                                     "asm! and global_asm! sym operands are not yet supported",
                                 );
@@ -65,7 +65,7 @@ pub(crate) fn codegen_global_asm_item(tcx: TyCtxt<'_>, global_asm: &mut String,
                         }
                         InlineAsmOperand::SymStatic { path: _, def_id } => {
                             if cfg!(not(feature = "inline_asm_sym")) {
-                                tcx.sess.span_err(
+                                tcx.dcx().span_err(
                                     item.span,
                                     "asm! and global_asm! sym operands are not yet supported",
                                 );

src/inline_asm.rs

@@ -84,7 +84,7 @@ pub(crate) fn codegen_inline_asm_terminator<'tcx>(
             InlineAsmOperand::SymFn { ref value } => {
                 if cfg!(not(feature = "inline_asm_sym")) {
                     fx.tcx
-                        .sess
+                        .dcx()
                         .span_err(span, "asm! and global_asm! sym operands are not yet supported");
                 }
 
@@ -455,7 +455,7 @@ impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> {
             }
             _ => self
                 .tcx
-                .sess
+                .dcx()
                 .fatal(format!("Unsupported binary format for inline asm: {binary_format:?}")),
         }
 
@@ -563,7 +563,7 @@ impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> {
             BinaryFormat::Macho | BinaryFormat::Coff => {}
             _ => self
                 .tcx
-                .sess
+                .dcx()
                 .fatal(format!("Unsupported binary format for inline asm: {binary_format:?}")),
         }
 

src/intrinsics/llvm.rs

@@ -68,7 +68,7 @@ pub(crate) fn codegen_llvm_intrinsic_call<'tcx>(
 
         _ => {
             fx.tcx
-                .sess
+                .dcx()
                 .warn(format!("unsupported llvm intrinsic {}; replacing with trap", intrinsic));
             crate::trap::trap_unimplemented(fx, intrinsic);
             return;

src/intrinsics/llvm_aarch64.rs

@@ -309,7 +309,7 @@ pub(crate) fn codegen_aarch64_llvm_intrinsic_call<'tcx>(
         }
         */
         _ => {
-            fx.tcx.sess.warn(format!(
+            fx.tcx.dcx().warn(format!(
                 "unsupported AArch64 llvm intrinsic {}; replacing with trap",
                 intrinsic
             ));

src/intrinsics/llvm_x86.rs

@@ -960,7 +960,9 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
             {
                 imm8
             } else {
-                fx.tcx.sess.span_fatal(span, "Index argument for `_mm_cmpestri` is not a constant");
+                fx.tcx
+                    .dcx()
+                    .span_fatal(span, "Index argument for `_mm_cmpestri` is not a constant");
             };
 
             let imm8 = imm8.try_to_u8().unwrap_or_else(|_| panic!("kind not scalar: {:?}", imm8));
@@ -1011,7 +1013,9 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
             {
                 imm8
             } else {
-                fx.tcx.sess.span_fatal(span, "Index argument for `_mm_cmpestrm` is not a constant");
+                fx.tcx
+                    .dcx()
+                    .span_fatal(span, "Index argument for `_mm_cmpestrm` is not a constant");
             };
 
             let imm8 = imm8.try_to_u8().unwrap_or_else(|_| panic!("kind not scalar: {:?}", imm8));
@@ -1056,7 +1060,7 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
             {
                 imm8
             } else {
-                fx.tcx.sess.span_fatal(
+                fx.tcx.dcx().span_fatal(
                     span,
                     "Index argument for `_mm_clmulepi64_si128` is not a constant",
                 );
@@ -1093,7 +1097,7 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
             {
                 imm8
             } else {
-                fx.tcx.sess.span_fatal(
+                fx.tcx.dcx().span_fatal(
                     span,
                     "Index argument for `_mm_aeskeygenassist_si128` is not a constant",
                 );
@@ -1361,7 +1365,7 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
 
         _ => {
             fx.tcx
-                .sess
+                .dcx()
                 .warn(format!("unsupported x86 llvm intrinsic {}; replacing with trap", intrinsic));
             crate::trap::trap_unimplemented(fx, intrinsic);
             return;