From 2d47816cbaebb3b8f400b11fa122feae00fd5c58 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sat, 5 Nov 2022 01:08:57 -0700 Subject: [PATCH 01/29] rustc_llvm: Add a `-Z print-llvm-stats` option to expose LLVM statistics. LLVM has a neat [statistics] feature that tracks how often optimizations kick in. It's very handy for optimization work. Since we expose the LLVM pass timings, I thought it made sense to expose the LLVM statistics too. [statistics]: https://llvm.org/docs/ProgrammersManual.html#the-statistic-class-stats-option --- compiler/rustc_codegen_gcc/src/lib.rs | 4 ++++ compiler/rustc_codegen_llvm/src/lib.rs | 5 +++++ compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 3 +++ compiler/rustc_codegen_llvm/src/llvm_util.rs | 4 ++++ compiler/rustc_codegen_ssa/src/back/write.rs | 4 ++++ compiler/rustc_codegen_ssa/src/traits/write.rs | 1 + compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 6 ++++++ compiler/rustc_session/src/options.rs | 3 +++ compiler/rustc_session/src/session.rs | 4 ++++ 10 files changed, 35 insertions(+) diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index 2a6b642782dfd..04ac0254a81ba 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -239,6 +239,10 @@ impl WriteBackendMethods for GccCodegenBackend { unimplemented!(); } + fn print_statistics(&self) { + unimplemented!() + } + unsafe fn optimize(_cgcx: &CodegenContext, _diag_handler: &Handler, module: &ModuleCodegen, config: &ModuleConfig) -> Result<(), FatalError> { module.module_llvm.context.set_optimization_level(to_gcc_opt_level(config.opt_level)); Ok(()) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 24ba28bbc82c2..713c22ebfebad 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -181,6 +181,11 @@ impl WriteBackendMethods for LlvmCodegenBackend { llvm::LLVMRustPrintPassTimings(); } } + fn print_statistics(&self) { + unsafe { + llvm::LLVMRustPrintStatistics(); + } + } fn run_link( cgcx: &CodegenContext, diag_handler: &Handler, diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 605f0154773a7..3eb0455574973 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1870,6 +1870,9 @@ extern "C" { /// Print the pass timings since static dtors aren't picking them up. pub fn LLVMRustPrintPassTimings(); + /// Print the statistics since static dtors aren't picking them up. + pub fn LLVMRustPrintStatistics(); + pub fn LLVMStructCreateNamed(C: &Context, Name: *const c_char) -> &Type; pub fn LLVMStructSetBody<'a>( diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 03be0654b50bb..12649de5866fc 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -110,6 +110,10 @@ unsafe fn configure_llvm(sess: &Session) { // Use non-zero `import-instr-limit` multiplier for cold callsites. add("-import-cold-multiplier=0.1", false); + if sess.print_llvm_stats() { + add("-stats", false); + } + for arg in sess_args { add(&(*arg), true); } diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index ececa29b23155..0ce6129a46252 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1945,6 +1945,10 @@ impl OngoingCodegen { self.backend.print_pass_timings() } + if sess.print_llvm_stats() { + self.backend.print_statistics() + } + ( CodegenResults { metadata: self.metadata, diff --git a/compiler/rustc_codegen_ssa/src/traits/write.rs b/compiler/rustc_codegen_ssa/src/traits/write.rs index 9826256a4c5d5..a3b11ed4fe8f6 100644 --- a/compiler/rustc_codegen_ssa/src/traits/write.rs +++ b/compiler/rustc_codegen_ssa/src/traits/write.rs @@ -35,6 +35,7 @@ pub trait WriteBackendMethods: 'static + Sized + Clone { cached_modules: Vec<(SerializedModule, WorkProduct)>, ) -> Result<(Vec>, Vec), FatalError>; fn print_pass_timings(&self); + fn print_statistics(&self); unsafe fn optimize( cgcx: &CodegenContext, diag_handler: &Handler, diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 09141afd13707..0eac098e8a3e3 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -715,6 +715,7 @@ fn test_unstable_options_tracking_hash() { // `pre_link_arg` is omitted because it just forwards to `pre_link_args`. untracked!(pre_link_args, vec![String::from("abc"), String::from("def")]); untracked!(print_llvm_passes, true); + untracked!(print_llvm_stats, true); untracked!(print_mono_items, Some(String::from("abc"))); untracked!(print_type_sizes, true); untracked!(proc_macro_backtrace, true); diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index ab5fa961b95a4..89beb09db7550 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1,4 +1,5 @@ #include "LLVMWrapper.h" +#include "llvm/ADT/Statistic.h" #include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DiagnosticHandler.h" #include "llvm/IR/DiagnosticInfo.h" @@ -116,6 +117,11 @@ extern "C" void LLVMRustPrintPassTimings() { TimerGroup::printAll(OS); } +extern "C" void LLVMRustPrintStatistics() { + raw_fd_ostream OS(2, false); // stderr. + llvm::PrintStatistics(OS); +} + extern "C" LLVMValueRef LLVMRustGetNamedValue(LLVMModuleRef M, const char *Name, size_t NameLen) { return wrap(unwrap(M)->getNamedValue(StringRef(Name, NameLen))); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 87d67c099cedb..90007d7c8496a 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1672,6 +1672,9 @@ options! { "make rustc print the total optimization fuel used by a crate"), print_llvm_passes: bool = (false, parse_bool, [UNTRACKED], "print the LLVM optimization passes being run (default: no)"), + #[rustc_lint_opt_deny_field_access("use `Session::print_llvm_stats` instead of this field")] + print_llvm_stats: bool = (true, parse_bool, [UNTRACKED], + "print LLVM statistics (default: no)"), print_mono_items: Option = (None, parse_opt_string, [UNTRACKED], "print the result of the monomorphization collection pass"), print_type_sizes: bool = (false, parse_bool, [UNTRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 5be122ffbdeb0..c2588b9a99a1b 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1057,6 +1057,10 @@ impl Session { self.opts.unstable_opts.verbose } + pub fn print_llvm_stats(&self) -> bool { + self.opts.unstable_opts.print_llvm_stats + } + pub fn verify_llvm_ir(&self) -> bool { self.opts.unstable_opts.verify_llvm_ir || option_env!("RUSTC_VERIFY_LLVM_IR").is_some() } From 138f522b590492d1ef80f1483382a2a678dec7d9 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sat, 5 Nov 2022 01:13:42 -0700 Subject: [PATCH 02/29] Don't enable by default :) --- compiler/rustc_session/src/options.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 90007d7c8496a..5b5cafa965625 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1673,7 +1673,7 @@ options! { print_llvm_passes: bool = (false, parse_bool, [UNTRACKED], "print the LLVM optimization passes being run (default: no)"), #[rustc_lint_opt_deny_field_access("use `Session::print_llvm_stats` instead of this field")] - print_llvm_stats: bool = (true, parse_bool, [UNTRACKED], + print_llvm_stats: bool = (false, parse_bool, [UNTRACKED], "print LLVM statistics (default: no)"), print_mono_items: Option = (None, parse_opt_string, [UNTRACKED], "print the result of the monomorphization collection pass"), From 4d307c482271ea3575a13b6c04222911e7706189 Mon Sep 17 00:00:00 2001 From: khei4 Date: Mon, 17 Jul 2023 00:37:52 +0900 Subject: [PATCH 03/29] print on rustc_codegen_llvm and rename malloc and cpy c_char --- compiler/rustc_codegen_llvm/src/lib.rs | 30 +++++++++++++++---- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 4 +-- compiler/rustc_interface/src/tests.rs | 2 +- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 26 +++++++++++----- compiler/rustc_session/src/options.rs | 6 ++-- compiler/rustc_session/src/session.rs | 2 +- 6 files changed, 49 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 713c22ebfebad..c03b218882445 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -177,14 +177,32 @@ impl WriteBackendMethods for LlvmCodegenBackend { type ThinData = back::lto::ThinData; type ThinBuffer = back::lto::ThinBuffer; fn print_pass_timings(&self) { - unsafe { - llvm::LLVMRustPrintPassTimings(); - } + let msg = unsafe { + let cstr = llvm::LLVMRustPrintPassTimings(); + if cstr.is_null() { + "failed to get pass timings".into() + } else { + let timings = CStr::from_ptr(cstr).to_bytes(); + let timings = String::from_utf8_lossy(timings).to_string(); + libc::free(cstr as *mut _); + timings + } + }; + println!("{}", msg); } fn print_statistics(&self) { - unsafe { - llvm::LLVMRustPrintStatistics(); - } + let msg = unsafe { + let cstr = llvm::LLVMRustPrintStatistics(); + if cstr.is_null() { + "failed to get stats".into() + } else { + let stats = CStr::from_ptr(cstr).to_bytes(); + let stats = String::from_utf8_lossy(stats).to_string(); + libc::free(cstr as *mut _); + stats + } + }; + println!("{}", msg); } fn run_link( cgcx: &CodegenContext, diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 3eb0455574973..7cc79d859a389 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1868,10 +1868,10 @@ extern "C" { pub fn LLVMRustGetLastError() -> *const c_char; /// Print the pass timings since static dtors aren't picking them up. - pub fn LLVMRustPrintPassTimings(); + pub fn LLVMRustPrintPassTimings() -> *const c_char; /// Print the statistics since static dtors aren't picking them up. - pub fn LLVMRustPrintStatistics(); + pub fn LLVMRustPrintStatistics() -> *const c_char; pub fn LLVMStructCreateNamed(C: &Context, Name: *const c_char) -> &Type; diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 0eac098e8a3e3..9aee39962df40 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -714,8 +714,8 @@ fn test_unstable_options_tracking_hash() { untracked!(perf_stats, true); // `pre_link_arg` is omitted because it just forwards to `pre_link_args`. untracked!(pre_link_args, vec![String::from("abc"), String::from("def")]); + untracked!(print_codegen_stats, true); untracked!(print_llvm_passes, true); - untracked!(print_llvm_stats, true); untracked!(print_mono_items, Some(String::from("abc"))); untracked!(print_type_sizes, true); untracked!(proc_macro_backtrace, true); diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 89beb09db7550..695b8847a976d 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -112,14 +112,24 @@ extern "C" void LLVMRustSetNormalizedTarget(LLVMModuleRef M, unwrap(M)->setTargetTriple(Triple::normalize(Triple)); } -extern "C" void LLVMRustPrintPassTimings() { - raw_fd_ostream OS(2, false); // stderr. - TimerGroup::printAll(OS); -} - -extern "C" void LLVMRustPrintStatistics() { - raw_fd_ostream OS(2, false); // stderr. - llvm::PrintStatistics(OS); +extern "C" const char *LLVMRustPrintPassTimings(void) { + std::string buf; + raw_string_ostream SS(buf); + TimerGroup::printAll(SS); + SS.flush(); + char* CStr = (char*) malloc((buf.length() + 1) * sizeof(char)); + strcpy(CStr, buf.c_str()); + return CStr; +} + +extern "C" const char *LLVMRustPrintStatistics(void) { + std::string buf; + raw_string_ostream SS(buf); + llvm::PrintStatistics(SS); + SS.flush(); + char* CStr = (char*) malloc((buf.length() + 1) * sizeof(char)); + strcpy(CStr, buf.c_str()); + return CStr; } extern "C" LLVMValueRef LLVMRustGetNamedValue(LLVMModuleRef M, const char *Name, diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 5b5cafa965625..39efe9abeecdb 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1668,13 +1668,13 @@ options! { "use a more precise version of drop elaboration for matches on enums (default: yes). \ This results in better codegen, but has caused miscompilations on some tier 2 platforms. \ See #77382 and #74551."), + #[rustc_lint_opt_deny_field_access("use `Session::print_codegen_stats` instead of this field")] + print_codegen_stats: bool = (false, parse_bool, [UNTRACKED], + "print codegen statistics (default: no)"), print_fuel: Option = (None, parse_opt_string, [TRACKED], "make rustc print the total optimization fuel used by a crate"), print_llvm_passes: bool = (false, parse_bool, [UNTRACKED], "print the LLVM optimization passes being run (default: no)"), - #[rustc_lint_opt_deny_field_access("use `Session::print_llvm_stats` instead of this field")] - print_llvm_stats: bool = (false, parse_bool, [UNTRACKED], - "print LLVM statistics (default: no)"), print_mono_items: Option = (None, parse_opt_string, [UNTRACKED], "print the result of the monomorphization collection pass"), print_type_sizes: bool = (false, parse_bool, [UNTRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index c2588b9a99a1b..6ebba596a9433 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1058,7 +1058,7 @@ impl Session { } pub fn print_llvm_stats(&self) -> bool { - self.opts.unstable_opts.print_llvm_stats + self.opts.unstable_opts.print_codegen_stats } pub fn verify_llvm_ir(&self) -> bool { From b1398cac9d529fb818b0fdde44255e99fa63641e Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 18 Jul 2023 09:56:55 +0900 Subject: [PATCH 04/29] Make {Rc,Arc}::allocator associated functions --- library/alloc/src/rc.rs | 8 ++++++-- library/alloc/src/sync.rs | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index b3ec830a7d7c6..bf01b2082eda2 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -661,10 +661,14 @@ impl Rc { impl Rc { /// Returns a reference to the underlying allocator. + /// + /// Note: this is an associated function, which means that you have + /// to call it as `Rc::allocator(&r)` instead of `r.allocator()`. This + /// is so that there is no conflict with a method on the inner type. #[inline] #[unstable(feature = "allocator_api", issue = "32838")] - pub fn allocator(&self) -> &A { - &self.alloc + pub fn allocator(this: &Self) -> &A { + &this.alloc } /// Constructs a new `Rc` in the provided allocator. /// diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index e00850eb5d8a1..c2202f2fce553 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -678,10 +678,14 @@ impl Arc { impl Arc { /// Returns a reference to the underlying allocator. + /// + /// Note: this is an associated function, which means that you have + /// to call it as `Arc::allocator(&a)` instead of `a.allocator()`. This + /// is so that there is no conflict with a method on the inner type. #[inline] #[unstable(feature = "allocator_api", issue = "32838")] - pub fn allocator(&self) -> &A { - &self.alloc + pub fn allocator(this: &Self) -> &A { + &this.alloc } /// Constructs a new `Arc` in the provided allocator. /// From c7bf20dfdcbedbba05445035bcabd4f706ba9e42 Mon Sep 17 00:00:00 2001 From: khei4 Date: Wed, 19 Jul 2023 17:00:06 +0900 Subject: [PATCH 05/29] address feedback from nikic and oli-obk https://github.com/rust-lang/rust/pull/113723/files use slice memcpy rather than strcpy and write it on stdout use println on failure Co-authored-by: Oli Scherer --- compiler/rustc_codegen_llvm/src/lib.rs | 31 +++++++++---------- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 4 +-- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 14 +++++---- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index c03b218882445..b035437621033 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -46,6 +46,7 @@ use rustc_span::symbol::Symbol; use std::any::Any; use std::ffi::CStr; +use std::io::Write; mod back { pub mod archive; @@ -177,32 +178,30 @@ impl WriteBackendMethods for LlvmCodegenBackend { type ThinData = back::lto::ThinData; type ThinBuffer = back::lto::ThinBuffer; fn print_pass_timings(&self) { - let msg = unsafe { - let cstr = llvm::LLVMRustPrintPassTimings(); + unsafe { + let mut size = 0; + let cstr = llvm::LLVMRustPrintPassTimings(&mut size as *mut usize); if cstr.is_null() { - "failed to get pass timings".into() + println!("failed to get pass timings"); } else { - let timings = CStr::from_ptr(cstr).to_bytes(); - let timings = String::from_utf8_lossy(timings).to_string(); + let timings = std::slice::from_raw_parts(cstr as *const u8, size); + std::io::stdout().write_all(timings).unwrap(); libc::free(cstr as *mut _); - timings } - }; - println!("{}", msg); + } } fn print_statistics(&self) { - let msg = unsafe { - let cstr = llvm::LLVMRustPrintStatistics(); + unsafe { + let mut size = 0; + let cstr = llvm::LLVMRustPrintStatistics(&mut size as *mut usize); if cstr.is_null() { - "failed to get stats".into() + println!("failed to get pass stats"); } else { - let stats = CStr::from_ptr(cstr).to_bytes(); - let stats = String::from_utf8_lossy(stats).to_string(); + let stats = std::slice::from_raw_parts(cstr as *const u8, size); + std::io::stdout().write_all(stats).unwrap(); libc::free(cstr as *mut _); - stats } - }; - println!("{}", msg); + } } fn run_link( cgcx: &CodegenContext, diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 7cc79d859a389..fd88994c31c2a 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1868,10 +1868,10 @@ extern "C" { pub fn LLVMRustGetLastError() -> *const c_char; /// Print the pass timings since static dtors aren't picking them up. - pub fn LLVMRustPrintPassTimings() -> *const c_char; + pub fn LLVMRustPrintPassTimings(size: *const size_t) -> *const c_char; /// Print the statistics since static dtors aren't picking them up. - pub fn LLVMRustPrintStatistics() -> *const c_char; + pub fn LLVMRustPrintStatistics(size: *const size_t) -> *const c_char; pub fn LLVMStructCreateNamed(C: &Context, Name: *const c_char) -> &Type; diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 695b8847a976d..870a2e02cbaa5 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -112,23 +112,25 @@ extern "C" void LLVMRustSetNormalizedTarget(LLVMModuleRef M, unwrap(M)->setTargetTriple(Triple::normalize(Triple)); } -extern "C" const char *LLVMRustPrintPassTimings(void) { +extern "C" const char *LLVMRustPrintPassTimings(size_t *Len) { std::string buf; raw_string_ostream SS(buf); TimerGroup::printAll(SS); SS.flush(); - char* CStr = (char*) malloc((buf.length() + 1) * sizeof(char)); - strcpy(CStr, buf.c_str()); + *Len = buf.length(); + char *CStr = (char *)malloc(*Len); + memcpy(CStr, buf.c_str(), *Len); return CStr; } -extern "C" const char *LLVMRustPrintStatistics(void) { +extern "C" const char *LLVMRustPrintStatistics(size_t *Len) { std::string buf; raw_string_ostream SS(buf); llvm::PrintStatistics(SS); SS.flush(); - char* CStr = (char*) malloc((buf.length() + 1) * sizeof(char)); - strcpy(CStr, buf.c_str()); + *Len = buf.length(); + char *CStr = (char *)malloc(*Len); + memcpy(CStr, buf.c_str(), *Len); return CStr; } From 11dcd1d3d7e805a555a41992312137700ee370a1 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 24 Apr 2023 17:57:41 -0700 Subject: [PATCH 06/29] Add test of --print KIND=PATH --- tests/run-make/print-cfg/Makefile | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/run-make/print-cfg/Makefile b/tests/run-make/print-cfg/Makefile index 126f5768c90f2..6b153e5b54edd 100644 --- a/tests/run-make/print-cfg/Makefile +++ b/tests/run-make/print-cfg/Makefile @@ -2,7 +2,7 @@ include ../tools.mk -all: default +all: default output_to_file $(RUSTC) --target x86_64-pc-windows-gnu --print cfg | $(CGREP) windows $(RUSTC) --target x86_64-pc-windows-gnu --print cfg | $(CGREP) x86_64 $(RUSTC) --target i686-pc-windows-msvc --print cfg | $(CGREP) msvc @@ -11,6 +11,23 @@ all: default $(RUSTC) --target arm-unknown-linux-gnueabihf --print cfg | $(CGREP) target_abi= $(RUSTC) --target arm-unknown-linux-gnueabihf --print cfg | $(CGREP) eabihf +output_to_file: + # Backend-independent, printed by rustc_driver_impl/src/lib.rs + $(RUSTC) --target x86_64-pc-windows-gnu --print cfg=$(TMPDIR)/cfg.txt + $(CGREP) windows < $(TMPDIR)/cfg.txt + + # Printed from CodegenBackend trait impl in rustc_codegen_llvm/src/lib.rs + $(RUSTC) --print relocation-models=$(TMPDIR)/relocation-models.txt + $(CGREP) dynamic-no-pic < $(TMPDIR)/relocation-models.txt + + # Printed by compiler/rustc_codegen_llvm/src/llvm_util.rs + $(RUSTC) --target wasm32-unknown-unknown --print target-features=$(TMPDIR)/target-features.txt + $(CGREP) reference-types < $(TMPDIR)/target-features.txt + + # Printed by C++ code in rustc_llvm/llvm-wrapper/PassWrapper.cpp + $(RUSTC) --target wasm32-unknown-unknown --print target-cpus=$(TMPDIR)/target-cpus.txt + $(CGREP) generic < $(TMPDIR)/target-cpus.txt + ifdef IS_WINDOWS default: $(RUSTC) --print cfg | $(CGREP) windows From c0dc0c6875e1907f1a8d690bae800c9d0c89b4f1 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 17:20:28 -0700 Subject: [PATCH 07/29] Store individual output file name with every PrintRequest --- compiler/rustc_codegen_llvm/src/lib.rs | 16 ++--- compiler/rustc_codegen_llvm/src/llvm_util.rs | 10 +-- compiler/rustc_codegen_ssa/src/back/link.rs | 6 +- .../rustc_codegen_ssa/src/traits/backend.rs | 2 +- compiler/rustc_driver_impl/src/lib.rs | 14 ++-- compiler/rustc_session/src/config.rs | 69 ++++++++++--------- 6 files changed, 61 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 24ba28bbc82c2..38a32ac845d26 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -40,7 +40,7 @@ use rustc_metadata::EncodedMetadata; use rustc_middle::dep_graph::{WorkProduct, WorkProductId}; use rustc_middle::query::Providers; use rustc_middle::ty::TyCtxt; -use rustc_session::config::{OptLevel, OutputFilenames, PrintRequest}; +use rustc_session::config::{OptLevel, OutputFilenames, PrintKind, PrintRequest}; use rustc_session::Session; use rustc_span::symbol::Symbol; @@ -262,9 +262,9 @@ impl CodegenBackend for LlvmCodegenBackend { |tcx, ()| llvm_util::global_llvm_features(tcx.sess, true) } - fn print(&self, req: PrintRequest, sess: &Session) { - match req { - PrintRequest::RelocationModels => { + fn print(&self, req: &PrintRequest, sess: &Session) { + match req.kind { + PrintKind::RelocationModels => { println!("Available relocation models:"); for name in &[ "static", @@ -280,21 +280,21 @@ impl CodegenBackend for LlvmCodegenBackend { } println!(); } - PrintRequest::CodeModels => { + PrintKind::CodeModels => { println!("Available code models:"); for name in &["tiny", "small", "kernel", "medium", "large"] { println!(" {}", name); } println!(); } - PrintRequest::TlsModels => { + PrintKind::TlsModels => { println!("Available TLS models:"); for name in &["global-dynamic", "local-dynamic", "initial-exec", "local-exec"] { println!(" {}", name); } println!(); } - PrintRequest::StackProtectorStrategies => { + PrintKind::StackProtectorStrategies => { println!( r#"Available stack protector strategies: all @@ -319,7 +319,7 @@ impl CodegenBackend for LlvmCodegenBackend { "# ); } - req => llvm_util::print(req, sess), + _other => llvm_util::print(req, sess), } } diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 03be0654b50bb..84d9acf45de57 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -12,7 +12,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::small_c_str::SmallCStr; use rustc_fs_util::path_to_c_string; use rustc_middle::bug; -use rustc_session::config::PrintRequest; +use rustc_session::config::{PrintKind, PrintRequest}; use rustc_session::Session; use rustc_span::symbol::Symbol; use rustc_target::spec::{MergeFunctions, PanicStrategy}; @@ -400,11 +400,11 @@ fn print_target_features(sess: &Session, tm: &llvm::TargetMachine) { println!("and may be renamed or removed in a future version of LLVM or rustc.\n"); } -pub(crate) fn print(req: PrintRequest, sess: &Session) { +pub(crate) fn print(req: &PrintRequest, sess: &Session) { require_inited(); let tm = create_informational_target_machine(sess); - match req { - PrintRequest::TargetCPUs => { + match req.kind { + PrintKind::TargetCPUs => { // SAFETY generate a C compatible string from a byte slice to pass // the target CPU name into LLVM, the lifetime of the reference is // at least as long as the C function @@ -412,7 +412,7 @@ pub(crate) fn print(req: PrintRequest, sess: &Session) { .unwrap_or_else(|e| bug!("failed to convert to cstring: {}", e)); unsafe { llvm::LLVMRustPrintTargetCPUs(tm, cpu_cstring.as_ptr()) }; } - PrintRequest::TargetFeatures => print_target_features(sess, tm), + PrintKind::TargetFeatures => print_target_features(sess, tm), _ => bug!("rustc_codegen_llvm can't handle print request: {:?}", req), } } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 0dfb41f42f0dc..bb8a520321a11 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -13,7 +13,7 @@ use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile; use rustc_middle::middle::dependency_format::Linkage; use rustc_middle::middle::exported_symbols::SymbolExportKind; use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, Strip}; -use rustc_session::config::{OutputFilenames, OutputType, PrintRequest, SplitDwarfKind}; +use rustc_session::config::{OutputFilenames, OutputType, PrintKind, SplitDwarfKind}; use rustc_session::cstore::DllImport; use rustc_session::output::{check_file_is_writeable, invalid_output_for_target, out_filename}; use rustc_session::search_paths::PathKind; @@ -596,7 +596,7 @@ fn link_staticlib<'a>( all_native_libs.extend_from_slice(&codegen_results.crate_info.used_libraries); - if sess.opts.prints.contains(&PrintRequest::NativeStaticLibs) { + if sess.opts.prints.iter().any(|print| print.kind == PrintKind::NativeStaticLibs) { print_native_static_libs(sess, &all_native_libs, &all_rust_dylibs); } @@ -744,7 +744,7 @@ fn link_natively<'a>( cmd.env_remove(k.as_ref()); } - if sess.opts.prints.contains(&PrintRequest::LinkArgs) { + if sess.opts.prints.iter().any(|print| print.kind == PrintKind::LinkArgs) { println!("{:?}", &cmd); } diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index b3c9ecf8b938b..f10bbbeb97cc2 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -61,7 +61,7 @@ pub trait CodegenBackend { fn locale_resource(&self) -> &'static str; fn init(&self, _sess: &Session) {} - fn print(&self, _req: PrintRequest, _sess: &Session) {} + fn print(&self, _req: &PrintRequest, _sess: &Session) {} fn target_features(&self, _sess: &Session, _allow_unstable: bool) -> Vec { vec![] } diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 25c043149e81c..27dfb6c3bca60 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -35,9 +35,7 @@ use rustc_interface::{interface, Queries}; use rustc_lint::LintStore; use rustc_metadata::locator; use rustc_session::config::{nightly_options, CG_OPTIONS, Z_OPTIONS}; -use rustc_session::config::{ - ErrorOutputType, Input, OutFileName, OutputType, PrintRequest, TrimmedDefPaths, -}; +use rustc_session::config::{ErrorOutputType, Input, OutFileName, OutputType, TrimmedDefPaths}; use rustc_session::cstore::MetadataLoader; use rustc_session::getopts::{self, Matches}; use rustc_session::lint::{Lint, LintId}; @@ -714,10 +712,10 @@ fn print_crate_info( sess: &Session, parse_attrs: bool, ) -> Compilation { - use rustc_session::config::PrintRequest::*; + use rustc_session::config::PrintKind::*; // NativeStaticLibs and LinkArgs are special - printed during linking // (empty iterator returns true) - if sess.opts.prints.iter().all(|&p| p == NativeStaticLibs || p == LinkArgs) { + if sess.opts.prints.iter().all(|p| p.kind == NativeStaticLibs || p.kind == LinkArgs) { return Compilation::Continue; } @@ -734,7 +732,7 @@ fn print_crate_info( None }; for req in &sess.opts.prints { - match *req { + match req.kind { TargetList => { let mut targets = rustc_target::spec::TARGETS.to_vec(); targets.sort_unstable(); @@ -761,7 +759,7 @@ fn print_crate_info( }; let t_outputs = rustc_interface::util::build_output_filenames(attrs, sess); let id = rustc_session::output::find_crate_name(sess, attrs); - if *req == PrintRequest::CrateName { + if req.kind == CrateName { safe_println!("{id}"); continue; } @@ -817,7 +815,7 @@ fn print_crate_info( | TargetCPUs | StackProtectorStrategies | TargetFeatures => { - codegen_backend.print(*req, sess); + codegen_backend.print(req, sess); } // Any output here interferes with Cargo's parsing of other printed output NativeStaticLibs => {} diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 593983f117d03..261d3529b141e 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -710,8 +710,14 @@ impl ExternEntry { } } +#[derive(Clone, PartialEq, Debug)] +pub struct PrintRequest { + pub kind: PrintKind, + pub out: OutFileName, +} + #[derive(Copy, Clone, PartialEq, Eq, Debug)] -pub enum PrintRequest { +pub enum PrintKind { FileNames, Sysroot, TargetLibdir, @@ -2091,41 +2097,41 @@ fn collect_print_requests( ) -> Vec { let mut prints = Vec::::new(); if cg.target_cpu.as_ref().is_some_and(|s| s == "help") { - prints.push(PrintRequest::TargetCPUs); + prints.push(PrintRequest { kind: PrintKind::TargetCPUs, out: OutFileName::Stdout }); cg.target_cpu = None; }; if cg.target_feature == "help" { - prints.push(PrintRequest::TargetFeatures); + prints.push(PrintRequest { kind: PrintKind::TargetFeatures, out: OutFileName::Stdout }); cg.target_feature = String::new(); } - const PRINT_REQUESTS: &[(&str, PrintRequest)] = &[ - ("crate-name", PrintRequest::CrateName), - ("file-names", PrintRequest::FileNames), - ("sysroot", PrintRequest::Sysroot), - ("target-libdir", PrintRequest::TargetLibdir), - ("cfg", PrintRequest::Cfg), - ("calling-conventions", PrintRequest::CallingConventions), - ("target-list", PrintRequest::TargetList), - ("target-cpus", PrintRequest::TargetCPUs), - ("target-features", PrintRequest::TargetFeatures), - ("relocation-models", PrintRequest::RelocationModels), - ("code-models", PrintRequest::CodeModels), - ("tls-models", PrintRequest::TlsModels), - ("native-static-libs", PrintRequest::NativeStaticLibs), - ("stack-protector-strategies", PrintRequest::StackProtectorStrategies), - ("target-spec-json", PrintRequest::TargetSpec), - ("all-target-specs-json", PrintRequest::AllTargetSpecs), - ("link-args", PrintRequest::LinkArgs), - ("split-debuginfo", PrintRequest::SplitDebuginfo), - ("deployment-target", PrintRequest::DeploymentTarget), + const PRINT_KINDS: &[(&str, PrintKind)] = &[ + ("crate-name", PrintKind::CrateName), + ("file-names", PrintKind::FileNames), + ("sysroot", PrintKind::Sysroot), + ("target-libdir", PrintKind::TargetLibdir), + ("cfg", PrintKind::Cfg), + ("calling-conventions", PrintKind::CallingConventions), + ("target-list", PrintKind::TargetList), + ("target-cpus", PrintKind::TargetCPUs), + ("target-features", PrintKind::TargetFeatures), + ("relocation-models", PrintKind::RelocationModels), + ("code-models", PrintKind::CodeModels), + ("tls-models", PrintKind::TlsModels), + ("native-static-libs", PrintKind::NativeStaticLibs), + ("stack-protector-strategies", PrintKind::StackProtectorStrategies), + ("target-spec-json", PrintKind::TargetSpec), + ("all-target-specs-json", PrintKind::AllTargetSpecs), + ("link-args", PrintKind::LinkArgs), + ("split-debuginfo", PrintKind::SplitDebuginfo), + ("deployment-target", PrintKind::DeploymentTarget), ]; prints.extend(matches.opt_strs("print").into_iter().map(|req| { - match PRINT_REQUESTS.iter().find(|&&(name, _)| name == req) { - Some((_, PrintRequest::TargetSpec)) => { + let kind = match PRINT_KINDS.iter().find(|&&(name, _)| name == req) { + Some((_, PrintKind::TargetSpec)) => { if unstable_opts.unstable_options { - PrintRequest::TargetSpec + PrintKind::TargetSpec } else { handler.early_error( "the `-Z unstable-options` flag must also be passed to \ @@ -2133,9 +2139,9 @@ fn collect_print_requests( ); } } - Some((_, PrintRequest::AllTargetSpecs)) => { + Some((_, PrintKind::AllTargetSpecs)) => { if unstable_opts.unstable_options { - PrintRequest::AllTargetSpecs + PrintKind::AllTargetSpecs } else { handler.early_error( "the `-Z unstable-options` flag must also be passed to \ @@ -2143,16 +2149,17 @@ fn collect_print_requests( ); } } - Some(&(_, print_request)) => print_request, + Some(&(_, print_kind)) => print_kind, None => { let prints = - PRINT_REQUESTS.iter().map(|(name, _)| format!("`{name}`")).collect::>(); + PRINT_KINDS.iter().map(|(name, _)| format!("`{name}`")).collect::>(); let prints = prints.join(", "); handler.early_error(format!( "unknown print request `{req}`. Valid print requests are: {prints}" )); } - } + }; + PrintRequest { kind, out: OutFileName::Stdout } })); prints From f72bdb150180f3fb9ccd6447f13e9e37b3e33e79 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 17:58:46 -0700 Subject: [PATCH 08/29] Parse --print KIND=PATH command line syntax --- compiler/rustc_session/src/config.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 261d3529b141e..738625485c494 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2011,13 +2011,7 @@ fn parse_output_types( if !unstable_opts.parse_only { for list in matches.opt_strs("emit") { for output_type in list.split(',') { - let (shorthand, path) = match output_type.split_once('=') { - None => (output_type, None), - Some((shorthand, "-")) => (shorthand, Some(OutFileName::Stdout)), - Some((shorthand, path)) => { - (shorthand, Some(OutFileName::Real(PathBuf::from(path)))) - } - }; + let (shorthand, path) = split_out_file_name(output_type); let output_type = OutputType::from_shorthand(shorthand).unwrap_or_else(|| { handler.early_error(format!( "unknown emission type: `{shorthand}` - expected one of: {display}", @@ -2034,6 +2028,14 @@ fn parse_output_types( OutputTypes(output_types) } +fn split_out_file_name(arg: &str) -> (&str, Option) { + match arg.split_once('=') { + None => (arg, None), + Some((kind, "-")) => (kind, Some(OutFileName::Stdout)), + Some((kind, path)) => (kind, Some(OutFileName::Real(PathBuf::from(path)))), + } +} + fn should_override_cgus_and_disable_thinlto( handler: &EarlyErrorHandler, output_types: &OutputTypes, @@ -2128,6 +2130,8 @@ fn collect_print_requests( ]; prints.extend(matches.opt_strs("print").into_iter().map(|req| { + let (req, out) = split_out_file_name(&req); + let kind = match PRINT_KINDS.iter().find(|&&(name, _)| name == req) { Some((_, PrintKind::TargetSpec)) => { if unstable_opts.unstable_options { @@ -2159,7 +2163,9 @@ fn collect_print_requests( )); } }; - PrintRequest { kind, out: OutFileName::Stdout } + + let out = out.unwrap_or(OutFileName::Stdout); + PrintRequest { kind, out } })); prints From 32cac2e0024aa4d02661859f9bfc69d33e6e8ba9 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 19:06:04 -0700 Subject: [PATCH 09/29] Disallow overlapping prints to the same location --- compiler/rustc_session/src/config.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 738625485c494..ee871dc8005cf 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2129,6 +2129,12 @@ fn collect_print_requests( ("deployment-target", PrintKind::DeploymentTarget), ]; + // We disallow reusing the same path in multiple prints, such as `--print + // cfg=output.txt --print link-args=output.txt`, because outputs are printed + // by disparate pieces of the compiler, and keeping track of which files + // need to be overwritten vs appended to is annoying. + let mut printed_paths = FxHashSet::default(); + prints.extend(matches.opt_strs("print").into_iter().map(|req| { let (req, out) = split_out_file_name(&req); @@ -2165,6 +2171,15 @@ fn collect_print_requests( }; let out = out.unwrap_or(OutFileName::Stdout); + if let OutFileName::Real(path) = &out { + if !printed_paths.insert(path.clone()) { + handler.early_error(format!( + "cannot print multiple outputs to the same path: {}", + path.display(), + )); + } + } + PrintRequest { kind, out } })); From f2e3d3fc63ea238385aecbb77f633fd451b9470a Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 22:13:08 -0700 Subject: [PATCH 10/29] Move OutFileName writing into rustc_session --- compiler/rustc_driver_impl/messages.ftl | 2 -- compiler/rustc_driver_impl/src/pretty.rs | 13 +------------ .../rustc_driver_impl/src/session_diagnostics.rs | 7 ------- compiler/rustc_session/messages.ftl | 2 ++ compiler/rustc_session/src/config.rs | 13 +++++++++++++ compiler/rustc_session/src/errors.rs | 7 +++++++ tests/ui/unpretty/avoid-crash.stderr | 2 +- 7 files changed, 24 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_driver_impl/messages.ftl b/compiler/rustc_driver_impl/messages.ftl index 22b4ec6b0d1b1..f8e25e0080086 100644 --- a/compiler/rustc_driver_impl/messages.ftl +++ b/compiler/rustc_driver_impl/messages.ftl @@ -15,5 +15,3 @@ driver_impl_rlink_rustc_version_mismatch = .rlink file was produced by rustc ver driver_impl_rlink_unable_to_read = failed to read rlink file: `{$err}` driver_impl_rlink_wrong_file_type = The input does not look like a .rlink file - -driver_impl_unpretty_dump_fail = pretty-print failed to write `{$path}` due to error `{$err}` diff --git a/compiler/rustc_driver_impl/src/pretty.rs b/compiler/rustc_driver_impl/src/pretty.rs index 24a5f4030b88d..222c7b5d6a72a 100644 --- a/compiler/rustc_driver_impl/src/pretty.rs +++ b/compiler/rustc_driver_impl/src/pretty.rs @@ -1,6 +1,5 @@ //! The various pretty-printing routines. -use crate::session_diagnostics::UnprettyDumpFail; use rustc_ast as ast; use rustc_ast_pretty::pprust; use rustc_errors::ErrorGuaranteed; @@ -358,17 +357,7 @@ fn get_source(sess: &Session) -> (String, FileName) { } fn write_or_print(out: &str, sess: &Session) { - match &sess.io.output_file { - None | Some(OutFileName::Stdout) => print!("{out}"), - Some(OutFileName::Real(p)) => { - if let Err(e) = std::fs::write(p, out) { - sess.emit_fatal(UnprettyDumpFail { - path: p.display().to_string(), - err: e.to_string(), - }); - } - } - } + sess.io.output_file.as_ref().unwrap_or(&OutFileName::Stdout).overwrite(out, sess); } pub fn print_after_parsing(sess: &Session, krate: &ast::Crate, ppm: PpMode) { diff --git a/compiler/rustc_driver_impl/src/session_diagnostics.rs b/compiler/rustc_driver_impl/src/session_diagnostics.rs index 638b368f70214..8e5347eba96cc 100644 --- a/compiler/rustc_driver_impl/src/session_diagnostics.rs +++ b/compiler/rustc_driver_impl/src/session_diagnostics.rs @@ -32,13 +32,6 @@ pub(crate) struct RLinkRustcVersionMismatch<'a> { #[diag(driver_impl_rlink_no_a_file)] pub(crate) struct RlinkNotAFile; -#[derive(Diagnostic)] -#[diag(driver_impl_unpretty_dump_fail)] -pub(crate) struct UnprettyDumpFail { - pub path: String, - pub err: String, -} - #[derive(Diagnostic)] #[diag(driver_impl_ice)] pub(crate) struct Ice; diff --git a/compiler/rustc_session/messages.ftl b/compiler/rustc_session/messages.ftl index 4897bd8d5daec..ee24c6d902fa5 100644 --- a/compiler/rustc_session/messages.ftl +++ b/compiler/rustc_session/messages.ftl @@ -26,6 +26,8 @@ session_feature_gate_error = {$explain} session_file_is_not_writeable = output file {$file} is not writeable -- check its permissions +session_file_write_fail = failed to write `{$path}` due to error `{$err}` + session_hexadecimal_float_literal_not_supported = hexadecimal float literal is not supported session_incompatible_linker_flavor = linker flavor `{$flavor}` is incompatible with the current target diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index ee871dc8005cf..a8147ede970c3 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -3,6 +3,7 @@ pub use crate::options::*; +use crate::errors::FileWriteFail; use crate::search_paths::SearchPath; use crate::utils::{CanonicalizedPath, NativeLib, NativeLibKind}; use crate::{lint, HashStableContext}; @@ -31,6 +32,7 @@ use std::collections::btree_map::{ use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsStr; use std::fmt; +use std::fs; use std::hash::Hash; use std::iter; use std::path::{Path, PathBuf}; @@ -861,6 +863,17 @@ impl OutFileName { OutFileName::Stdout => outputs.temp_path(flavor, codegen_unit_name), } } + + pub fn overwrite(&self, content: &str, sess: &Session) { + match self { + OutFileName::Stdout => print!("{content}"), + OutFileName::Real(path) => { + if let Err(e) = fs::write(path, content) { + sess.emit_fatal(FileWriteFail { path, err: e.to_string() }); + } + } + } + } } #[derive(Clone, Hash, Debug, HashStable_Generic)] diff --git a/compiler/rustc_session/src/errors.rs b/compiler/rustc_session/src/errors.rs index 4a3e668da111a..dd15ad45145f1 100644 --- a/compiler/rustc_session/src/errors.rs +++ b/compiler/rustc_session/src/errors.rs @@ -163,6 +163,13 @@ pub struct FileIsNotWriteable<'a> { pub file: &'a std::path::Path, } +#[derive(Diagnostic)] +#[diag(session_file_write_fail)] +pub(crate) struct FileWriteFail<'a> { + pub path: &'a std::path::Path, + pub err: String, +} + #[derive(Diagnostic)] #[diag(session_crate_name_does_not_match)] pub struct CrateNameDoesNotMatch { diff --git a/tests/ui/unpretty/avoid-crash.stderr b/tests/ui/unpretty/avoid-crash.stderr index 11cd3866fa869..15bcc277e6491 100644 --- a/tests/ui/unpretty/avoid-crash.stderr +++ b/tests/ui/unpretty/avoid-crash.stderr @@ -1,4 +1,4 @@ -error: pretty-print failed to write `/tmp/` due to $ERROR_MESSAGE +error: failed to write `/tmp/` due to $ERROR_MESSAGE error: aborting due to previous error From 5a60660ff8c35e2f270658900073443bbb984180 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 22:29:05 -0700 Subject: [PATCH 11/29] Implement printing to file in print_crate_info --- compiler/rustc_driver_impl/src/lib.rs | 55 ++++++++++++++++++--------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 27dfb6c3bca60..a173102063234 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -49,6 +49,7 @@ use std::cmp::max; use std::collections::BTreeMap; use std::env; use std::ffi::OsString; +use std::fmt::Write as _; use std::fs; use std::io::{self, IsTerminal, Read, Write}; use std::panic::{self, catch_unwind}; @@ -65,6 +66,11 @@ macro do_not_use_print($($t:tt)*) { ) } +#[allow(unused_macros)] +macro do_not_use_safe_print($($t:tt)*) { + std::compile_error!("Don't use `safe_print` or `safe_println` here, use `println_info` instead") +} + // This import blocks the use of panicking `print` and `println` in all the code // below. Please use `safe_print` and `safe_println` to avoid ICE when // encountering an I/O error during print. @@ -713,6 +719,13 @@ fn print_crate_info( parse_attrs: bool, ) -> Compilation { use rustc_session::config::PrintKind::*; + + // This import prevents the following code from using the printing macros + // used by the rest of the module. Within this function, we only write to + // the output specified by `sess.io.output_file`. + #[allow(unused_imports)] + use {do_not_use_safe_print as safe_print, do_not_use_safe_print as safe_println}; + // NativeStaticLibs and LinkArgs are special - printed during linking // (empty iterator returns true) if sess.opts.prints.iter().all(|p| p.kind == NativeStaticLibs || p.kind == LinkArgs) { @@ -731,17 +744,23 @@ fn print_crate_info( } else { None }; + for req in &sess.opts.prints { + let mut crate_info = String::new(); + macro println_info($($arg:tt)*) { + crate_info.write_fmt(format_args!("{}\n", format_args!($($arg)*))).unwrap() + } + match req.kind { TargetList => { let mut targets = rustc_target::spec::TARGETS.to_vec(); targets.sort_unstable(); - safe_println!("{}", targets.join("\n")); + println_info!("{}", targets.join("\n")); } - Sysroot => safe_println!("{}", sess.sysroot.display()), - TargetLibdir => safe_println!("{}", sess.target_tlib_path.dir.display()), + Sysroot => println_info!("{}", sess.sysroot.display()), + TargetLibdir => println_info!("{}", sess.target_tlib_path.dir.display()), TargetSpec => { - safe_println!("{}", serde_json::to_string_pretty(&sess.target.to_json()).unwrap()); + println_info!("{}", serde_json::to_string_pretty(&sess.target.to_json()).unwrap()); } AllTargetSpecs => { let mut targets = BTreeMap::new(); @@ -750,7 +769,7 @@ fn print_crate_info( let target = Target::expect_builtin(&triple); targets.insert(name, target.to_json()); } - safe_println!("{}", serde_json::to_string_pretty(&targets).unwrap()); + println_info!("{}", serde_json::to_string_pretty(&targets).unwrap()); } FileNames | CrateName => { let Some(attrs) = attrs.as_ref() else { @@ -760,14 +779,14 @@ fn print_crate_info( let t_outputs = rustc_interface::util::build_output_filenames(attrs, sess); let id = rustc_session::output::find_crate_name(sess, attrs); if req.kind == CrateName { - safe_println!("{id}"); - continue; - } - let crate_types = collect_crate_types(sess, attrs); - for &style in &crate_types { - let fname = - rustc_session::output::filename_for_input(sess, style, id, &t_outputs); - safe_println!("{}", fname.as_path().file_name().unwrap().to_string_lossy()); + println_info!("{id}"); + } else { + let crate_types = collect_crate_types(sess, attrs); + for &style in &crate_types { + let fname = + rustc_session::output::filename_for_input(sess, style, id, &t_outputs); + println_info!("{}", fname.as_path().file_name().unwrap().to_string_lossy()); + } } } Cfg => { @@ -801,13 +820,13 @@ fn print_crate_info( cfgs.sort(); for cfg in cfgs { - safe_println!("{cfg}"); + println_info!("{cfg}"); } } CallingConventions => { let mut calling_conventions = rustc_target::spec::abi::all_names(); calling_conventions.sort_unstable(); - safe_println!("{}", calling_conventions.join("\n")); + println_info!("{}", calling_conventions.join("\n")); } RelocationModels | CodeModels @@ -825,7 +844,7 @@ fn print_crate_info( for split in &[Off, Packed, Unpacked] { if sess.target.options.supported_split_debuginfo.contains(split) { - safe_println!("{split}"); + println_info!("{split}"); } } } @@ -833,7 +852,7 @@ fn print_crate_info( use rustc_target::spec::current_apple_deployment_target; if sess.target.is_like_osx { - safe_println!( + println_info!( "deployment_target={}", current_apple_deployment_target(&sess.target) .expect("unknown Apple target OS") @@ -844,6 +863,8 @@ fn print_crate_info( } } } + + req.out.overwrite(&crate_info, sess); } Compilation::Stop } From c80cbe4baedfe1ef8ea6f88f3cf2f8db06c8c399 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 22:33:38 -0700 Subject: [PATCH 12/29] Implement printing to file in codegen_backend.print --- compiler/rustc_codegen_llvm/src/lib.rs | 23 ++++++++++--------- .../rustc_codegen_ssa/src/traits/backend.rs | 20 +++++++++++++++- compiler/rustc_codegen_ssa/src/traits/mod.rs | 2 +- compiler/rustc_driver_impl/src/lib.rs | 2 +- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 38a32ac845d26..3ff6641364709 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -262,10 +262,10 @@ impl CodegenBackend for LlvmCodegenBackend { |tcx, ()| llvm_util::global_llvm_features(tcx.sess, true) } - fn print(&self, req: &PrintRequest, sess: &Session) { + fn print(&self, req: &PrintRequest, out: &mut dyn PrintBackendInfo, sess: &Session) { match req.kind { PrintKind::RelocationModels => { - println!("Available relocation models:"); + writeln!(out, "Available relocation models:"); for name in &[ "static", "pic", @@ -276,26 +276,27 @@ impl CodegenBackend for LlvmCodegenBackend { "ropi-rwpi", "default", ] { - println!(" {}", name); + writeln!(out, " {}", name); } - println!(); + writeln!(out); } PrintKind::CodeModels => { - println!("Available code models:"); + writeln!(out, "Available code models:"); for name in &["tiny", "small", "kernel", "medium", "large"] { - println!(" {}", name); + writeln!(out, " {}", name); } - println!(); + writeln!(out); } PrintKind::TlsModels => { - println!("Available TLS models:"); + writeln!(out, "Available TLS models:"); for name in &["global-dynamic", "local-dynamic", "initial-exec", "local-exec"] { - println!(" {}", name); + writeln!(out, " {}", name); } - println!(); + writeln!(out); } PrintKind::StackProtectorStrategies => { - println!( + writeln!( + out, r#"Available stack protector strategies: all Generate stack canaries in all functions. diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index f10bbbeb97cc2..1991b55f19141 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -23,6 +23,8 @@ use rustc_span::symbol::Symbol; use rustc_target::abi::call::FnAbi; use rustc_target::spec::Target; +use std::fmt; + pub trait BackendTypes { type Value: CodegenObject; type Function: CodegenObject; @@ -61,7 +63,7 @@ pub trait CodegenBackend { fn locale_resource(&self) -> &'static str; fn init(&self, _sess: &Session) {} - fn print(&self, _req: &PrintRequest, _sess: &Session) {} + fn print(&self, _req: &PrintRequest, _out: &mut dyn PrintBackendInfo, _sess: &Session) {} fn target_features(&self, _sess: &Session, _allow_unstable: bool) -> Vec { vec![] } @@ -162,3 +164,19 @@ pub trait ExtraBackendMethods: std::thread::Builder::new().name(name).spawn(f) } } + +pub trait PrintBackendInfo { + fn infallible_write_fmt(&mut self, args: fmt::Arguments<'_>); +} + +impl PrintBackendInfo for String { + fn infallible_write_fmt(&mut self, args: fmt::Arguments<'_>) { + fmt::Write::write_fmt(self, args).unwrap(); + } +} + +impl dyn PrintBackendInfo + '_ { + pub fn write_fmt(&mut self, args: fmt::Arguments<'_>) { + self.infallible_write_fmt(args); + } +} diff --git a/compiler/rustc_codegen_ssa/src/traits/mod.rs b/compiler/rustc_codegen_ssa/src/traits/mod.rs index 8cb58bd4c704d..0b69df33d2645 100644 --- a/compiler/rustc_codegen_ssa/src/traits/mod.rs +++ b/compiler/rustc_codegen_ssa/src/traits/mod.rs @@ -30,7 +30,7 @@ mod write; pub use self::abi::AbiBuilderMethods; pub use self::asm::{AsmBuilderMethods, AsmMethods, GlobalAsmOperandRef, InlineAsmOperandRef}; -pub use self::backend::{Backend, BackendTypes, CodegenBackend, ExtraBackendMethods}; +pub use self::backend::{Backend, BackendTypes, CodegenBackend, ExtraBackendMethods, PrintBackendInfo}; pub use self::builder::{BuilderMethods, OverflowOp}; pub use self::consts::ConstMethods; pub use self::coverageinfo::CoverageInfoBuilderMethods; diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index a173102063234..9e1d7499a3524 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -834,7 +834,7 @@ fn print_crate_info( | TargetCPUs | StackProtectorStrategies | TargetFeatures => { - codegen_backend.print(req, sess); + codegen_backend.print(req, &mut crate_info, sess); } // Any output here interferes with Cargo's parsing of other printed output NativeStaticLibs => {} From 6e734fce633fa5de1140f8e5d1dc981c40be1f92 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 13 Jul 2023 16:54:25 -0700 Subject: [PATCH 13/29] Implement printing to file in llvm_util --- compiler/rustc_codegen_llvm/src/lib.rs | 2 +- compiler/rustc_codegen_llvm/src/llvm_util.rs | 25 ++++++++++---------- compiler/rustc_codegen_ssa/src/traits/mod.rs | 4 +++- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 3ff6641364709..50cc44bfbb450 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -320,7 +320,7 @@ impl CodegenBackend for LlvmCodegenBackend { "# ); } - _other => llvm_util::print(req, sess), + _other => llvm_util::print(req, out, sess), } } diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 84d9acf45de57..844eb58333fc4 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -8,6 +8,7 @@ use libc::c_int; use rustc_codegen_ssa::target_features::{ supported_target_features, tied_target_features, RUSTC_SPECIFIC_FEATURES, }; +use rustc_codegen_ssa::traits::PrintBackendInfo; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::small_c_str::SmallCStr; use rustc_fs_util::path_to_c_string; @@ -350,7 +351,7 @@ fn llvm_target_features(tm: &llvm::TargetMachine) -> Vec<(&str, &str)> { ret } -fn print_target_features(sess: &Session, tm: &llvm::TargetMachine) { +fn print_target_features(out: &mut dyn PrintBackendInfo, sess: &Session, tm: &llvm::TargetMachine) { let mut llvm_target_features = llvm_target_features(tm); let mut known_llvm_target_features = FxHashSet::<&'static str>::default(); let mut rustc_target_features = supported_target_features(sess) @@ -383,24 +384,24 @@ fn print_target_features(sess: &Session, tm: &llvm::TargetMachine) { .max() .unwrap_or(0); - println!("Features supported by rustc for this target:"); + writeln!(out, "Features supported by rustc for this target:"); for (feature, desc) in &rustc_target_features { - println!(" {1:0$} - {2}.", max_feature_len, feature, desc); + writeln!(out, " {1:0$} - {2}.", max_feature_len, feature, desc); } - println!("\nCode-generation features supported by LLVM for this target:"); + writeln!(out, "\nCode-generation features supported by LLVM for this target:"); for (feature, desc) in &llvm_target_features { - println!(" {1:0$} - {2}.", max_feature_len, feature, desc); + writeln!(out, " {1:0$} - {2}.", max_feature_len, feature, desc); } if llvm_target_features.is_empty() { - println!(" Target features listing is not supported by this LLVM version."); + writeln!(out, " Target features listing is not supported by this LLVM version."); } - println!("\nUse +feature to enable a feature, or -feature to disable it."); - println!("For example, rustc -C target-cpu=mycpu -C target-feature=+feature1,-feature2\n"); - println!("Code-generation features cannot be used in cfg or #[target_feature],"); - println!("and may be renamed or removed in a future version of LLVM or rustc.\n"); + writeln!(out, "\nUse +feature to enable a feature, or -feature to disable it."); + writeln!(out, "For example, rustc -C target-cpu=mycpu -C target-feature=+feature1,-feature2\n"); + writeln!(out, "Code-generation features cannot be used in cfg or #[target_feature],"); + writeln!(out, "and may be renamed or removed in a future version of LLVM or rustc.\n"); } -pub(crate) fn print(req: &PrintRequest, sess: &Session) { +pub(crate) fn print(req: &PrintRequest, out: &mut dyn PrintBackendInfo, sess: &Session) { require_inited(); let tm = create_informational_target_machine(sess); match req.kind { @@ -412,7 +413,7 @@ pub(crate) fn print(req: &PrintRequest, sess: &Session) { .unwrap_or_else(|e| bug!("failed to convert to cstring: {}", e)); unsafe { llvm::LLVMRustPrintTargetCPUs(tm, cpu_cstring.as_ptr()) }; } - PrintKind::TargetFeatures => print_target_features(sess, tm), + PrintKind::TargetFeatures => print_target_features(out, sess, tm), _ => bug!("rustc_codegen_llvm can't handle print request: {:?}", req), } } diff --git a/compiler/rustc_codegen_ssa/src/traits/mod.rs b/compiler/rustc_codegen_ssa/src/traits/mod.rs index 0b69df33d2645..728c2bc8c49bc 100644 --- a/compiler/rustc_codegen_ssa/src/traits/mod.rs +++ b/compiler/rustc_codegen_ssa/src/traits/mod.rs @@ -30,7 +30,9 @@ mod write; pub use self::abi::AbiBuilderMethods; pub use self::asm::{AsmBuilderMethods, AsmMethods, GlobalAsmOperandRef, InlineAsmOperandRef}; -pub use self::backend::{Backend, BackendTypes, CodegenBackend, ExtraBackendMethods, PrintBackendInfo}; +pub use self::backend::{ + Backend, BackendTypes, CodegenBackend, ExtraBackendMethods, PrintBackendInfo, +}; pub use self::builder::{BuilderMethods, OverflowOp}; pub use self::consts::ConstMethods; pub use self::coverageinfo::CoverageInfoBuilderMethods; From 815a114974debd5b9c6a19a8d94f96b675a27347 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 13 Jul 2023 16:56:29 -0700 Subject: [PATCH 14/29] Implement printing to file in PassWrapper --- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 7 +++- compiler/rustc_codegen_llvm/src/llvm_util.rs | 18 +++++++++-- .../rustc_llvm/llvm-wrapper/PassWrapper.cpp | 32 +++++++++++++------ 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 605f0154773a7..03b5b45d9a3db 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2280,7 +2280,12 @@ extern "C" { pub fn LLVMRustHasFeature(T: &TargetMachine, s: *const c_char) -> bool; - pub fn LLVMRustPrintTargetCPUs(T: &TargetMachine, cpu: *const c_char); + pub fn LLVMRustPrintTargetCPUs( + T: &TargetMachine, + cpu: *const c_char, + print: unsafe extern "C" fn(out: *mut c_void, string: *const c_char, len: usize), + out: *mut c_void, + ); pub fn LLVMRustGetTargetFeaturesCount(T: &TargetMachine) -> size_t; pub fn LLVMRustGetTargetFeature( T: &TargetMachine, diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 844eb58333fc4..8d57f59764fbd 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -17,8 +17,8 @@ use rustc_session::config::{PrintKind, PrintRequest}; use rustc_session::Session; use rustc_span::symbol::Symbol; use rustc_target::spec::{MergeFunctions, PanicStrategy}; -use std::ffi::{CStr, CString}; +use std::ffi::{c_char, c_void, CStr, CString}; use std::path::Path; use std::ptr; use std::slice; @@ -401,7 +401,7 @@ fn print_target_features(out: &mut dyn PrintBackendInfo, sess: &Session, tm: &ll writeln!(out, "and may be renamed or removed in a future version of LLVM or rustc.\n"); } -pub(crate) fn print(req: &PrintRequest, out: &mut dyn PrintBackendInfo, sess: &Session) { +pub(crate) fn print(req: &PrintRequest, mut out: &mut dyn PrintBackendInfo, sess: &Session) { require_inited(); let tm = create_informational_target_machine(sess); match req.kind { @@ -411,7 +411,19 @@ pub(crate) fn print(req: &PrintRequest, out: &mut dyn PrintBackendInfo, sess: &S // at least as long as the C function let cpu_cstring = CString::new(handle_native(sess.target.cpu.as_ref())) .unwrap_or_else(|e| bug!("failed to convert to cstring: {}", e)); - unsafe { llvm::LLVMRustPrintTargetCPUs(tm, cpu_cstring.as_ptr()) }; + unsafe extern "C" fn callback(out: *mut c_void, string: *const c_char, len: usize) { + let out = &mut *(out as *mut &mut dyn PrintBackendInfo); + let bytes = slice::from_raw_parts(string as *const u8, len); + write!(out, "{}", String::from_utf8_lossy(bytes)); + } + unsafe { + llvm::LLVMRustPrintTargetCPUs( + tm, + cpu_cstring.as_ptr(), + callback, + &mut out as *mut &mut dyn PrintBackendInfo as *mut c_void, + ); + } } PrintKind::TargetFeatures => print_target_features(out, sess, tm), _ => bug!("rustc_codegen_llvm can't handle print request: {:?}", req), diff --git a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp index eb3d67e720f2d..e5fb6b0953f5a 100644 --- a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -306,44 +307,55 @@ static size_t getLongestEntryLength(ArrayRef Table) { return MaxLen; } -extern "C" void LLVMRustPrintTargetCPUs(LLVMTargetMachineRef TM, const char* TargetCPU) { +using PrintBackendInfo = void(void*, const char* Data, size_t Len); + +extern "C" void LLVMRustPrintTargetCPUs(LLVMTargetMachineRef TM, + const char* TargetCPU, + PrintBackendInfo Print, + void* Out) { const TargetMachine *Target = unwrap(TM); const MCSubtargetInfo *MCInfo = Target->getMCSubtargetInfo(); const Triple::ArchType HostArch = Triple(sys::getDefaultTargetTriple()).getArch(); const Triple::ArchType TargetArch = Target->getTargetTriple().getArch(); + std::ostringstream Buf; + #if LLVM_VERSION_GE(17, 0) const ArrayRef CPUTable = MCInfo->getAllProcessorDescriptions(); #elif defined(LLVM_RUSTLLVM) const ArrayRef CPUTable = MCInfo->getCPUTable(); #else - printf("Full target CPU help is not supported by this LLVM version.\n\n"); + Buf << "Full target CPU help is not supported by this LLVM version.\n\n"; SubtargetSubTypeKV TargetCPUKV = { TargetCPU, {{}}, {{}} }; const ArrayRef CPUTable = TargetCPUKV; #endif unsigned MaxCPULen = getLongestEntryLength(CPUTable); - printf("Available CPUs for this target:\n"); + Buf << "Available CPUs for this target:\n"; // Don't print the "native" entry when the user specifies --target with a // different arch since that could be wrong or misleading. if (HostArch == TargetArch) { MaxCPULen = std::max(MaxCPULen, (unsigned) std::strlen("native")); const StringRef HostCPU = sys::getHostCPUName(); - printf(" %-*s - Select the CPU of the current host (currently %.*s).\n", - MaxCPULen, "native", (int)HostCPU.size(), HostCPU.data()); + Buf << " " << std::left << std::setw(MaxCPULen) << "native" + << " - Select the CPU of the current host " + "(currently " << HostCPU.str() << ").\n"; } for (auto &CPU : CPUTable) { // Compare cpu against current target to label the default if (strcmp(CPU.Key, TargetCPU) == 0) { - printf(" %-*s - This is the default target CPU" - " for the current build target (currently %s).", - MaxCPULen, CPU.Key, Target->getTargetTriple().str().c_str()); + Buf << " " << std::left << std::setw(MaxCPULen) << CPU.Key + << " - This is the default target CPU for the current build target " + "(currently " << Target->getTargetTriple().str() << ")."; } else { - printf(" %-*s", MaxCPULen, CPU.Key); + Buf << " " << CPU.Key; } - printf("\n"); + Buf << "\n"; } + + const auto &BufString = Buf.str(); + Print(Out, BufString.data(), BufString.size()); } extern "C" size_t LLVMRustGetTargetFeaturesCount(LLVMTargetMachineRef TM) { From dcfe94a009369734c9c84930f42afc1f6d17998d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 23:08:04 -0700 Subject: [PATCH 15/29] Implement printing to file for link-args and native-static-libs --- compiler/rustc_codegen_ssa/src/back/link.rs | 32 ++++++++++++++------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index bb8a520321a11..ecd58541e992e 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -12,7 +12,7 @@ use rustc_metadata::fs::{copy_to_stdout, emit_wrapper_file, METADATA_FILENAME}; use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile; use rustc_middle::middle::dependency_format::Linkage; use rustc_middle::middle::exported_symbols::SymbolExportKind; -use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, Strip}; +use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, OutFileName, Strip}; use rustc_session::config::{OutputFilenames, OutputType, PrintKind, SplitDwarfKind}; use rustc_session::cstore::DllImport; use rustc_session::output::{check_file_is_writeable, invalid_output_for_target, out_filename}; @@ -596,8 +596,10 @@ fn link_staticlib<'a>( all_native_libs.extend_from_slice(&codegen_results.crate_info.used_libraries); - if sess.opts.prints.iter().any(|print| print.kind == PrintKind::NativeStaticLibs) { - print_native_static_libs(sess, &all_native_libs, &all_rust_dylibs); + for print in &sess.opts.prints { + if print.kind == PrintKind::NativeStaticLibs { + print_native_static_libs(sess, &print.out, &all_native_libs, &all_rust_dylibs); + } } Ok(()) @@ -744,8 +746,11 @@ fn link_natively<'a>( cmd.env_remove(k.as_ref()); } - if sess.opts.prints.iter().any(|print| print.kind == PrintKind::LinkArgs) { - println!("{:?}", &cmd); + for print in &sess.opts.prints { + if print.kind == PrintKind::LinkArgs { + let content = format!("{:?}", cmd); + print.out.overwrite(&content, sess); + } } // May have not found libraries in the right formats. @@ -1386,6 +1391,7 @@ enum RlibFlavor { fn print_native_static_libs( sess: &Session, + out: &OutFileName, all_native_libs: &[NativeLib], all_rust_dylibs: &[&Path], ) { @@ -1459,11 +1465,17 @@ fn print_native_static_libs( lib_args.push(format!("-l{}", lib)); } } - if !lib_args.is_empty() { - sess.emit_note(errors::StaticLibraryNativeArtifacts); - // Prefix for greppability - // Note: This must not be translated as tools are allowed to depend on this exact string. - sess.note_without_error(format!("native-static-libs: {}", &lib_args.join(" "))); + + match out { + OutFileName::Real(_) => out.overwrite(&lib_args.join(" "), sess), + OutFileName::Stdout => { + if !lib_args.is_empty() { + sess.emit_note(errors::StaticLibraryNativeArtifacts); + // Prefix for greppability + // Note: This must not be translated as tools are allowed to depend on this exact string. + sess.note_without_error(format!("native-static-libs: {}", &lib_args.join(" "))); + } + } } } From 7ee059b8ac986f5c127063870336e934f898530e Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 14 Jul 2023 10:51:54 -0700 Subject: [PATCH 16/29] Add ui test of LLVM print-from-C++ changes --- tests/ui/codegen/target-cpus.rs | 4 ++++ tests/ui/codegen/target-cpus.stdout | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 tests/ui/codegen/target-cpus.rs create mode 100644 tests/ui/codegen/target-cpus.stdout diff --git a/tests/ui/codegen/target-cpus.rs b/tests/ui/codegen/target-cpus.rs new file mode 100644 index 0000000000000..1dff3ee6011bf --- /dev/null +++ b/tests/ui/codegen/target-cpus.rs @@ -0,0 +1,4 @@ +// needs-llvm-components: webassembly +// min-llvm-version: 17 +// compile-flags: --print=target-cpus --target=wasm32-unknown-unknown +// check-pass diff --git a/tests/ui/codegen/target-cpus.stdout b/tests/ui/codegen/target-cpus.stdout new file mode 100644 index 0000000000000..f60ba0f5034ba --- /dev/null +++ b/tests/ui/codegen/target-cpus.stdout @@ -0,0 +1,4 @@ +Available CPUs for this target: + bleeding-edge + generic - This is the default target CPU for the current build target (currently wasm32-unknown-unknown). + mvp From 5ca0946ac0ae7c75feb3474039c7fb9a5348601d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 23:05:12 -0700 Subject: [PATCH 17/29] Document --print KIND=PATH in Command-line Arguments documentation --- src/doc/rustc/src/command-line-arguments.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/doc/rustc/src/command-line-arguments.md b/src/doc/rustc/src/command-line-arguments.md index 2c7c05c0c4b88..4d32897cc14c7 100644 --- a/src/doc/rustc/src/command-line-arguments.md +++ b/src/doc/rustc/src/command-line-arguments.md @@ -260,6 +260,10 @@ The valid types of print values are: This returns rustc's minimum supported deployment target if no `*_DEPLOYMENT_TARGET` variable is present in the environment, or otherwise returns the variable's parsed value. +A filepath may optionally be specified for each requested information kind, in +the format `--print KIND=PATH`, just like for `--emit`. When a path is +specified, information will be written there instead of to stdout. + [conditional compilation]: ../reference/conditional-compilation.html [deployment target]: https://developer.apple.com/library/archive/documentation/DeveloperTools/Conceptual/cross_development/Configuring/configuring.html From 26fd6b15b058df4b53fd95109a92316a74e7e94a Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 19 Jul 2023 15:13:40 -0700 Subject: [PATCH 18/29] Add note about writing native-static-libs to file --- compiler/rustc_codegen_ssa/messages.ftl | 2 ++ compiler/rustc_codegen_ssa/src/back/link.rs | 7 ++++++- compiler/rustc_codegen_ssa/src/errors.rs | 6 ++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index f73080182bfcd..b6c70c6224977 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -197,6 +197,8 @@ codegen_ssa_specify_libraries_to_link = use the `-l` flag to specify native libr codegen_ssa_static_library_native_artifacts = Link against the following native artifacts when linking against this static library. The order and any duplication can be significant on some platforms. +codegen_ssa_static_library_native_artifacts_to_file = Native artifacts to link against have been written to {$path}. The order and any duplication can be significant on some platforms. + codegen_ssa_stripping_debug_info_failed = stripping debug info with `{$util}` failed: {$status} .note = {$output} diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index ecd58541e992e..eefa4ac34dde2 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1467,7 +1467,12 @@ fn print_native_static_libs( } match out { - OutFileName::Real(_) => out.overwrite(&lib_args.join(" "), sess), + OutFileName::Real(path) => { + out.overwrite(&lib_args.join(" "), sess); + if !lib_args.is_empty() { + sess.emit_note(errors::StaticLibraryNativeArtifactsToFile { path }); + } + } OutFileName::Stdout => { if !lib_args.is_empty() { sess.emit_note(errors::StaticLibraryNativeArtifacts); diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 056b4abd23533..c72d37be74815 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -455,6 +455,12 @@ pub struct LinkerFileStem; #[diag(codegen_ssa_static_library_native_artifacts)] pub struct StaticLibraryNativeArtifacts; +#[derive(Diagnostic)] +#[diag(codegen_ssa_static_library_native_artifacts_to_file)] +pub struct StaticLibraryNativeArtifactsToFile<'a> { + pub path: &'a Path, +} + #[derive(Diagnostic)] #[diag(codegen_ssa_link_script_unavailable)] pub struct LinkScriptUnavailable; From 11ae0afc931b00f921818eb99dd894768558bdcf Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 19 Jul 2023 15:15:46 -0700 Subject: [PATCH 19/29] Create separate match arms for FileNames and CrateNames This introduces a bit of code duplication, but we don't have the build_output_filenames in the CrateName arm and this seems a little cleaner overall. --- compiler/rustc_driver_impl/src/lib.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 9e1d7499a3524..f2286fef370a6 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -771,24 +771,28 @@ fn print_crate_info( } println_info!("{}", serde_json::to_string_pretty(&targets).unwrap()); } - FileNames | CrateName => { + FileNames => { let Some(attrs) = attrs.as_ref() else { // no crate attributes, print out an error and exit return Compilation::Continue; }; let t_outputs = rustc_interface::util::build_output_filenames(attrs, sess); let id = rustc_session::output::find_crate_name(sess, attrs); - if req.kind == CrateName { - println_info!("{id}"); - } else { - let crate_types = collect_crate_types(sess, attrs); - for &style in &crate_types { - let fname = - rustc_session::output::filename_for_input(sess, style, id, &t_outputs); - println_info!("{}", fname.as_path().file_name().unwrap().to_string_lossy()); - } + let crate_types = collect_crate_types(sess, attrs); + for &style in &crate_types { + let fname = + rustc_session::output::filename_for_input(sess, style, id, &t_outputs); + println_info!("{}", fname.as_path().file_name().unwrap().to_string_lossy()); } } + CrateName => { + let Some(attrs) = attrs.as_ref() else { + // no crate attributes, print out an error and exit + return Compilation::Continue; + }; + let id = rustc_session::output::find_crate_name(sess, attrs); + println_info!("{id}"); + } Cfg => { let mut cfgs = sess .parse_sess From 40e116489feabac4289f4a5d65913a28dc36927d Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 20 Jul 2023 23:05:32 +0100 Subject: [PATCH 20/29] Minor improvements to Windows TLS dtors --- .../std/src/sys/windows/thread_local_dtor.rs | 27 +---------- .../std/src/sys/windows/thread_local_key.rs | 46 ++++++++++++++++++- .../src/sys/windows/thread_local_key/tests.rs | 4 ++ 3 files changed, 49 insertions(+), 28 deletions(-) diff --git a/library/std/src/sys/windows/thread_local_dtor.rs b/library/std/src/sys/windows/thread_local_dtor.rs index 9707a95dff21b..cf542d2bfb838 100644 --- a/library/std/src/sys/windows/thread_local_dtor.rs +++ b/library/std/src/sys/windows/thread_local_dtor.rs @@ -4,29 +4,4 @@ #![unstable(feature = "thread_local_internals", issue = "none")] #![cfg(target_thread_local)] -// Using a per-thread list avoids the problems in synchronizing global state. -#[thread_local] -static mut DESTRUCTORS: Vec<(*mut u8, unsafe extern "C" fn(*mut u8))> = Vec::new(); - -// Ensure this can never be inlined because otherwise this may break in dylibs. -// See #44391. -#[inline(never)] -pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { - DESTRUCTORS.push((t, dtor)); -} - -#[inline(never)] // See comment above -/// Runs destructors. This should not be called until thread exit. -pub unsafe fn run_keyless_dtors() { - // Drop all the destructors. - // - // Note: While this is potentially an infinite loop, it *should* be - // the case that this loop always terminates because we provide the - // guarantee that a TLS key cannot be set after it is flagged for - // destruction. - while let Some((ptr, dtor)) = DESTRUCTORS.pop() { - (dtor)(ptr); - } - // We're done so free the memory. - DESTRUCTORS = Vec::new(); -} +pub use super::thread_local_key::register_keyless_dtor as register_dtor; diff --git a/library/std/src/sys/windows/thread_local_key.rs b/library/std/src/sys/windows/thread_local_key.rs index 17628b7579b8d..036d96596e9c0 100644 --- a/library/std/src/sys/windows/thread_local_key.rs +++ b/library/std/src/sys/windows/thread_local_key.rs @@ -1,7 +1,7 @@ use crate::cell::UnsafeCell; use crate::ptr; use crate::sync::atomic::{ - AtomicPtr, AtomicU32, + AtomicBool, AtomicPtr, AtomicU32, Ordering::{AcqRel, Acquire, Relaxed, Release}, }; use crate::sys::c; @@ -9,6 +9,41 @@ use crate::sys::c; #[cfg(test)] mod tests; +/// An optimization hint. The compiler is often smart enough to know if an atomic +/// is never set and can remove dead code based on that fact. +static HAS_DTORS: AtomicBool = AtomicBool::new(false); + +// Using a per-thread list avoids the problems in synchronizing global state. +#[thread_local] +#[cfg(target_thread_local)] +static mut DESTRUCTORS: Vec<(*mut u8, unsafe extern "C" fn(*mut u8))> = Vec::new(); + +// Ensure this can never be inlined because otherwise this may break in dylibs. +// See #44391. +#[inline(never)] +#[cfg(target_thread_local)] +pub unsafe fn register_keyless_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { + DESTRUCTORS.push((t, dtor)); + HAS_DTORS.store(true, Relaxed); +} + +#[inline(never)] // See comment above +#[cfg(target_thread_local)] +/// Runs destructors. This should not be called until thread exit. +unsafe fn run_keyless_dtors() { + // Drop all the destructors. + // + // Note: While this is potentially an infinite loop, it *should* be + // the case that this loop always terminates because we provide the + // guarantee that a TLS key cannot be set after it is flagged for + // destruction. + while let Some((ptr, dtor)) = DESTRUCTORS.pop() { + (dtor)(ptr); + } + // We're done so free the memory. + DESTRUCTORS = Vec::new(); +} + type Key = c::DWORD; type Dtor = unsafe extern "C" fn(*mut u8); @@ -156,6 +191,8 @@ static DTORS: AtomicPtr = AtomicPtr::new(ptr::null_mut()); /// Should only be called once per key, otherwise loops or breaks may occur in /// the linked list. unsafe fn register_dtor(key: &'static StaticKey) { + // Ensure this is never run when native thread locals are available. + assert_eq!(false, cfg!(target_thread_local)); let this = <*const StaticKey>::cast_mut(key); // Use acquire ordering to pass along the changes done by the previously // registered keys when we store the new head with release ordering. @@ -167,6 +204,7 @@ unsafe fn register_dtor(key: &'static StaticKey) { Err(new) => head = new, } } + HAS_DTORS.store(true, Release); } // ------------------------------------------------------------------------- @@ -240,10 +278,14 @@ pub static p_thread_callback: unsafe extern "system" fn(c::LPVOID, c::DWORD, c:: #[allow(dead_code, unused_variables)] unsafe extern "system" fn on_tls_callback(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID) { + if !HAS_DTORS.load(Acquire) { + return; + } if dwReason == c::DLL_THREAD_DETACH || dwReason == c::DLL_PROCESS_DETACH { + #[cfg(not(target_thread_local))] run_dtors(); #[cfg(target_thread_local)] - super::thread_local_dtor::run_keyless_dtors(); + run_keyless_dtors(); } // See comments above for what this is doing. Note that we don't need this diff --git a/library/std/src/sys/windows/thread_local_key/tests.rs b/library/std/src/sys/windows/thread_local_key/tests.rs index c95f383fb90e3..c739f0caf3ec0 100644 --- a/library/std/src/sys/windows/thread_local_key/tests.rs +++ b/library/std/src/sys/windows/thread_local_key/tests.rs @@ -1,3 +1,7 @@ +// This file only tests the thread local key fallback. +// Windows targets with native thread local support do not use this. +#![cfg(not(target_thread_local))] + use super::StaticKey; use crate::ptr; From 715efa418e6a1367e7d2125b02cda77d13a8e44f Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 28 Jun 2023 15:15:22 -0700 Subject: [PATCH 21/29] style-guide: Remove material about tool configurability The style guide discusses the default Rust style. Configurability of Rust formatting tools are not the domain of the style guide. --- src/doc/style-guide/src/README.md | 7 ------ src/doc/style-guide/src/expressions.md | 6 +---- src/doc/style-guide/src/items.md | 32 +------------------------- src/doc/style-guide/src/statements.md | 3 --- 4 files changed, 2 insertions(+), 46 deletions(-) diff --git a/src/doc/style-guide/src/README.md b/src/doc/style-guide/src/README.md index b8aa64ba14f6f..d6af3338ded41 100644 --- a/src/doc/style-guide/src/README.md +++ b/src/doc/style-guide/src/README.md @@ -40,7 +40,6 @@ options. * Each level of indentation must be four spaces (that is, all indentation outside of string literals and comments must be a multiple of four). * The maximum width for a line is 100 characters. -* A tool may choose to make some of these configurable. #### Block indent @@ -99,8 +98,6 @@ fn bar() {} fn baz() {} ``` -Formatting tools may wish to make the bounds on blank lines configurable. - ### [Module-level items](items.md) ### [Statements](statements.md) ### [Expressions](expressions.md) @@ -231,10 +228,6 @@ complexity of an item (for example, that all components must be simple names, not more complex sub-expressions). For more discussion on suitable heuristics, see [this issue](https://github.com/rust-lang-nursery/fmt-rfcs/issues/47). -Tools should give the user an option to ignore such heuristics and always use -the normal formatting. - - ## [Non-formatting conventions](advice.md) ## [Cargo.toml conventions](cargo.md) diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index 41a9eb82cb5af..7e9d71344427a 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -814,8 +814,7 @@ let arr = [combinable( )]; ``` -Such behaviour should extend recursively, however, tools may choose to limit the -depth of nesting. +Such behaviour should extend recursively. Only where the multi-line sub-expression is a closure with an explicit block, this combining behaviour may be used where there are other arguments, as long as @@ -852,9 +851,6 @@ compound expression, then use parentheses around it, e.g., `..(x + 1)`, Hexadecimal literals may use upper- or lower-case letters, but they must not be mixed within the same literal. Projects should use the same case for all literals, but we do not make a recommendation for either lower- or upper-case. -Tools should have an option to convert mixed case literals to upper-case, and -may have an option to convert all literals to either lower- or upper-case. - ## Patterns diff --git a/src/doc/style-guide/src/items.md b/src/doc/style-guide/src/items.md index de31e8a8799b3..7b5e523d66efa 100644 --- a/src/doc/style-guide/src/items.md +++ b/src/doc/style-guide/src/items.md @@ -15,9 +15,6 @@ alphabetically. When sorting, `self` and `super` must come before any other names. Module declarations should not be moved if they are annotated with `#[macro_use]`, since that may be semantics changing. -Tools should make the above ordering optional. - - ### Function definitions In Rust, people often find functions by searching for `fn [function-name]`, so @@ -372,33 +369,6 @@ where + Index, Output = Self::Output> + Index ``` -#### Option - `where_single_line` - -`where_single_line` is `false` by default. If `true`, then a where clause with -exactly one component may be formatted on a single line if the rest of the -item's signature is also kept on one line. In this case, there is no need for a -trailing comma and if followed by a block, no need for a newline before the -block. E.g., - -```rust -// May be single-lined. -fn foo(args) -> ReturnType -where T: Bound { - body -} - -// Must be multi-lined. -fn foo( - args -) -> ReturnType -where - T: Bound, -{ - body -} -``` - - ### Type aliases Type aliases should generally be kept on one line. If necessary to break the @@ -458,7 +428,7 @@ use a::b::{foo, bar, baz}; #### Large list imports Prefer to use multiple imports rather than a multi-line import. However, tools -should not split imports by default (they may offer this as an option). +should not split imports by default. If an import does require multiple lines (either because a list of single names does not fit within the max width, or because of the rules for nested imports diff --git a/src/doc/style-guide/src/statements.md b/src/doc/style-guide/src/statements.md index a5cd6da10616d..7d5d8401318d9 100644 --- a/src/doc/style-guide/src/statements.md +++ b/src/doc/style-guide/src/statements.md @@ -120,9 +120,6 @@ following are true: let Some(1) = opt else { return }; ``` -Formatters may allow users to configure the value of the threshold -used to determine whether a let-else statement is *short*. - Otherwise, the let-else statement requires some line breaks. If breaking a let-else statement across multiple lines, never break between the From cf4b20d7cc0e90843e0dc301658b163d1dc6493b Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 28 Jun 2023 18:42:01 -0700 Subject: [PATCH 22/29] style-guide: Fix typo: s/forth/fourth/g --- src/doc/style-guide/src/expressions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index 7e9d71344427a..bd3dde68163be 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -608,7 +608,7 @@ match foo { | a_very_long_pattern | another_pattern | yet_another_pattern - | a_forth_pattern => { + | a_fourth_pattern => { ... } } @@ -623,7 +623,7 @@ match foo { a_very_long_pattern | another_pattern | yet_another_pattern - | a_forth_pattern => { + | a_fourth_pattern => { ... } } From 615b58b9f987158e0d264b3f249339a9ebd70ca0 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Mon, 3 Jul 2023 16:01:35 -0700 Subject: [PATCH 23/29] style-guide: Fix an example to match the style The style guide requires a trailing comma on where clause components, but then gives an example that doesn't include one. Add the missing trailing comma. --- src/doc/style-guide/src/items.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/style-guide/src/items.md b/src/doc/style-guide/src/items.md index 7b5e523d66efa..ae8a36d68e2f3 100644 --- a/src/doc/style-guide/src/items.md +++ b/src/doc/style-guide/src/items.md @@ -366,7 +366,7 @@ where + Index, Output = Self::Output> + Index, Output = Self::Output> + Index, Output = Self::Output> - + Index, Output = Self::Output> + Index + + Index, Output = Self::Output> + Index, ``` ### Type aliases From 081e15a0d81b8feba7e72bb7955543008a89bda0 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Mon, 3 Jul 2023 16:14:55 -0700 Subject: [PATCH 24/29] style-guide: Simplify the structure of a recommendation (no semantic change) Avoid putting a sentence fragment after a list; integrate it with the sentence before the list. --- src/doc/style-guide/src/items.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/doc/style-guide/src/items.md b/src/doc/style-guide/src/items.md index ae8a36d68e2f3..29a6c6d145c9c 100644 --- a/src/doc/style-guide/src/items.md +++ b/src/doc/style-guide/src/items.md @@ -491,14 +491,12 @@ example, `a::*` comes before `b::a` but `a::b` comes before `a::*`. E.g., #### Normalisation -Tools must make the following normalisations: +Tools must make the following normalisations, recursively: * `use a::self;` -> `use a;` * `use a::{};` -> (nothing) * `use a::{b};` -> `use a::b;` -And must apply these recursively. - Tools must not otherwise merge or un-merge import lists or adjust glob imports (without an explicit option). From ce5aca9f5a1726ef892f0feca34ba5454c164857 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 28 Jun 2023 16:56:53 -0700 Subject: [PATCH 25/29] style-guide: Avoid using "should" or "may" for required parts of the default style The style guide inconsistently used language like "there should be a space" or "it should be on its own line", or "may be written on a single line", for things that are required components of the default Rust style. "should" and especially "may" come across as optional. While the style guide overall now has a statement at the top that the default style itself is a *recommendation*, the *definition* of the default style should not be ambiguous about what's part of the default style. Rewrite language in the style guide to only use "should" and "may" and similar for truly optional components of the style (e.g. things a tool cannot or should not enforce in its default configuration). In their place, either use "must", or rewrite in imperative style ("put a space", "start it on the same line"). The latter also substantially reduces the use of passive voice. This is a purely editorial change, and does not affect the semantic definition of the Rust style. --- src/doc/style-guide/src/README.md | 39 ++-- src/doc/style-guide/src/expressions.md | 246 ++++++++++++++----------- src/doc/style-guide/src/items.md | 113 ++++++------ src/doc/style-guide/src/statements.md | 58 +++--- src/doc/style-guide/src/types.md | 16 +- 5 files changed, 252 insertions(+), 220 deletions(-) diff --git a/src/doc/style-guide/src/README.md b/src/doc/style-guide/src/README.md index d6af3338ded41..29204603e58d3 100644 --- a/src/doc/style-guide/src/README.md +++ b/src/doc/style-guide/src/README.md @@ -62,7 +62,8 @@ example) and less rightward drift. ### Trailing commas -Lists should have a trailing comma when followed by a newline: +In comma-separated lists of any kind, use a trailing comma when followed by a +newline: ```rust function_call( @@ -111,17 +112,17 @@ formatter might skip formatting of comments. Prefer line comments (`//`) to block comments (`/* ... */`). -When using line comments there should be a single space after the opening sigil. +When using line comments, put a single space after the opening sigil. -When using single-line block comments there should be a single space after the -opening sigil and before the closing sigil. Multi-line block comments should -have a newline after the opening sigil and before the closing sigil. +When using single-line block comments, put a single space after the opening +sigil and before the closing sigil. For multi-line block comments, put a +newline after the opening sigil, and a newline before the closing sigil. -Prefer to put a comment on its own line. Where a comment follows code, there -should be a single space before it. Where a block comment is inline, there -should be surrounding whitespace as if it were an identifier or keyword. There -should be no trailing whitespace after a comment or at the end of any line in a -multi-line comment. Examples: +Prefer to put a comment on its own line. Where a comment follows code, put a +single space before it. Where a block comment appears inline, use surrounding +whitespace as if it were an identifier or keyword. Do not include trailing +whitespace after a comment or at the end of any line in a multi-line comment. +Examples: ```rust // A comment on an item. @@ -170,7 +171,7 @@ Prefer line comments (`///`) to block comments (`/** ... */`). Prefer outer doc comments (`///` or `/** ... */`), only use inner doc comments (`//!` and `/*! ... */`) to write module-level or crate-level documentation. -Doc comments should come before attributes. +Put doc comments before attributes. ### Attributes @@ -195,18 +196,20 @@ struct CRepr { } ``` -For attributes with an equal sign, there should be a single space before and -after the `=`, e.g., `#[foo = 42]`. +For attributes with an equal sign, put a single space before and after the `=`, +e.g., `#[foo = 42]`. There must only be a single `derive` attribute. Note for tool authors: if combining multiple `derive` attributes into a single attribute, the ordering of -the derived names should be preserved. E.g., `#[derive(bar)] #[derive(foo)] -struct Baz;` should be formatted to `#[derive(bar, foo)] struct Baz;`. +the derived names must generally be preserved for correctness: +`#[derive(Foo)] #[derive(Bar)] struct Baz;` must be formatted to +`#[derive(Foo, Bar)] struct Baz;`. ### *small* items -In many places in this guide we specify that a formatter may format an item -differently if it is *small*, for example struct literals: +In many places in this guide we specify formatting that depends on a code +construct being *small*. For example, single-line vs multi-line struct +literals: ```rust // Normal formatting @@ -215,7 +218,7 @@ Foo { f2: another_expression(), } -// *small* formatting +// "small" formatting Foo { f1, f2 } ``` diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index bd3dde68163be..fec6ae5acf3d6 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -2,10 +2,13 @@ ### Blocks -A block expression should have a newline after the initial `{` and before the -terminal `}`. Any qualifier before the block (e.g., `unsafe`) should always be -on the same line as the opening brace, and separated with a single space. The -contents of the block should be block indented: +A block expression must have a newline after the initial `{` and before the +terminal `}`, unless it qualifies to be written as a single line based on +another style rule. + +A keyword before the block (such as `unsafe` or `async`) must be on the same +line as the opening brace, with a single space between the keyword and the +opening brace. Indent the contents of the block. ```rust fn block_as_stmt() { @@ -40,7 +43,7 @@ fn unsafe_block_as_stmt() { } ``` -If a block has an attribute, it should be on its own line: +If a block has an attribute, put it on its own line before the block: ```rust fn block_as_stmt() { @@ -54,18 +57,18 @@ fn block_as_stmt() { } ``` -Avoid writing comments on the same line as the braces. +Avoid writing comments on the same lines as either of the braces. -An empty block should be written as `{}`. +Write an empty block as `{}`. -A block may be written on a single line if: +Write a block on a single line if: * it is either used in expression position (not statement position) or is an unsafe block in statement position -* contains a single-line expression and no statements -* contains no comments +* it contains a single-line expression and no statements +* it contains no comments -A single line block should have spaces after the opening brace and before the +For a single-line block, put spaces after the opening brace and before the closing brace. Examples: @@ -117,9 +120,9 @@ fn main() { ### Closures Don't put any extra spaces before the first `|` (unless the closure is prefixed -by `move`); put a space between the second `|` and the expression of the -closure. Between the `|`s, you should use function definition syntax, however, -elide types where possible. +by a keyword such as `move`); put a space between the second `|` and the +expression of the closure. Between the `|`s, use function definition syntax, +but elide types where possible. Use closures without the enclosing `{}`, if possible. Add the `{}` when you have a return type, when there are statements, there are comments in the body, or the @@ -155,13 +158,14 @@ move |arg1: i32, arg2: i32| -> i32 { ### Struct literals -If a struct literal is *small* it may be formatted on a single line. If not, -each field should be on it's own, block-indented line. There should be a -trailing comma in the multi-line form only. There should be a space after the -colon only. +If a struct literal is *small*, format it on a single line, and do not use a +trailing comma. If not, split it across multiple lines, with each field on its +own block-indented line, and use a trailing comma. + +For each `field: value` entry, put a space after the colon only. -There should be a space before the opening brace. In the single-line form there -should be spaces after the opening brace and before the closing brace. +Put a space before the opening brace. In the single-line form, put spaces after +the opening brace and before the closing brace. ```rust Foo { field1, field2: 0 } @@ -172,7 +176,7 @@ let f = Foo { ``` Functional record update syntax is treated like a field, but it must never have -a trailing comma. There should be no space after `..`. +a trailing comma. Do not put a space after `..`. let f = Foo { field1, @@ -182,9 +186,13 @@ let f = Foo { ### Tuple literals -Use a single-line form where possible. There should not be spaces around the -parentheses. Where a single-line form is not possible, each element of the tuple -should be on its own block-indented line and there should be a trailing comma. +Use a single-line form where possible. Do not put spaces between the opening +parenthesis and the first element, or between the last element and the closing +parenthesis. Separate elements with a comma followed by a space. + +Where a single-line form is not possible, write the tuple across +multiple lines, with each element of the tuple on its own block-indented line, +and use a trailing comma. ```rust (a, b, c) @@ -198,14 +206,23 @@ let x = ( ### Tuple struct literals -There should be no space between the identifier and the opening parenthesis. -Otherwise, follow the rules for tuple literals, e.g., `Foo(a, b)`. +Do not put space between the identifier and the opening parenthesis. Otherwise, +follow the rules for tuple literals: + +```rust +Foo(a, b, c) + +let x = Foo( + a_long_expr, + another_very_long_expr, +); +``` ### Enum literals Follow the formatting rules for the various struct literals. Prefer using the -name of the enum as a qualifying name, unless the enum is in the prelude. E.g., +name of the enum as a qualifying name, unless the enum is in the prelude: ```rust Foo::Bar(a, b) @@ -219,24 +236,29 @@ Ok(an_expr) ### Array literals -For simple array literals, avoid line breaking, no spaces around square -brackets, contents of the array should be separated by commas and spaces. If -using the repeating initialiser, there should be a space after the semicolon -only. Apply the same rules if using the `vec!` or similar macros (always use -square brackets here). Examples: +Write small array literals on a single line. Do not put spaces between the opening +square bracket and the first element, or between the last element and the closing +square bracket. Separate elements with a comma followed by a space. + +If using the repeating initializer, put a space after the semicolon +only. + +Apply the same rules if using `vec!` or similar array-like macros; always use +square brackets with such macros. Examples: ```rust fn main() { - [1, 2, 3]; - vec![a, b, c, d]; + let x = [1, 2, 3]; + let y = vec![a, b, c, d]; let a = [42; 10]; } ``` -If a line must be broken, prefer breaking only after the `;`, if possible. -Otherwise, follow the rules below for function calls. In any case, the contents -of the initialiser should be block indented and there should be line breaks -after the opening bracket and before the closing bracket: +For arrays large enough to break across lines, if using the repeating +initializer, break after the `;`, not before. Otherwise, follow the rules below +for function calls. In any case, block-indent the contents of the initializer, +and put line breaks after the opening square bracket and before the closing +square bracket: ```rust fn main() { @@ -255,11 +277,12 @@ fn main() { ### Array accesses, indexing, and slicing. -No spaces around the square brackets, avoid breaking lines if possible, never -break a line between the target expression and the opening bracket. If the -indexing expression covers multiple lines, then it should be block indented and -there should be newlines after the opening brackets and before the closing -bracket. However, this should be avoided where possible. +Don't put spaces around the square brackets. Avoid breaking lines if possible. +Never break a line between the target expression and the opening square +bracket. If the indexing expression must be broken onto a subsequent line, or +spans multiple lines itself, then block-indent the indexing expression, and put +newlines after the opening square bracket and before the closing square +bracket: Examples: @@ -291,7 +314,7 @@ if you have `t: &T`, and `u: U`, prefer `*t op u` to `t op &u`. In general, within expressions, prefer dereferencing to taking references, unless necessary (e.g. to avoid an unnecessarily expensive operation). -Use parentheses liberally, do not necessarily elide them due to precedence. +Use parentheses liberally; do not necessarily elide them due to precedence. Tools should not automatically insert or remove parentheses. Do not use spaces to indicate precedence. @@ -353,10 +376,10 @@ foo(x, y, z) #### Multi-line calls If the function call is not *small*, it would otherwise over-run the max width, -or any argument or the callee is multi-line, then the call should be formatted -across multiple lines. In this case, each argument should be on it's own block- -indented line, there should be a newline after the opening parenthesis and -before the closing parenthesis, and there should be a trailing comma. E.g., +or any argument or the callee is multi-line, then format the call across +multiple lines. In this case, put each argument on its own block-indented line, +break after the opening parenthesis and before the closing parenthesis, +and use a trailing comma: ```rust a_function_call( @@ -379,17 +402,18 @@ x.foo().bar().baz(x, y, z); ### Macro uses -Macros which can be parsed like other constructs should be formatted like those +If a macro can be parsed like other constructs, format it like those constructs. For example, a macro use `foo!(a, b, c)` can be parsed like a -function call (ignoring the `!`), therefore it should be formatted following the -rules for function calls. +function call (ignoring the `!`), so format it using the rules for function +calls. #### Special case macros -Macros which take a format string and where all other arguments are *small* may -be formatted with arguments before and after the format string on a single line -and the format string on its own line, rather than putting each argument on its -own line. For example, +For macros which take a format string, if all other arguments are *small*, +format the arguments before the format string on a single line if they fit, and +format the arguments after the format string on a single line if they fit, with +the format string on its own line. If the arguments are not small or do not +fit, put each on its own line as with a function. For example: ```rust println!( @@ -416,13 +440,13 @@ let cstr = "Hi\0" as *const str as *const [u8] as *const std::os::raw::c_char; ### Chains of fields and method calls -A chain is a sequence of field accesses and/or method calls. A chain may also -include the try operator ('?'). E.g., `a.b.c().d` or `foo?.bar().baz?`. +A chain is a sequence of field accesses, method calls, and/or uses of the try +operator `?`. E.g., `a.b.c().d` or `foo?.bar().baz?`. -Prefer formatting on one line if possible, and the chain is *small*. If -formatting on multiple lines, each field access or method call in the chain -should be on its own line with the line-break before the `.` and after any `?`. -Each line should be block-indented. E.g., +Format the chain on one line if it is "small" and otherwise possible to do so. +If formatting on multiple lines, put each field access or method call in the +chain on its own line, with the line-break before the `.` and after any `?`. +Block-indent each subsequent line: ```rust let foo = bar @@ -431,8 +455,8 @@ let foo = bar ``` If the length of the last line of the first element plus its indentation is -less than or equal to the indentation of the second line (and there is space), -then combine the first and second lines, e.g., +less than or equal to the indentation of the second line, then combine the +first and second lines if they fit: ```rust x.baz? @@ -489,13 +513,13 @@ self.pre_comment.as_ref().map_or( This section covers `if`, `if let`, `loop`, `while`, `while let`, and `for` expressions. -The keyword, any initial clauses, and the opening brace of the block should be -on a single line. The usual rules for [block formatting](#blocks) should be -applied to the block. +Put the keyword, any initial clauses, and the opening brace of the block all on +a single line, if they fit. Apply the usual rules for [block +formatting](#blocks) to the block. -If there is an `else` component, then the closing brace, `else`, any following -clause, and the opening brace should all be on the same line. There should be a -single space before and after the `else` keyword. For example: +If there is an `else` component, then put the closing brace, `else`, any +following clause, and the opening brace all on the same line, with a single +space before and after the `else` keyword: ```rust if ... { @@ -513,10 +537,10 @@ if let ... { } ``` -If the control line needs to be broken, then prefer to break before the `=` in -`* let` expressions and before `in` in a `for` expression; the following line -should be block indented. If the control line is broken for any reason, then the -opening brace should be on its own line and not indented. Examples: +If the control line needs to be broken, prefer to break before the `=` in `* +let` expressions and before `in` in a `for` expression; block-indent the +following line. If the control line is broken for any reason, put the opening +brace on its own line, not indented. Examples: ```rust while let Some(foo) @@ -539,10 +563,10 @@ if a_long_expression } ``` -Where the initial clause is multi-lined and ends with one or more closing -parentheses, square brackets, or braces, and there is nothing else on that line, -and that line is not indented beyond the indent on the first line of the control -flow expression, then the opening brace of the block should be put on the same +Where the initial clause spans multiple lines and ends with one or more closing +parentheses, square brackets, or braces, and there is nothing else on that +line, and that line is not indented beyond the indent on the first line of the +control flow expression, then put the opening brace of the block on the same line with a preceding space. For example: ```rust @@ -558,9 +582,9 @@ if !self.config.file_lines().intersects( #### Single line `if else` -Formatters may place an `if else` or `if let else` on a single line if it occurs -in expression context (i.e., is not a standalone statement), it contains a -single `else` clause, and is *small*. For example: +Put an `if else` or `if let else` on a single line if it occurs in expression +context (i.e., is not a standalone statement), it contains a single `else` +clause, and is *small*: ```rust let y = if x { 0 } else { 1 }; @@ -582,9 +606,9 @@ if x { ### Match -Prefer not to line-break inside the discriminant expression. There must always -be a line break after the opening brace and before the closing brace. The match -arms must be block indented once: +Prefer not to line-break inside the discriminant expression. Always break after +the opening brace and before the closing brace. Block-indent the match arms +once: ```rust match foo { @@ -598,7 +622,7 @@ let x = match foo.bar.baz() { Use a trailing comma for a match arm if and only if not using a block. -Never start a match arm pattern with `|`, e.g., +Never start a match arm pattern with `|`: ```rust match foo { @@ -614,8 +638,7 @@ match foo { } ``` -Prefer - +Prefer: ```rust match foo { @@ -633,11 +656,11 @@ Avoid splitting the left-hand side (before the `=>`) of a match arm where possible. If the right-hand side of the match arm is kept on the same line, never use a block (unless the block is empty). -If the right-hand side consists of multiple statements or has line comments or -the start of the line cannot be fit on the same line as the left-hand side, use -a block. +If the right-hand side consists of multiple statements, or has line comments, +or the start of the line does not fit on the same line as the left-hand side, +use a block. -The body of a block arm should be block indented once. +Block-indent the body of a block arm. Examples: @@ -662,8 +685,8 @@ match foo { ``` If the body is a single expression with no line comments and not a control flow -expression, then it may be started on the same line as the right-hand side. If -not, then it must be in a block. Example, +expression, start it on the same line as the right-hand side. If not, then it +must be in a block. Example: ```rust match foo { @@ -687,8 +710,8 @@ match foo { #### Line-breaking -Where it is possible to use a block form on the right-hand side and avoid -breaking the left-hand side, do that. E.g. +If using a block form on the right-hand side of a match arm makes it possible +to avoid breaking on the left-hand side, do that: ```rust // Assuming the following line does not fit in the max width @@ -720,7 +743,7 @@ body on a new line: If required to break the pattern, put each clause of the pattern on its own line with no additional indent, breaking before the `|`. If there is an `if` -clause, then you must use the above form: +clause, use the above form: ```rust a_very_long_pattern @@ -740,7 +763,7 @@ clause, then you must use the above form: ``` If the pattern is multi-line, and the last line is less wide than the indent, do -not put the `if` clause on a newline. E.g., +not put the `if` clause on a new line. E.g., ```rust Token::Dimension { @@ -753,8 +776,8 @@ not put the `if` clause on a newline. E.g., ``` If every clause in a pattern is *small*, but the whole pattern does not fit on -one line, then the pattern may be formatted across multiple lines with as many -clauses per line as possible. Again break before a `|`: +one line, then format the pattern across multiple lines with as many clauses +per line as possible. Again, break before a `|`: ```rust foo | bar | baz @@ -783,8 +806,8 @@ E.g., `&&Some(foo)` matches, `Foo(4, Bar)` does not. ### Combinable expressions Where a function call has a single argument, and that argument is formatted -across multiple-lines, the outer call may be formatted as if it were a single- -line call. The same combining behaviour may be applied to any similar +across multiple-lines, format the outer call as if it were a single-line call, +if the result fits. Apply the same combining behaviour to any similar expressions which have multi-line, block-indented lists of sub-expressions delimited by parentheses (e.g., macros or tuple struct literals). E.g., @@ -814,12 +837,12 @@ let arr = [combinable( )]; ``` -Such behaviour should extend recursively. +Apply this behavior recursively. -Only where the multi-line sub-expression is a closure with an explicit block, -this combining behaviour may be used where there are other arguments, as long as -all the arguments and the first line of the closure fit on the first line, the -closure is the last argument, and there is only one closure argument: +For a function with multiple arguments, if the last argument is a multi-line +closure with an explicit block, there are no other closure arguments, and all +the arguments and the first line of the closure fit on the first line, use the +same combining behavior: ```rust foo(first_arg, x, |param| { @@ -834,16 +857,17 @@ foo(first_arg, x, |param| { Do not put spaces in ranges, e.g., `0..10`, `x..=y`, `..x.len()`, `foo..`. When writing a range with both upper and lower bounds, if the line must be -broken, break before the range operator and block indent the second line: +broken within the range, break before the range operator and block indent the +second line: ```rust a_long_expression ..another_long_expression ``` -For the sake of indicating precedence, we recommend that if either bound is a -compound expression, then use parentheses around it, e.g., `..(x + 1)`, -`(x.f)..(x.f.len())`, or `0..(x - 10)`. +For the sake of indicating precedence, if either bound is a compound +expression, use parentheses around it, e.g., `..(x + 1)`, `(x.f)..(x.f.len())`, +or `0..(x - 10)`. ### Hexadecimal literals @@ -854,5 +878,5 @@ literals, but we do not make a recommendation for either lower- or upper-case. ## Patterns -Patterns should be formatted like their corresponding expressions. See the -section on `match` for additional formatting for patterns in match arms. +Format patterns like their corresponding expressions. See the section on +`match` for additional formatting for patterns in match arms. diff --git a/src/doc/style-guide/src/items.md b/src/doc/style-guide/src/items.md index 29a6c6d145c9c..1e7a88065b5ee 100644 --- a/src/doc/style-guide/src/items.md +++ b/src/doc/style-guide/src/items.md @@ -9,16 +9,17 @@ an item appears at module level or within another item. alphabetically. `use` statements, and module *declarations* (`mod foo;`, not `mod { ... }`) -must come before other items. We recommend that imports come before module -declarations; if imports and modules are separated, then they should be ordered -alphabetically. When sorting, `self` and `super` must come before any other -names. Module declarations should not be moved if they are annotated with -`#[macro_use]`, since that may be semantics changing. +must come before other items. Put imports before module declarations. Sort each +alphabetically, except that `self` and `super` must come before any other +names. + +Don't automatically move module declarations annotated with `#[macro_use]`, +since that might change semantics. ### Function definitions In Rust, people often find functions by searching for `fn [function-name]`, so -the formatting of function definitions shold enable this. +the formatting of function definitions must enable this. The proper ordering and spacing is: @@ -80,9 +81,9 @@ enum FooBar { } ``` -If a struct variant is [*small*](index.html#small-items), it may be formatted on -one line. In this case, do not use a trailing comma for the field list, but do -put spaces around each brace: +If a struct variant is [*small*](index.html#small-items), format it on one +line. In this case, do not use a trailing comma for the field list, but do put +spaces around each brace: ```rust enum FooBar { @@ -91,9 +92,9 @@ enum FooBar { ``` In an enum with multiple struct variants, if any struct variant is written on -multiple lines, then the multi-line formatting should be used for all struct -variants. However, such a situation might be an indication that you should -factor out the fields of the variant into their own struct. +multiple lines, use the multi-line formatting for all struct variants. However, +such a situation might be an indication that you should factor out the fields +of the variant into their own struct. ### Structs and Unions @@ -140,9 +141,9 @@ union Foo { ### Tuple structs -Put the whole struct on one line if possible. Types in the parentheses should be -separated by a comma and space with no trailing comma. No spaces around the -parentheses or semicolon: +Put the whole struct on one line if possible. Separate types within the +parentheses using a comma and space. Don't use a trailing comma for a +single-line tuple struct. Don't put spaces around the parentheses or semicolon: ```rust pub struct Foo(String, u8); @@ -151,9 +152,11 @@ pub struct Foo(String, u8); Prefer unit structs to empty tuple structs (these only exist to simplify code generation), e.g., `struct Foo;` rather than `struct Foo();`. -For more than a few fields, prefer a proper struct with named fields. Given -this, a tuple struct should always fit on one line. If it does not, block format -the fields with a field on each line and a trailing comma: +For more than a few fields (in particular if the tuple struct does not fit on +one line), prefer a proper struct with named fields. + +For a multi-line tuple struct, block-format the fields with a field on each +line and a trailing comma: ```rust pub struct Foo( @@ -165,9 +168,9 @@ pub struct Foo( ### Traits -Trait items should be block-indented. If there are no items, the trait may be -formatted on a single line. Otherwise there should be line-breaks after the -opening brace and before the closing brace: +Use block-indent for trait items. If there are no items, format the trait (including its `{}`) +on a single line. Otherwise, break after the opening brace and before +the closing brace: ```rust trait Foo {} @@ -177,8 +180,8 @@ pub trait Bar { } ``` -If the trait has bounds, there should be a space after the colon but not before -and before and after each `+`, e.g., +If the trait has bounds, put a space after the colon but not before, +and put spaces around each `+`, e.g., ```rust trait Foo: Debug + Bar {} @@ -204,9 +207,9 @@ pub trait IndexRanges: ### Impls -Impl items should be block indented. If there are no items, the impl may be -formatted on a single line. Otherwise there should be line-breaks after the -opening brace and before the closing brace: +Use block-indent for impl items. If there are no items, format the impl +(including its `{}`) on a single line. Otherwise, break after the opening brace +and before the closing brace: ```rust impl Foo {} @@ -264,12 +267,12 @@ macro_rules! foo { Prefer to put a generics clause on one line. Break other parts of an item declaration rather than line-breaking a generics clause. If a generics clause is -large enough to require line-breaking, you should prefer to use a `where` clause -instead. +large enough to require line-breaking, prefer a `where` clause instead. -Do not put spaces before or after `<` nor before `>`. Only put a space after `>` -if it is followed by a word or opening brace, not an opening parenthesis. There -should be a space after each comma and no trailing comma. +Do not put spaces before or after `<` nor before `>`. Only put a space after +`>` if it is followed by a word or opening brace, not an opening parenthesis. +Put a space after each comma. Do not use a trailing comma for a single-line +generics clause. ```rust fn foo(x: Vec, y: Vec) ... @@ -277,10 +280,9 @@ fn foo(x: Vec, y: Vec) ... impl SomeType { ... ``` -If the generics clause must be formatted across multiple lines, each parameter -should have its own block-indented line, there should be newlines after the -opening bracket and before the closing bracket, and there should be a trailing -comma. +If the generics clause must be formatted across multiple lines, put each +parameter on its own block-indented line, break after the opening `<` and +before the closing `>`, and use a trailing comma. ```rust fn foo< @@ -289,8 +291,7 @@ fn foo< >(x: Vec, y: Vec) ... ``` -If an associated type is bound in a generic type, then there should be spaces on -either side of the `=`: +If an associated type is bound in a generic type, put spaces around the `=`: ```rust > @@ -303,12 +304,14 @@ Prefer to use single-letter names for generic parameters. These rules apply for `where` clauses on any item. -A `where` clause may immediately follow a closing bracket of any kind. -Otherwise, it must start a new line, with no indent. Each component of a `where` -clause must be on its own line and be block indented. There should be a trailing +If immediately following a closing bracket of any kind, write the keyword +`where` on the same line, with a space before it. + +Otherwise, put `where` on a new line at the same indentation level. Put each +component of a `where` clause on its own line, block-indented. Use a trailing comma, unless the clause is terminated with a semicolon. If the `where` clause -is followed by a block (or assignment), the block should be started on a new -line. Examples: +is followed by a block (or assignment), start that block on a new line. +Examples: ```rust fn function(args) @@ -352,12 +355,12 @@ where = Bar; ``` -If a `where` clause is very short, we recommend using an inline bound on the -type parameter. - +If a `where` clause is very short, prefer using an inline bound on the type +parameter. -If a component of a `where` clause is long, it may be broken before `+` and -further block indented. Each bound should go on its own line. E.g., +If a component of a `where` clause does not fit and contains `+`, break it +before each `+` and block-indent the continuation lines. Put each bound on its +own line. E.g., ```rust impl IndexRanges for T @@ -371,8 +374,8 @@ where ### Type aliases -Type aliases should generally be kept on one line. If necessary to break the -line, do so after the `=`; the right-hand-side should be block indented: +Keep type aliases on one line when they fit. If necessary to break the line, do +so after the `=`, and block-indent the right-hand side: ```rust pub type Foo = Bar; @@ -397,9 +400,8 @@ where ### Associated types -Associated types should follow the guidelines above for type aliases. Where an -associated type has a bound, there should be a space after the colon but not -before: +Format associated types like type aliases. Where an associated type has a +bound, put a space after the colon but not before: ```rust pub type Foo: Bar; @@ -408,15 +410,14 @@ pub type Foo: Bar; ### extern items -When writing extern items (such as `extern "C" fn`), always be explicit about -the ABI. For example, write `extern "C" fn foo ...`, not `extern fn foo ...`, or +When writing extern items (such as `extern "C" fn`), always specify the ABI. +For example, write `extern "C" fn foo ...`, not `extern fn foo ...`, or `extern "C" { ... }`. ### Imports (`use` statements) -If an import can be formatted on one line, do so. There should be no spaces -around braces. +Format imports on one line where possible. Don't put spaces around braces. ```rust use a::b::c; diff --git a/src/doc/style-guide/src/statements.md b/src/doc/style-guide/src/statements.md index 7d5d8401318d9..8c8f893fbe904 100644 --- a/src/doc/style-guide/src/statements.md +++ b/src/doc/style-guide/src/statements.md @@ -2,8 +2,8 @@ ### Let statements -There should be spaces after the `:` and on both sides of the `=` (if they are -present). No space before the semicolon. +Put a space after the `:` and on both sides of the `=` (if they are present). +Don't put a space before the semicolon. ```rust // A comment. @@ -14,19 +14,19 @@ let pattern: Type; let pattern = expr; ``` -If possible the declaration should be formatted on a single line. If this is not -possible, then try splitting after the `=`, if the declaration can fit on two -lines. The expression should be block indented. +If possible, format the declaration on a single line. If not possible, then try +splitting after the `=`, if the declaration fits on two lines. Block-indent the +expression. ```rust let pattern: Type = expr; ``` -If the first line does not fit on a single line, then split after the colon, -using block indentation. If the type covers multiple lines, even after line- -breaking after the `:`, then the first line may be placed on the same line as -the `:`, subject to the [combining rules](https://github.com/rust-lang-nursery/fmt-rfcs/issues/61) (WIP). +If the first line still does not fit on a single line, split after the `:`, and +use block indentation. If the type requires multiple lines, even after +line-breaking after the `:`, then place the first line on the same line as the +`:`, subject to the [combining rules](expressions.html#combinable-expressions). ```rust @@ -51,12 +51,12 @@ let (abcd, ``` If the expression covers multiple lines, if the first line of the expression -fits in the remaining space, it stays on the same line as the `=`, the rest of the -expression is not indented. If the first line does not fit, then it should start -on the next lines, and should be block indented. If the expression is a block -and the type or pattern cover multiple lines, then the opening brace should be -on a new line and not indented (this provides separation for the interior of the -block from the type), otherwise the opening brace follows the `=`. +fits in the remaining space, it stays on the same line as the `=`, and the rest +of the expression is not further indented. If the first line does not fit, then +put the expression on subsequent lines, block indented. If the expression is a +block and the type or pattern cover multiple lines, put the opening brace on a +new line and not indented (this provides separation for the interior of the +block from the type); otherwise, the opening brace follows the `=`. Examples: @@ -108,8 +108,8 @@ In this case, always apply the same formatting rules to the components preceding the `else` block (i.e. the `let pattern: Type = initializer_expr` portion) as described [for other let statements](#let-statements). -The entire let-else statement may be formatted on a single line if all the -following are true: +Format the entire let-else statement on a single line if all the following are +true: * the entire statement is *short* * the `else` block contains only a single-line expression and no statements @@ -154,9 +154,9 @@ before the `else`. }; ``` -If the initializer expression is multi-line, the `else` keyword and opening -brace of the block (i.e. `else {`) should be put on the same line as the end of -the initializer expression, with a space between them, if and only if all the +If the initializer expression is multi-line, put the `else` keyword and opening +brace of the block (i.e. `else {`) on the same line as the end of the +initializer expression, with a space between them, if and only if all the following are true: * The initializer expression ends with one or more closing @@ -179,9 +179,9 @@ let Some(x) = y.foo( } ``` -Otherwise, the `else` keyword and opening brace should be placed on the next -line after the end of the initializer expression, and the `else` keyword should -have the same indentation level as the `let` keyword. +Otherwise, put the `else` keyword and opening brace on the next line after the +end of the initializer expression, with the `else` keyword at the same +indentation level as the `let` keyword. For example: @@ -233,9 +233,9 @@ fn main() { ### Macros in statement position -A macro use in statement position should use parentheses or square brackets as -delimiters and should be terminated with a semicolon. There should be no spaces -between the name, `!`, the delimiters, or the `;`. +For a macro use in statement position, use parentheses or square brackets as +delimiters, and terminate it with a semicolon. Do not put spaces around the +name, `!`, the delimiters, or the `;`. ```rust // A comment. @@ -245,14 +245,14 @@ a_macro!(...); ### Expressions in statement position -There should be no space between the expression and the semicolon. +Do not put space between the expression and the semicolon. ``` ; ``` -All expressions in statement position should be terminated with a semicolon, -unless they end with a block or are used as the value for a block. +Terminate all expressions in statement position with a semicolon, unless they +end with a block or are used as the value for a block. E.g., diff --git a/src/doc/style-guide/src/types.md b/src/doc/style-guide/src/types.md index ae456ef21c8d4..15b001d4f2ea5 100644 --- a/src/doc/style-guide/src/types.md +++ b/src/doc/style-guide/src/types.md @@ -7,14 +7,14 @@ * `*const T`, `*mut T` (no space after `*`, space before type) * `&'a T`, `&T`, `&'a mut T`, `&mut T` (no space after `&`, single spaces separating other words) * `unsafe extern "C" fn<'a, 'b, 'c>(T, U, V) -> W` or `fn()` (single spaces around keywords and sigils, and after commas, no trailing commas, no spaces around brackets) -* `!` should be treated like any other type name, `Name` +* `!` gets treated like any other type name, `Name` * `(A, B, C, D)` (spaces after commas, no spaces around parens, no trailing comma unless it is a one-tuple) * ` as SomeTrait>::Foo::Bar` or `Foo::Bar` or `::Foo::Bar` (no spaces around `::` or angle brackets, single spaces around `as`) * `Foo::Bar` (spaces after commas, no trailing comma, no spaces around angle brackets) * `T + T + T` (single spaces between types, and `+`). * `impl T + T + T` (single spaces between keyword, types, and `+`). -Parentheses used in types should not be surrounded by whitespace, e.g., `(Foo)` +Do not put space around parentheses used in types, e.g., `(Foo)` ### Line breaks @@ -37,13 +37,17 @@ Foo> ``` -`[T; expr]` may be broken after the `;` if necessary. +If a type requires line-breaks in order to fit, this section outlines where to +break such types if necessary. -Function types may be broken following the rules for function declarations. +Break `[T; expr]` after the `;` if necessary. -Generic types may be broken following the rules for generics. +Break function types following the rules for function declarations. -Types with `+` may be broken after any `+` using block indent and breaking before the `+`. When breaking such a type, all `+`s should be line broken, e.g., +Break generic types following the rules for generics. + +Break types with `+` by breaking before the `+` and block-indenting the +subsequent lines. When breaking such a type, break before *every* `+`: ```rust impl Clone From 9ccc104d14a93c593b8e4e3982037623a6fe5e0d Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 5 Jul 2023 15:21:56 -0700 Subject: [PATCH 26/29] style-guide: Add an additional chaining example Make it clear the rule for stacking the second line on the first applies recursively, as long as the condition holds. --- src/doc/style-guide/src/expressions.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index fec6ae5acf3d6..633fdc909bd03 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -456,12 +456,15 @@ let foo = bar If the length of the last line of the first element plus its indentation is less than or equal to the indentation of the second line, then combine the -first and second lines if they fit: +first and second lines if they fit. Apply this rule recursively. ```rust x.baz? .qux() +x.y.z + .qux() + let foo = x .baz? .qux(); From 69d29a70daa5e8df79d2016af681b13c82f561df Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 5 Jul 2023 16:33:21 -0700 Subject: [PATCH 27/29] style-guide: Fix typo: s/right-hand side/left-hand side/ --- src/doc/style-guide/src/expressions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index 633fdc909bd03..42ecc442eb36c 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -688,7 +688,7 @@ match foo { ``` If the body is a single expression with no line comments and not a control flow -expression, start it on the same line as the right-hand side. If not, then it +expression, start it on the same line as the left-hand side. If not, then it must be in a block. Example: ```rust From 144e8a38669c874464624e4d27f418eb02f29eff Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 5 Jul 2023 16:36:47 -0700 Subject: [PATCH 28/29] style-guide: Fix example to match the rule it exemplifies (and match rustfmt) An example immediately following "Put each bound on its own line." did not put each bound on its own line. --- src/doc/style-guide/src/items.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/doc/style-guide/src/items.md b/src/doc/style-guide/src/items.md index 1e7a88065b5ee..6e5ea335e6ab2 100644 --- a/src/doc/style-guide/src/items.md +++ b/src/doc/style-guide/src/items.md @@ -369,7 +369,8 @@ where + Index, Output = Self::Output> + Index, Output = Self::Output> + Index, Output = Self::Output> - + Index, Output = Self::Output> + Index, + + Index, Output = Self::Output> + + Index, ``` ### Type aliases From 77d09cb69e7e459b6eee9d7564844c73064b2e84 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Thu, 20 Jul 2023 13:51:46 -0700 Subject: [PATCH 29/29] Clarify wording on breaking arrays across lines Co-authored-by: Caleb Cartwright --- src/doc/style-guide/src/expressions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index 42ecc442eb36c..0aed8763a0628 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -254,7 +254,7 @@ fn main() { } ``` -For arrays large enough to break across lines, if using the repeating +For arrays that have to be broken across lines, if using the repeating initializer, break after the `;`, not before. Otherwise, follow the rules below for function calls. In any case, block-indent the contents of the initializer, and put line breaks after the opening square bracket and before the closing