From 2ce89ee1c420798aca7ed17359f3363e86799719 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 9 Dec 2024 14:45:16 +0000 Subject: [PATCH 1/9] Add a test for mangling of named constants in const generics and array length --- tests/ui/symbol-names/types.legacy.stderr | 20 ++++++++++++++++++- tests/ui/symbol-names/types.rs | 11 ++++++++++ tests/ui/symbol-names/types.v0.stderr | 20 ++++++++++++++++++- .../symbol-names/types.verbose-legacy.stderr | 20 ++++++++++++++++++- 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/tests/ui/symbol-names/types.legacy.stderr b/tests/ui/symbol-names/types.legacy.stderr index 87c3acae0bd6c..8ccf5317fdd14 100644 --- a/tests/ui/symbol-names/types.legacy.stderr +++ b/tests/ui/symbol-names/types.legacy.stderr @@ -502,5 +502,23 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 84 previous errors +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling(a::b::Type<[u8; _]>::h[HASH]) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling-alt(a::b::Type<[u8; _]>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 87 previous errors diff --git a/tests/ui/symbol-names/types.rs b/tests/ui/symbol-names/types.rs index 7ed19e0e5a825..bedfcd80a04a8 100644 --- a/tests/ui/symbol-names/types.rs +++ b/tests/ui/symbol-names/types.rs @@ -266,6 +266,17 @@ pub fn b() { //[v0]~| ERROR ::b::Type<[_; _]>>) //[v0]~| ERROR demangling-alt(>) impl Type<[T; N]> {} + + const ZERO: usize = 0; + + #[rustc_symbol_name] + //[legacy,verbose-legacy]~^ ERROR symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$ + //[legacy,verbose-legacy]~| ERROR demangling(a::b::Type<[u8; _]>:: + //[legacy,verbose-legacy]~| ERROR demangling-alt(a::b::Type<[u8; _]>) + //[v0]~^^^^ ERROR symbol-name(_RMsq_NvCsCRATE_HASH_1a1bINtB_4TypeAhj0_E) + //[v0]~| ERROR ::b::Type<[u8; 0usize]>>) + //[v0]~| ERROR demangling-alt(>) + impl Type<[u8; ZERO]> {} } fn main() {} diff --git a/tests/ui/symbol-names/types.v0.stderr b/tests/ui/symbol-names/types.v0.stderr index 58680e002022a..90012a2dcf72f 100644 --- a/tests/ui/symbol-names/types.v0.stderr +++ b/tests/ui/symbol-names/types.v0.stderr @@ -502,5 +502,23 @@ error: demangling-alt(>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 84 previous errors +error: symbol-name(_RMsq_NvCsCRATE_HASH_1a1bINtB_4TypeAhj0_E) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling(>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling-alt(>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 87 previous errors diff --git a/tests/ui/symbol-names/types.verbose-legacy.stderr b/tests/ui/symbol-names/types.verbose-legacy.stderr index 87c3acae0bd6c..8ccf5317fdd14 100644 --- a/tests/ui/symbol-names/types.verbose-legacy.stderr +++ b/tests/ui/symbol-names/types.verbose-legacy.stderr @@ -502,5 +502,23 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 84 previous errors +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling(a::b::Type<[u8; _]>::h[HASH]) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling-alt(a::b::Type<[u8; _]>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 87 previous errors From 9ecdc54d82fe1e797103f8ac750baf9f8f861bec Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 9 Dec 2024 14:45:16 +0000 Subject: [PATCH 2/9] Try to evaluate constants in legacy mangling --- compiler/rustc_symbol_mangling/src/legacy.rs | 29 ++++++++++++++++++- tests/ui/symbol-names/types.legacy.stderr | 6 ++-- tests/ui/symbol-names/types.rs | 6 ++-- .../symbol-names/types.verbose-legacy.stderr | 6 ++-- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_symbol_mangling/src/legacy.rs b/compiler/rustc_symbol_mangling/src/legacy.rs index 59ccd6dff8588..0d6d8488a23ca 100644 --- a/compiler/rustc_symbol_mangling/src/legacy.rs +++ b/compiler/rustc_symbol_mangling/src/legacy.rs @@ -2,7 +2,7 @@ use std::fmt::{self, Write}; use std::mem::{self, discriminant}; use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher}; -use rustc_hir::def_id::CrateNum; +use rustc_hir::def_id::{CrateNum, DefId}; use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData}; use rustc_middle::bug; use rustc_middle::ty::print::{PrettyPrinter, Print, PrintError, Printer}; @@ -378,6 +378,33 @@ impl<'tcx> Printer<'tcx> for SymbolPrinter<'tcx> { Ok(()) } } + + fn print_impl_path( + &mut self, + impl_def_id: DefId, + args: &'tcx [GenericArg<'tcx>], + mut self_ty: Ty<'tcx>, + mut impl_trait_ref: Option>, + ) -> Result<(), PrintError> { + let mut typing_env = ty::TypingEnv::post_analysis(self.tcx, impl_def_id); + if !args.is_empty() { + typing_env.param_env = + ty::EarlyBinder::bind(typing_env.param_env).instantiate(self.tcx, args); + } + + match &mut impl_trait_ref { + Some(impl_trait_ref) => { + assert_eq!(impl_trait_ref.self_ty(), self_ty); + *impl_trait_ref = self.tcx.normalize_erasing_regions(typing_env, *impl_trait_ref); + self_ty = impl_trait_ref.self_ty(); + } + None => { + self_ty = self.tcx.normalize_erasing_regions(typing_env, self_ty); + } + } + + self.default_print_impl_path(impl_def_id, args, self_ty, impl_trait_ref) + } } impl<'tcx> PrettyPrinter<'tcx> for SymbolPrinter<'tcx> { diff --git a/tests/ui/symbol-names/types.legacy.stderr b/tests/ui/symbol-names/types.legacy.stderr index 8ccf5317fdd14..c368b31860989 100644 --- a/tests/ui/symbol-names/types.legacy.stderr +++ b/tests/ui/symbol-names/types.legacy.stderr @@ -502,19 +502,19 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$0$u5d$$GT$17h[HASH]E) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling(a::b::Type<[u8; _]>::h[HASH]) +error: demangling(a::b::Type<[u8; 0]>::h[HASH]) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling-alt(a::b::Type<[u8; _]>) +error: demangling-alt(a::b::Type<[u8; 0]>) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] diff --git a/tests/ui/symbol-names/types.rs b/tests/ui/symbol-names/types.rs index bedfcd80a04a8..38735e1aa5098 100644 --- a/tests/ui/symbol-names/types.rs +++ b/tests/ui/symbol-names/types.rs @@ -270,9 +270,9 @@ pub fn b() { const ZERO: usize = 0; #[rustc_symbol_name] - //[legacy,verbose-legacy]~^ ERROR symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$ - //[legacy,verbose-legacy]~| ERROR demangling(a::b::Type<[u8; _]>:: - //[legacy,verbose-legacy]~| ERROR demangling-alt(a::b::Type<[u8; _]>) + //[legacy,verbose-legacy]~^ ERROR symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$0$u5d$$GT$ + //[legacy,verbose-legacy]~| ERROR demangling(a::b::Type<[u8; 0]>:: + //[legacy,verbose-legacy]~| ERROR demangling-alt(a::b::Type<[u8; 0]>) //[v0]~^^^^ ERROR symbol-name(_RMsq_NvCsCRATE_HASH_1a1bINtB_4TypeAhj0_E) //[v0]~| ERROR ::b::Type<[u8; 0usize]>>) //[v0]~| ERROR demangling-alt(>) diff --git a/tests/ui/symbol-names/types.verbose-legacy.stderr b/tests/ui/symbol-names/types.verbose-legacy.stderr index 8ccf5317fdd14..c368b31860989 100644 --- a/tests/ui/symbol-names/types.verbose-legacy.stderr +++ b/tests/ui/symbol-names/types.verbose-legacy.stderr @@ -502,19 +502,19 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$0$u5d$$GT$17h[HASH]E) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling(a::b::Type<[u8; _]>::h[HASH]) +error: demangling(a::b::Type<[u8; 0]>::h[HASH]) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling-alt(a::b::Type<[u8; _]>) +error: demangling-alt(a::b::Type<[u8; 0]>) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] From de53fe245d7f937640bef68a4eb3622b43c39674 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 12 Dec 2024 18:33:33 +1100 Subject: [PATCH 3/9] coverage: Tidy up creation of covmap records --- .../src/coverageinfo/mapgen.rs | 60 +++++++++---------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index a573a37beb3fa..7c7903ce84290 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -75,9 +75,6 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // Encode all filenames referenced by coverage mappings in this CGU. let filenames_buffer = global_file_table.make_filenames_buffer(tcx); - - let filenames_size = filenames_buffer.len(); - let filenames_val = cx.const_bytes(&filenames_buffer); let filenames_ref = llvm_cov::hash_bytes(&filenames_buffer); let mut unused_function_names = Vec::new(); @@ -126,7 +123,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // Generate the coverage map header, which contains the filenames used by // this CGU's coverage mappings, and store it in a well-known global. // (This is skipped if we returned early due to having no covfun records.) - generate_covmap_record(cx, covmap_version, filenames_size, filenames_val); + generate_covmap_record(cx, covmap_version, &filenames_buffer); } /// Maps "global" (per-CGU) file ID numbers to their underlying filenames. @@ -225,38 +222,35 @@ fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol { /// Generates the contents of the covmap record for this CGU, which mostly /// consists of a header and a list of filenames. The record is then stored /// as a global variable in the `__llvm_covmap` section. -fn generate_covmap_record<'ll>( - cx: &CodegenCx<'ll, '_>, - version: u32, - filenames_size: usize, - filenames_val: &'ll llvm::Value, -) { - debug!("cov map: filenames_size = {}, 0-based version = {}", filenames_size, version); - - // Create the coverage data header (Note, fields 0 and 2 are now always zero, - // as of `llvm::coverage::CovMapVersion::Version4`.) - let zero_was_n_records_val = cx.const_u32(0); - let filenames_size_val = cx.const_u32(filenames_size as u32); - let zero_was_coverage_size_val = cx.const_u32(0); - let version_val = cx.const_u32(version); - let cov_data_header_val = cx.const_struct( - &[zero_was_n_records_val, filenames_size_val, zero_was_coverage_size_val, version_val], - /*packed=*/ false, +fn generate_covmap_record<'ll>(cx: &CodegenCx<'ll, '_>, version: u32, filenames_buffer: &[u8]) { + // A covmap record consists of four target-endian u32 values, followed by + // the encoded filenames table. Two of the header fields are unused in + // modern versions of the LLVM coverage mapping format, and are always 0. + // + // See also `src/llvm-project/clang/lib/CodeGen/CoverageMappingGen.cpp`. + let covmap_header = cx.const_struct( + &[ + cx.const_u32(0), // (unused) + cx.const_u32(filenames_buffer.len() as u32), + cx.const_u32(0), // (unused) + cx.const_u32(version), + ], + /* packed */ false, ); - - // Create the complete LLVM coverage data value to add to the LLVM IR - let covmap_data = - cx.const_struct(&[cov_data_header_val, filenames_val], /*packed=*/ false); - - let llglobal = llvm::add_global(cx.llmod, cx.val_ty(covmap_data), &llvm_cov::covmap_var_name()); - llvm::set_initializer(llglobal, covmap_data); - llvm::set_global_constant(llglobal, true); - llvm::set_linkage(llglobal, llvm::Linkage::PrivateLinkage); - llvm::set_section(llglobal, &llvm_cov::covmap_section_name(cx.llmod)); + let covmap_record = cx + .const_struct(&[covmap_header, cx.const_bytes(filenames_buffer)], /* packed */ false); + + let covmap_global = + llvm::add_global(cx.llmod, cx.val_ty(covmap_record), &llvm_cov::covmap_var_name()); + llvm::set_initializer(covmap_global, covmap_record); + llvm::set_global_constant(covmap_global, true); + llvm::set_linkage(covmap_global, llvm::Linkage::PrivateLinkage); + llvm::set_section(covmap_global, &llvm_cov::covmap_section_name(cx.llmod)); // LLVM's coverage mapping format specifies 8-byte alignment for items in this section. // - llvm::set_alignment(llglobal, Align::EIGHT); - cx.add_used_global(llglobal); + llvm::set_alignment(covmap_global, Align::EIGHT); + + cx.add_used_global(covmap_global); } /// Each CGU will normally only emit coverage metadata for the functions that it actually generates. From 5f5745beb06cac3f0d0cabf1d8edd1ffa53a9b55 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 12 Dec 2024 20:28:10 +1100 Subject: [PATCH 4/9] coverage: Tidy up creation of covfun records --- .../src/coverageinfo/mapgen.rs | 7 ++- .../src/coverageinfo/mapgen/covfun.rs | 59 +++++++++---------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 7c7903ce84290..4f2af73252751 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -75,7 +75,10 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // Encode all filenames referenced by coverage mappings in this CGU. let filenames_buffer = global_file_table.make_filenames_buffer(tcx); - let filenames_ref = llvm_cov::hash_bytes(&filenames_buffer); + // The `llvm-cov` tool uses this hash to associate each covfun record with + // its corresponding filenames table, since the final binary will typically + // contain multiple covmap records from different compilation units. + let filenames_hash = llvm_cov::hash_bytes(&filenames_buffer); let mut unused_function_names = Vec::new(); @@ -98,7 +101,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { for covfun in &covfun_records { unused_function_names.extend(covfun.mangled_function_name_if_unused()); - covfun::generate_covfun_record(cx, filenames_ref, covfun) + covfun::generate_covfun_record(cx, filenames_hash, covfun) } // For unused functions, we need to take their mangled names and store them diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 530e6827f55d3..33e7a0f2f201b 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -136,7 +136,7 @@ fn fill_region_tables<'tcx>( /// as a global variable in the `__llvm_covfun` section. pub(crate) fn generate_covfun_record<'tcx>( cx: &CodegenCx<'_, 'tcx>, - filenames_ref: u64, + filenames_hash: u64, covfun: &CovfunRecord<'tcx>, ) { let &CovfunRecord { @@ -155,46 +155,45 @@ pub(crate) fn generate_covfun_record<'tcx>( regions, ); - // Concatenate the encoded coverage mappings - let coverage_mapping_size = coverage_mapping_buffer.len(); - let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer); - + // A covfun record consists of four target-endian integers, followed by the + // encoded mapping data in bytes. Note that the length field is 32 bits. + // + // See also `src/llvm-project/clang/lib/CodeGen/CoverageMappingGen.cpp` and + // `COVMAP_V3` in `src/llvm-project/llvm/include/llvm/ProfileData/InstrProfData.inc`. let func_name_hash = llvm_cov::hash_bytes(mangled_function_name.as_bytes()); - let func_name_hash_val = cx.const_u64(func_name_hash); - let coverage_mapping_size_val = cx.const_u32(coverage_mapping_size as u32); - let source_hash_val = cx.const_u64(source_hash); - let filenames_ref_val = cx.const_u64(filenames_ref); - let func_record_val = cx.const_struct( + let covfun_record = cx.const_struct( &[ - func_name_hash_val, - coverage_mapping_size_val, - source_hash_val, - filenames_ref_val, - coverage_mapping_val, + cx.const_u64(func_name_hash), + cx.const_u32(coverage_mapping_buffer.len() as u32), + cx.const_u64(source_hash), + cx.const_u64(filenames_hash), + cx.const_bytes(&coverage_mapping_buffer), ], - /*packed=*/ true, + // This struct needs to be packed, so that the 32-bit length field + // doesn't have unexpected padding. + true, ); // Choose a variable name to hold this function's covfun data. // Functions that are used have a suffix ("u") to distinguish them from // unused copies of the same function (from different CGUs), so that if a // linker sees both it won't discard the used copy's data. - let func_record_var_name = - CString::new(format!("__covrec_{:X}{}", func_name_hash, if is_used { "u" } else { "" })) - .unwrap(); - debug!("function record var name: {:?}", func_record_var_name); - - let llglobal = llvm::add_global(cx.llmod, cx.val_ty(func_record_val), &func_record_var_name); - llvm::set_initializer(llglobal, func_record_val); - llvm::set_global_constant(llglobal, true); - llvm::set_linkage(llglobal, llvm::Linkage::LinkOnceODRLinkage); - llvm::set_visibility(llglobal, llvm::Visibility::Hidden); - llvm::set_section(llglobal, cx.covfun_section_name()); + let u = if is_used { "u" } else { "" }; + let covfun_var_name = CString::new(format!("__covrec_{func_name_hash:X}{u}")).unwrap(); + debug!("function record var name: {covfun_var_name:?}"); + + let covfun_global = llvm::add_global(cx.llmod, cx.val_ty(covfun_record), &covfun_var_name); + llvm::set_initializer(covfun_global, covfun_record); + llvm::set_global_constant(covfun_global, true); + llvm::set_linkage(covfun_global, llvm::Linkage::LinkOnceODRLinkage); + llvm::set_visibility(covfun_global, llvm::Visibility::Hidden); + llvm::set_section(covfun_global, cx.covfun_section_name()); // LLVM's coverage mapping format specifies 8-byte alignment for items in this section. // - llvm::set_alignment(llglobal, Align::EIGHT); + llvm::set_alignment(covfun_global, Align::EIGHT); if cx.target_spec().supports_comdat() { - llvm::set_comdat(cx.llmod, llglobal, &func_record_var_name); + llvm::set_comdat(cx.llmod, covfun_global, &covfun_var_name); } - cx.add_used_global(llglobal); + + cx.add_used_global(covfun_global); } From 2a6a7bef01c6b8fe16cd2ada1c7e7133b7dc596c Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 24 Dec 2023 19:49:23 -0500 Subject: [PATCH 5/9] don't show the full linker args unless `--verbose` is passed the linker arguments can be *very* long, especially for crates with many dependencies. some parts of them are not very useful. unless specifically requested: - omit object files specific to the current invocation - fold rlib files into a single braced argument (in shell expansion format) this shortens the output significantly without removing too much information. --- compiler/rustc_codegen_ssa/src/back/link.rs | 4 +- compiler/rustc_codegen_ssa/src/errors.rs | 70 +++++++++++++++++-- .../src/external_deps/rustc.rs | 6 ++ tests/run-make/link-args-order/rmake.rs | 6 +- tests/run-make/link-dedup/rmake.rs | 12 ++-- tests/run-make/linker-warning/fake-linker.rs | 13 ++++ tests/run-make/linker-warning/main.rs | 1 + tests/run-make/linker-warning/rmake.rs | 28 ++++++++ 8 files changed, 126 insertions(+), 14 deletions(-) create mode 100644 tests/run-make/linker-warning/fake-linker.rs create mode 100644 tests/run-make/linker-warning/main.rs create mode 100644 tests/run-make/linker-warning/rmake.rs diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 35d18d0206dbb..b030ea3f6df83 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -992,12 +992,12 @@ fn link_natively( let mut output = prog.stderr.clone(); output.extend_from_slice(&prog.stdout); let escaped_output = escape_linker_output(&output, flavor); - // FIXME: Add UI tests for this error. let err = errors::LinkingFailed { linker_path: &linker_path, exit_status: prog.status, - command: &cmd, + command: cmd, escaped_output, + verbose: sess.opts.verbose, }; sess.dcx().emit_err(err); // If MSVC's `link.exe` was expected but the return code diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 00f8654e670b1..c7213bbc801f9 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -1,6 +1,7 @@ //! Errors emitted by codegen_ssa use std::borrow::Cow; +use std::ffi::OsString; use std::io::Error; use std::num::ParseIntError; use std::path::{Path, PathBuf}; @@ -345,21 +346,82 @@ impl Diagnostic<'_, G> for ThorinErrorWrapper { } pub(crate) struct LinkingFailed<'a> { - pub linker_path: &'a PathBuf, + pub linker_path: &'a Path, pub exit_status: ExitStatus, - pub command: &'a Command, + pub command: Command, pub escaped_output: String, + pub verbose: bool, } impl Diagnostic<'_, G> for LinkingFailed<'_> { - fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> { + fn into_diag(mut self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> { let mut diag = Diag::new(dcx, level, fluent::codegen_ssa_linking_failed); diag.arg("linker_path", format!("{}", self.linker_path.display())); diag.arg("exit_status", format!("{}", self.exit_status)); let contains_undefined_ref = self.escaped_output.contains("undefined reference to"); - diag.note(format!("{:?}", self.command)).note(self.escaped_output); + if self.verbose { + diag.note(format!("{:?}", self.command)); + } else { + enum ArgGroup { + Regular(OsString), + Objects(usize), + Rlibs(PathBuf, Vec), + } + + // Omit rust object files and fold rlibs in the error by default to make linker errors a + // bit less verbose. + let orig_args = self.command.take_args(); + let mut args: Vec = vec![]; + for arg in orig_args { + if arg.as_encoded_bytes().ends_with(b".rcgu.o") { + if let Some(ArgGroup::Objects(n)) = args.last_mut() { + *n += 1; + } else { + args.push(ArgGroup::Objects(1)); + } + } else if arg.as_encoded_bytes().ends_with(b".rlib") { + let rlib_path = Path::new(&arg); + let dir = rlib_path.parent().unwrap(); + let filename = rlib_path.file_name().unwrap().to_owned(); + if let Some(ArgGroup::Rlibs(parent, rlibs)) = args.last_mut() { + if parent == dir { + rlibs.push(filename); + } else { + args.push(ArgGroup::Rlibs(dir.to_owned(), vec![filename])); + } + } else { + args.push(ArgGroup::Rlibs(dir.to_owned(), vec![filename])); + } + } else { + args.push(ArgGroup::Regular(arg)); + } + } + self.command.args(args.into_iter().map(|arg_group| match arg_group { + ArgGroup::Regular(arg) => arg, + ArgGroup::Objects(n) => OsString::from(format!("<{n} object files omitted>")), + ArgGroup::Rlibs(dir, rlibs) => { + let mut arg = dir.into_os_string(); + arg.push("/{"); + let mut first = true; + for rlib in rlibs { + if !first { + arg.push(","); + } + first = false; + arg.push(rlib); + } + arg.push("}"); + arg + } + })); + + diag.note(format!("{:?}", self.command)); + diag.note("some arguments are omitted. use `--verbose` to show all linker arguments"); + } + + diag.note(self.escaped_output); // Trying to match an error from OS linkers // which by now we have no way to translate. diff --git a/src/tools/run-make-support/src/external_deps/rustc.rs b/src/tools/run-make-support/src/external_deps/rustc.rs index ffe10092cc28a..8894ea7fb209b 100644 --- a/src/tools/run-make-support/src/external_deps/rustc.rs +++ b/src/tools/run-make-support/src/external_deps/rustc.rs @@ -325,6 +325,12 @@ impl Rustc { self } + /// Pass the `--verbose` flag. + pub fn verbose(&mut self) -> &mut Self { + self.cmd.arg("--verbose"); + self + } + /// `EXTRARSCXXFLAGS` pub fn extra_rs_cxx_flags(&mut self) -> &mut Self { // Adapted from tools.mk (trimmed): diff --git a/tests/run-make/link-args-order/rmake.rs b/tests/run-make/link-args-order/rmake.rs index b7ef8333267f2..fe0d02926eff4 100644 --- a/tests/run-make/link-args-order/rmake.rs +++ b/tests/run-make/link-args-order/rmake.rs @@ -15,8 +15,9 @@ fn main() { .link_args("b c") .link_args("d e") .link_arg("f") + .arg("--print=link-args") .run_fail() - .assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#); + .assert_stdout_contains(r#""a" "b" "c" "d" "e" "f""#); rustc() .input("empty.rs") .linker_flavor(linker) @@ -24,6 +25,7 @@ fn main() { .arg("-Zpre-link-args=b c") .arg("-Zpre-link-args=d e") .arg("-Zpre-link-arg=f") + .arg("--print=link-args") .run_fail() - .assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#); + .assert_stdout_contains(r#""a" "b" "c" "d" "e" "f""#); } diff --git a/tests/run-make/link-dedup/rmake.rs b/tests/run-make/link-dedup/rmake.rs index 6075f31095424..f38603dee8cbb 100644 --- a/tests/run-make/link-dedup/rmake.rs +++ b/tests/run-make/link-dedup/rmake.rs @@ -14,13 +14,13 @@ fn main() { rustc().input("depb.rs").run(); rustc().input("depc.rs").run(); - let output = rustc().input("empty.rs").cfg("bar").run_fail(); - output.assert_stderr_contains(needle_from_libs(&["testa", "testb", "testa"])); + let output = rustc().input("empty.rs").cfg("bar").arg("--print=link-args").run_fail(); + output.assert_stdout_contains(needle_from_libs(&["testa", "testb", "testa"])); - let output = rustc().input("empty.rs").run_fail(); - output.assert_stderr_contains(needle_from_libs(&["testa"])); - output.assert_stderr_not_contains(needle_from_libs(&["testb"])); - output.assert_stderr_not_contains(needle_from_libs(&["testa", "testa", "testa"])); + let output = rustc().input("empty.rs").arg("--print=link-args").run_fail(); + output.assert_stdout_contains(needle_from_libs(&["testa"])); + output.assert_stdout_not_contains(needle_from_libs(&["testb"])); + output.assert_stdout_not_contains(needle_from_libs(&["testa", "testa", "testa"])); // Adjacent identical native libraries are no longer deduplicated if // they come from different crates (https://github.com/rust-lang/rust/pull/103311) // so the following will fail: diff --git a/tests/run-make/linker-warning/fake-linker.rs b/tests/run-make/linker-warning/fake-linker.rs new file mode 100644 index 0000000000000..30497eea2ccd9 --- /dev/null +++ b/tests/run-make/linker-warning/fake-linker.rs @@ -0,0 +1,13 @@ +fn main() { + for arg in std::env::args() { + match &*arg { + "run_make_info" => println!("foo"), + "run_make_warn" => eprintln!("warning: bar"), + "run_make_error" => { + eprintln!("error: baz"); + std::process::exit(1); + } + _ => (), + } + } +} diff --git a/tests/run-make/linker-warning/main.rs b/tests/run-make/linker-warning/main.rs new file mode 100644 index 0000000000000..f328e4d9d04c3 --- /dev/null +++ b/tests/run-make/linker-warning/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/run-make/linker-warning/rmake.rs b/tests/run-make/linker-warning/rmake.rs new file mode 100644 index 0000000000000..4a916b534ef4e --- /dev/null +++ b/tests/run-make/linker-warning/rmake.rs @@ -0,0 +1,28 @@ +use run_make_support::{Rustc, rustc}; + +fn run_rustc() -> Rustc { + let mut rustc = rustc(); + rustc.arg("main.rs").output("main").linker("./fake-linker"); + rustc +} + +fn main() { + // first, compile our linker + rustc().arg("fake-linker.rs").output("fake-linker").run(); + + // Make sure we don't show the linker args unless `--verbose` is passed + run_rustc() + .link_arg("run_make_error") + .verbose() + .run_fail() + .assert_stderr_contains_regex("fake-linker.*run_make_error") + .assert_stderr_not_contains("object files omitted") + .assert_stderr_contains_regex(r"lib[/\\]libstd"); + run_rustc() + .link_arg("run_make_error") + .run_fail() + .assert_stderr_contains("fake-linker") + .assert_stderr_contains("object files omitted") + .assert_stderr_contains_regex(r"\{") + .assert_stderr_not_contains_regex(r"lib[/\\]libstd"); +} From 37bb774219659d5fc7f72c83ca2481eea87ff3dd Mon Sep 17 00:00:00 2001 From: Florian Bartels Date: Thu, 12 Dec 2024 14:03:25 +0100 Subject: [PATCH 6/9] Reduce the need to set archiver via environment variables --- src/bootstrap/src/utils/cc_detect.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index e8d5b60948aa8..10611490ce35b 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -44,6 +44,16 @@ fn cc2ar(cc: &Path, target: TargetSelection) -> Option { Some(PathBuf::from("ar")) } else if target.contains("vxworks") { Some(PathBuf::from("wr-ar")) + } else if target.contains("-nto-") { + if target.starts_with("i586") { + Some(PathBuf::from("ntox86-ar")) + } else if target.starts_with("aarch64") { + Some(PathBuf::from("ntoaarch64-ar")) + } else if target.starts_with("x86_64") { + Some(PathBuf::from("ntox86_64-ar")) + } else { + panic!("Unknown architecture, cannot determine archiver for Neutrino QNX"); + } } else if target.contains("android") || target.contains("-wasi") { Some(cc.parent().unwrap().join(PathBuf::from("llvm-ar"))) } else { From 3a90c4751b77ebe2cc077a3774c3561aaf1f30e2 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sat, 26 Oct 2024 04:57:49 +0900 Subject: [PATCH 7/9] Fix powerpc64 big-endian FreeBSD ABI --- compiler/rustc_target/src/callconv/powerpc64.rs | 2 +- .../rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs | 2 +- src/doc/rustc/src/platform-support.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_target/src/callconv/powerpc64.rs b/compiler/rustc_target/src/callconv/powerpc64.rs index 71e533b8cc519..3a71592cbe090 100644 --- a/compiler/rustc_target/src/callconv/powerpc64.rs +++ b/compiler/rustc_target/src/callconv/powerpc64.rs @@ -99,7 +99,7 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout + HasTargetSpec, { - let abi = if cx.target_spec().env == "musl" { + let abi = if cx.target_spec().env == "musl" || cx.target_spec().os == "freebsd" { ELFv2 } else if cx.target_spec().os == "aix" { AIX diff --git a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs index 68a3718035cd9..4ccb3ee466405 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs @@ -11,7 +11,7 @@ pub(crate) fn target() -> Target { Target { llvm_target: "powerpc64-unknown-freebsd".into(), metadata: crate::spec::TargetMetadata { - description: Some("PPC64 FreeBSD (ELFv1 and ELFv2)".into()), + description: Some("PPC64 FreeBSD (ELFv2)".into()), tier: Some(3), host_tools: Some(true), std: Some(true), diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index 3b82d123276d9..3dbcfe9703667 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -343,7 +343,7 @@ target | std | host | notes [`powerpc-unknown-openbsd`](platform-support/powerpc-unknown-openbsd.md) | * | | [`powerpc-wrs-vxworks-spe`](platform-support/vxworks.md) | ✓ | | [`powerpc-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | -`powerpc64-unknown-freebsd` | ✓ | ✓ | PPC64 FreeBSD (ELFv1 and ELFv2) +`powerpc64-unknown-freebsd` | ✓ | ✓ | PPC64 FreeBSD (ELFv2) `powerpc64le-unknown-freebsd` | ✓ | ✓ | PPC64LE FreeBSD `powerpc-unknown-freebsd` | ? | | PowerPC FreeBSD `powerpc64-unknown-linux-musl` | ? | | 64-bit PowerPC Linux with musl 1.2.3 From 7fb2fc01a5410163264ce195a9eb1db61fb54d87 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:47:41 -0500 Subject: [PATCH 8/9] feat: clarify how to use `black_box()` Co-authored-by: Ben Kimock --- library/core/src/hint.rs | 92 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 3 deletions(-) diff --git a/library/core/src/hint.rs b/library/core/src/hint.rs index c59e4414d3726..9c054b99a27ac 100644 --- a/library/core/src/hint.rs +++ b/library/core/src/hint.rs @@ -310,6 +310,8 @@ pub fn spin_loop() { /// behavior in the calling code. This property makes `black_box` useful for writing code in which /// certain optimizations are not desired, such as benchmarks. /// +///
+/// /// Note however, that `black_box` is only (and can only be) provided on a "best-effort" basis. The /// extent to which it can block optimisations may vary depending upon the platform and code-gen /// backend used. Programs cannot rely on `black_box` for *correctness*, beyond it behaving as the @@ -317,6 +319,8 @@ pub fn spin_loop() { /// This also means that this function does not offer any guarantees for cryptographic or security /// purposes. /// +///
+/// /// [`std::convert::identity`]: crate::convert::identity /// /// # When is this useful? @@ -357,7 +361,7 @@ pub fn spin_loop() { /// ``` /// use std::hint::black_box; /// -/// // Same `contains` function +/// // Same `contains` function. /// fn contains(haystack: &[&str], needle: &str) -> bool { /// haystack.iter().any(|x| x == &needle) /// } @@ -366,8 +370,13 @@ pub fn spin_loop() { /// let haystack = vec!["abc", "def", "ghi", "jkl", "mno"]; /// let needle = "ghi"; /// for _ in 0..10 { -/// // Adjust our benchmark loop contents -/// black_box(contains(black_box(&haystack), black_box(needle))); +/// // Force the compiler to run `contains`, even though it is a pure function whose +/// // results are unused. +/// black_box(contains( +/// // Prevent the compiler from making assumptions about the input. +/// black_box(&haystack), +/// black_box(needle), +/// )); /// } /// } /// ``` @@ -382,6 +391,83 @@ pub fn spin_loop() { /// /// This makes our benchmark much more realistic to how the function would actually be used, where /// arguments are usually not known at compile time and the result is used in some way. +/// +/// # How to use this +/// +/// In practice, `black_box` serves two purposes: +/// +/// 1. It prevents the compiler from making optimizations related to the value returned by `black_box` +/// 2. It forces the value passed to `black_box` to be calculated, even if the return value of `black_box` is unused +/// +/// ``` +/// use std::hint::black_box; +/// +/// let zero = 0; +/// let five = 5; +/// +/// // The compiler will see this and remove the `* five` call, because it knows that multiplying +/// // any integer by 0 will result in 0. +/// let c = zero * five; +/// +/// // Adding `black_box` here disables the compiler's ability to reason about the first operand in the multiplication. +/// // It is forced to assume that it can be any possible number, so it cannot remove the `* five` +/// // operation. +/// let c = black_box(zero) * five; +/// ``` +/// +/// While most cases will not be as clear-cut as the above example, it still illustrates how +/// `black_box` can be used. When benchmarking a function, you usually want to wrap its inputs in +/// `black_box` so the compiler cannot make optimizations that would be unrealistic in real-life +/// use. +/// +/// ``` +/// use std::hint::black_box; +/// +/// // This is a simple function that increments its input by 1. Note that it is pure, meaning it +/// // has no side-effects. This function has no effect if its result is unused. (An example of a +/// // function *with* side-effects is `println!()`.) +/// fn increment(x: u8) -> u8 { +/// x + 1 +/// } +/// +/// // Here, we call `increment` but discard its result. The compiler, seeing this and knowing that +/// // `increment` is pure, will eliminate this function call entirely. This may not be desired, +/// // though, especially if we're trying to track how much time `increment` takes to execute. +/// let _ = increment(black_box(5)); +/// +/// // Here, we force `increment` to be executed. This is because the compiler treats `black_box` +/// // as if it has side-effects, and thus must compute its input. +/// let _ = black_box(increment(black_box(5))); +/// ``` +/// +/// There may be additional situations where you want to wrap the result of a function in +/// `black_box` to force its execution. This is situational though, and may not have any effect +/// (such as when the function returns a zero-sized type such as [`()` unit][unit]). +/// +/// Note that `black_box` has no effect on how its input is treated, only its output. As such, +/// expressions passed to `black_box` may still be optimized: +/// +/// ``` +/// use std::hint::black_box; +/// +/// // The compiler sees this... +/// let y = black_box(5 * 10); +/// +/// // ...as this. As such, it will likely simplify `5 * 10` to just `50`. +/// let _0 = 5 * 10; +/// let y = black_box(_0); +/// ``` +/// +/// In the above example, the `5 * 10` expression is considered distinct from the `black_box` call, +/// and thus is still optimized by the compiler. You can prevent this by moving the multiplication +/// operation outside of `black_box`: +/// +/// ``` +/// use std::hint::black_box; +/// +/// // No assumptions can be made about either operand, so the multiplication is not optimized out. +/// let y = black_box(5) * black_box(10); +/// ``` #[inline] #[stable(feature = "bench_black_box", since = "1.66.0")] #[rustc_const_unstable(feature = "const_black_box", issue = "none")] From 2e412fef75ff2f45186863a76fdd7c5a9ec1f018 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 12 Dec 2024 11:49:31 +1100 Subject: [PATCH 9/9] Remove `Lexer`'s dependency on `Parser`. Lexing precedes parsing, as you'd expect: `Lexer` creates a `TokenStream` and `Parser` then parses that `TokenStream`. But, in a horrendous violation of layering abstractions and common sense, `Lexer` depends on `Parser`! The `Lexer::unclosed_delim_err` method does some error recovery that relies on creating a `Parser` to do some post-processing of the `TokenStream` that the `Lexer` just created. This commit just removes `unclosed_delim_err`. This change removes `Lexer`'s dependency on `Parser`, and also means that `lex_token_tree`'s return value can have a more typical form. The cost is slightly worse error messages in two obscure cases, as shown in these tests: - tests/ui/parser/brace-in-let-chain.rs: there is slightly less explanation in this case involving an extra `{`. - tests/ui/parser/diff-markers/unclosed-delims{,-in-macro}.rs: the diff marker detection is no longer supported (because that detection is implemented in the parser). In my opinion this cost is outweighed by the magnitude of the code cleanup. --- compiler/rustc_parse/src/lexer/mod.rs | 40 ++++---- compiler/rustc_parse/src/lexer/tokentrees.rs | 94 +++---------------- tests/ui/parser/brace-in-let-chain.rs | 4 +- tests/ui/parser/brace-in-let-chain.stderr | 30 +----- .../diff-markers/unclosed-delims-in-macro.rs | 6 +- .../unclosed-delims-in-macro.stderr | 27 ++---- .../ui/parser/diff-markers/unclosed-delims.rs | 14 +-- .../diff-markers/unclosed-delims.stderr | 27 ++---- 8 files changed, 68 insertions(+), 174 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/mod.rs b/compiler/rustc_parse/src/lexer/mod.rs index 2426eb81678e1..443ddfc94ec2b 100644 --- a/compiler/rustc_parse/src/lexer/mod.rs +++ b/compiler/rustc_parse/src/lexer/mod.rs @@ -69,24 +69,30 @@ pub(crate) fn lex_token_trees<'psess, 'src>( token: Token::dummy(), diag_info: TokenTreeDiagInfo::default(), }; - let (_open_spacing, stream, res) = lexer.lex_token_trees(/* is_delimited */ false); - let unmatched_delims = lexer.diag_info.unmatched_delims; - - if res.is_ok() && unmatched_delims.is_empty() { - Ok(stream) - } else { - // Return error if there are unmatched delimiters or unclosed delimiters. - // We emit delimiter mismatch errors first, then emit the unclosing delimiter mismatch - // because the delimiter mismatch is more likely to be the root cause of error - let mut buffer: Vec<_> = unmatched_delims - .into_iter() - .filter_map(|unmatched_delim| make_unclosed_delims_error(unmatched_delim, psess)) - .collect(); - if let Err(errs) = res { - // Add unclosing delimiter or diff marker errors - buffer.extend(errs); + let res = lexer.lex_token_trees(/* is_delimited */ false); + + let mut unmatched_delims: Vec<_> = lexer + .diag_info + .unmatched_delims + .into_iter() + .filter_map(|unmatched_delim| make_unclosed_delims_error(unmatched_delim, psess)) + .collect(); + + match res { + Ok((_open_spacing, stream)) => { + if unmatched_delims.is_empty() { + Ok(stream) + } else { + // Return error if there are unmatched delimiters or unclosed delimiters. + Err(unmatched_delims) + } + } + Err(errs) => { + // We emit delimiter mismatch errors first, then emit the unclosing delimiter mismatch + // because the delimiter mismatch is more likely to be the root cause of error + unmatched_delims.extend(errs); + Err(unmatched_delims) } - Err(buffer) } } diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index ee38f16d4ecd5..b3f83a320241e 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -1,12 +1,10 @@ use rustc_ast::token::{self, Delimiter, Token}; use rustc_ast::tokenstream::{DelimSpacing, DelimSpan, Spacing, TokenStream, TokenTree}; use rustc_ast_pretty::pprust::token_to_string; -use rustc_errors::{Applicability, Diag}; -use rustc_span::symbol::kw; +use rustc_errors::Diag; use super::diagnostics::{report_suspicious_mismatch_block, same_indentation_level}; use super::{Lexer, UnmatchedDelim}; -use crate::Parser; impl<'psess, 'src> Lexer<'psess, 'src> { // Lex into a token stream. The `Spacing` in the result is that of the @@ -14,7 +12,7 @@ impl<'psess, 'src> Lexer<'psess, 'src> { pub(super) fn lex_token_trees( &mut self, is_delimited: bool, - ) -> (Spacing, TokenStream, Result<(), Vec>>) { + ) -> Result<(Spacing, TokenStream), Vec>> { // Move past the opening delimiter. let open_spacing = self.bump_minimal(); @@ -27,25 +25,25 @@ impl<'psess, 'src> Lexer<'psess, 'src> { debug_assert!(!matches!(delim, Delimiter::Invisible(_))); buf.push(match self.lex_token_tree_open_delim(delim) { Ok(val) => val, - Err(errs) => return (open_spacing, TokenStream::new(buf), Err(errs)), + Err(errs) => return Err(errs), }) } token::CloseDelim(delim) => { // Invisible delimiters cannot occur here because `TokenTreesReader` parses // code directly from strings, with no macro expansion involved. debug_assert!(!matches!(delim, Delimiter::Invisible(_))); - return ( - open_spacing, - TokenStream::new(buf), - if is_delimited { Ok(()) } else { Err(vec![self.close_delim_err(delim)]) }, - ); + return if is_delimited { + Ok((open_spacing, TokenStream::new(buf))) + } else { + Err(vec![self.close_delim_err(delim)]) + }; } token::Eof => { - return ( - open_spacing, - TokenStream::new(buf), - if is_delimited { Err(vec![self.eof_err()]) } else { Ok(()) }, - ); + return if is_delimited { + Err(vec![self.eof_err()]) + } else { + Ok((open_spacing, TokenStream::new(buf))) + }; } _ => { // Get the next normal token. @@ -107,10 +105,7 @@ impl<'psess, 'src> Lexer<'psess, 'src> { // Lex the token trees within the delimiters. // We stop at any delimiter so we can try to recover if the user // uses an incorrect delimiter. - let (open_spacing, tts, res) = self.lex_token_trees(/* is_delimited */ true); - if let Err(errs) = res { - return Err(self.unclosed_delim_err(tts, errs)); - } + let (open_spacing, tts) = self.lex_token_trees(/* is_delimited */ true)?; // Expand to cover the entire delimited token tree. let delim_span = DelimSpan::from_pair(pre_span, self.token.span); @@ -247,67 +242,6 @@ impl<'psess, 'src> Lexer<'psess, 'src> { this_spacing } - fn unclosed_delim_err( - &mut self, - tts: TokenStream, - mut errs: Vec>, - ) -> Vec> { - // If there are unclosed delims, see if there are diff markers and if so, point them - // out instead of complaining about the unclosed delims. - let mut parser = Parser::new(self.psess, tts, None); - let mut diff_errs = vec![]; - // Suggest removing a `{` we think appears in an `if`/`while` condition. - // We want to suggest removing a `{` only if we think we're in an `if`/`while` condition, - // but we have no way of tracking this in the lexer itself, so we piggyback on the parser. - let mut in_cond = false; - while parser.token != token::Eof { - if let Err(diff_err) = parser.err_vcs_conflict_marker() { - diff_errs.push(diff_err); - } else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) { - in_cond = true; - } else if matches!( - parser.token.kind, - token::CloseDelim(Delimiter::Brace) | token::FatArrow - ) { - // End of the `if`/`while` body, or the end of a `match` guard. - in_cond = false; - } else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) { - // Store the `&&` and `let` to use their spans later when creating the diagnostic - let maybe_andand = parser.look_ahead(1, |t| t.clone()); - let maybe_let = parser.look_ahead(2, |t| t.clone()); - if maybe_andand == token::OpenDelim(Delimiter::Brace) { - // This might be the beginning of the `if`/`while` body (i.e., the end of the - // condition). - in_cond = false; - } else if maybe_andand == token::AndAnd && maybe_let.is_keyword(kw::Let) { - let mut err = parser.dcx().struct_span_err( - parser.token.span, - "found a `{` in the middle of a let-chain", - ); - err.span_suggestion( - parser.token.span, - "consider removing this brace to parse the `let` as part of the same chain", - "", - Applicability::MachineApplicable, - ); - err.span_label( - maybe_andand.span.to(maybe_let.span), - "you might have meant to continue the let-chain here", - ); - errs.push(err); - } - } - parser.bump(); - } - if !diff_errs.is_empty() { - for err in errs { - err.cancel(); - } - return diff_errs; - } - errs - } - fn close_delim_err(&mut self, delim: Delimiter) -> Diag<'psess> { // An unexpected closing delimiter (i.e., there is no matching opening delimiter). let token_str = token_to_string(&self.token); diff --git a/tests/ui/parser/brace-in-let-chain.rs b/tests/ui/parser/brace-in-let-chain.rs index 1f34c73a2c3bd..2009bc88d9e8d 100644 --- a/tests/ui/parser/brace-in-let-chain.rs +++ b/tests/ui/parser/brace-in-let-chain.rs @@ -3,7 +3,7 @@ #![feature(let_chains)] fn main() { if let () = () - && let () = () { //~ERROR: found a `{` in the middle of a let-chain + && let () = () { && let () = () { } @@ -11,7 +11,7 @@ fn main() { fn quux() { while let () = () - && let () = () { //~ERROR: found a `{` in the middle of a let-chain + && let () = () { && let () = () { } diff --git a/tests/ui/parser/brace-in-let-chain.stderr b/tests/ui/parser/brace-in-let-chain.stderr index 913a34700dfc9..12af95c278688 100644 --- a/tests/ui/parser/brace-in-let-chain.stderr +++ b/tests/ui/parser/brace-in-let-chain.stderr @@ -27,33 +27,5 @@ LL | } LL | } | ^ -error: found a `{` in the middle of a let-chain - --> $DIR/brace-in-let-chain.rs:14:24 - | -LL | && let () = () { - | ^ -LL | && let () = () - | ------ you might have meant to continue the let-chain here - | -help: consider removing this brace to parse the `let` as part of the same chain - | -LL - && let () = () { -LL + && let () = () - | - -error: found a `{` in the middle of a let-chain - --> $DIR/brace-in-let-chain.rs:6:24 - | -LL | && let () = () { - | ^ -LL | && let () = () - | ------ you might have meant to continue the let-chain here - | -help: consider removing this brace to parse the `let` as part of the same chain - | -LL - && let () = () { -LL + && let () = () - | - -error: aborting due to 3 previous errors +error: aborting due to 1 previous error diff --git a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs index da1774acea542..41a7de03d4b79 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs +++ b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs @@ -1,9 +1,11 @@ +// The diff marker detection was removed for this example, because it relied on +// the lexer having a dependency on the parser, which was horrible. + macro_rules! foo { <<<<<<< HEAD - //~^ ERROR encountered diff marker () { ======= () { // >>>>>>> 7a4f13c blah blah blah } -} +} //~ this file contains an unclosed delimiter diff --git a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr index 927821ddfaedb..b33f2b5d1b8b2 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr +++ b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr @@ -1,23 +1,16 @@ -error: encountered diff marker - --> $DIR/unclosed-delims-in-macro.rs:2:1 +error: this file contains an unclosed delimiter + --> $DIR/unclosed-delims-in-macro.rs:11:48 | +LL | macro_rules! foo { + | - unclosed delimiter LL | <<<<<<< HEAD - | ^^^^^^^ between this marker and `=======` is the code that we're merging into +LL | () { + | - this delimiter might not be properly closed... ... -LL | ======= - | ------- between this marker and `>>>>>>>` is the incoming code -LL | () { // -LL | >>>>>>> 7a4f13c blah blah blah - | ^^^^^^^ this marker concludes the conflict region - | - = note: conflict markers indicate that a merge was started but could not be completed due to merge conflicts - to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers - = help: if you're having merge conflicts after pulling new code: - the top section is the code you already had and the bottom section is the remote code - if you're in the middle of a rebase: - the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased - = note: for an explanation on these markers from the `git` documentation: - visit +LL | } + | - ^ + | | + | ...as it matches this but it has different indentation error: aborting due to 1 previous error diff --git a/tests/ui/parser/diff-markers/unclosed-delims.rs b/tests/ui/parser/diff-markers/unclosed-delims.rs index 7d400c3827bb6..827c1eebb9d5f 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims.rs +++ b/tests/ui/parser/diff-markers/unclosed-delims.rs @@ -1,18 +1,12 @@ +// The diff marker detection was removed for this example, because it relied on +// the lexer having a dependency on the parser, which was horrible. + mod tests { #[test] <<<<<<< HEAD -//~^ ERROR encountered diff marker -//~| NOTE between this marker and `=======` - -//~| NOTE conflict markers indicate that -//~| HELP if you're having merge conflicts -//~| NOTE for an explanation on these markers - fn test1() { ======= -//~^ NOTE between this marker and `>>>>>>>` fn test2() { >>>>>>> 7a4f13c blah blah blah -//~^ NOTE this marker concludes the conflict region } -} +} //~ this file contains an unclosed delimiter diff --git a/tests/ui/parser/diff-markers/unclosed-delims.stderr b/tests/ui/parser/diff-markers/unclosed-delims.stderr index 1eab96442b4f2..b2541aa47baed 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims.stderr +++ b/tests/ui/parser/diff-markers/unclosed-delims.stderr @@ -1,23 +1,16 @@ -error: encountered diff marker - --> $DIR/unclosed-delims.rs:3:1 +error: this file contains an unclosed delimiter + --> $DIR/unclosed-delims.rs:12:48 | -LL | <<<<<<< HEAD - | ^^^^^^^ between this marker and `=======` is the code that we're merging into +LL | mod tests { + | - unclosed delimiter ... -LL | ======= - | ------- between this marker and `>>>>>>>` is the incoming code +LL | fn test1() { + | - this delimiter might not be properly closed... ... -LL | >>>>>>> 7a4f13c blah blah blah - | ^^^^^^^ this marker concludes the conflict region - | - = note: conflict markers indicate that a merge was started but could not be completed due to merge conflicts - to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers - = help: if you're having merge conflicts after pulling new code: - the top section is the code you already had and the bottom section is the remote code - if you're in the middle of a rebase: - the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased - = note: for an explanation on these markers from the `git` documentation: - visit +LL | } + | - ^ + | | + | ...as it matches this but it has different indentation error: aborting due to 1 previous error