Auto merge of #54864 - ljedrz:cleanup_codegen_llvm_back, r=Mark-Simulacrum

Cleanup codegen_llvm/back

- improve allocations
- use `Cow<'static, str>` where applicable
- use `to_owned` instead of `to_string` with string literals
- remove a redundant `continue`
- possible minor speedup in logic
- use `mem::replace` instead of `swap` where more concise
- remove `'static` from consts
- improve common patterns
- remove explicit `return`s
- whitespace & formatting fixes

Author: bors

Diff for: src/librustc_codegen_llvm/back/archive.rs

@@ -83,15 +83,16 @@ impl<'a> ArchiveBuilder<'a> {
         if self.src_archive().is_none() {
             return Vec::new()
         }
+
         let archive = self.src_archive.as_ref().unwrap().as_ref().unwrap();
-        let ret = archive.iter()
-                         .filter_map(|child| child.ok())
-                         .filter(is_relevant_child)
-                         .filter_map(|child| child.name())
-                         .filter(|name| !self.removals.iter().any(|x| x == name))
-                         .map(|name| name.to_string())
-                         .collect();
-        return ret;
+
+        archive.iter()
+               .filter_map(|child| child.ok())
+               .filter(is_relevant_child)
+               .filter_map(|child| child.name())
+               .filter(|name| !self.removals.iter().any(|x| x == name))
+               .map(|name| name.to_owned())
+               .collect()
     }
 
     fn src_archive(&mut self) -> Option<&ArchiveRO> {
@@ -171,7 +172,7 @@ impl<'a> ArchiveBuilder<'a> {
         let name = file.file_name().unwrap().to_str().unwrap();
         self.additions.push(Addition::File {
             path: file.to_path_buf(),
-            name_in_archive: name.to_string(),
+            name_in_archive: name.to_owned(),
         });
     }
 
@@ -184,13 +185,8 @@ impl<'a> ArchiveBuilder<'a> {
     /// Combine the provided files, rlibs, and native libraries into a single
     /// `Archive`.
     pub fn build(&mut self) {
-        let kind = match self.llvm_archive_kind() {
-            Ok(kind) => kind,
-            Err(kind) => {
-                self.config.sess.fatal(&format!("Don't know how to build archive of type: {}",
-                                                kind));
-            }
-        };
+        let kind = self.llvm_archive_kind().unwrap_or_else(|kind|
+            self.config.sess.fatal(&format!("Don't know how to build archive of type: {}", kind)));
 
         if let Err(e) = self.build_with_llvm(kind) {
             self.config.sess.fatal(&format!("failed to build archive: {}", e));
@@ -281,10 +277,9 @@ impl<'a> ArchiveBuilder<'a> {
             let ret = if r.into_result().is_err() {
                 let err = llvm::LLVMRustGetLastError();
                 let msg = if err.is_null() {
-                    "failed to write archive".to_string()
+                    "failed to write archive".into()
                 } else {
                     String::from_utf8_lossy(CStr::from_ptr(err).to_bytes())
-                            .into_owned()
                 };
                 Err(io::Error::new(io::ErrorKind::Other, msg))
             } else {
@@ -293,7 +288,7 @@ impl<'a> ArchiveBuilder<'a> {
             for member in members {
                 llvm::LLVMRustArchiveMemberFree(member);
             }
-            return ret
+            ret
         }
     }
 }

Diff for: src/librustc_codegen_llvm/back/bytecode.rs

@@ -42,7 +42,7 @@ use flate2::write::DeflateEncoder;
 
 // This is the "magic number" expected at the beginning of a LLVM bytecode
 // object in an rlib.
-pub const RLIB_BYTECODE_OBJECT_MAGIC: &'static [u8] = b"RUST_OBJECT";
+pub const RLIB_BYTECODE_OBJECT_MAGIC: &[u8] = b"RUST_OBJECT";
 
 // The version number this compiler will write to bytecode objects in rlibs
 pub const RLIB_BYTECODE_OBJECT_VERSION: u8 = 2;
@@ -106,39 +106,39 @@ pub struct DecodedBytecode<'a> {
 }
 
 impl<'a> DecodedBytecode<'a> {
-    pub fn new(data: &'a [u8]) -> Result<DecodedBytecode<'a>, String> {
+    pub fn new(data: &'a [u8]) -> Result<DecodedBytecode<'a>, &'static str> {
         if !data.starts_with(RLIB_BYTECODE_OBJECT_MAGIC) {
-            return Err("magic bytecode prefix not found".to_string())
+            return Err("magic bytecode prefix not found")
         }
         let data = &data[RLIB_BYTECODE_OBJECT_MAGIC.len()..];
         if !data.starts_with(&[RLIB_BYTECODE_OBJECT_VERSION, 0, 0, 0]) {
-            return Err("wrong version prefix found in bytecode".to_string())
+            return Err("wrong version prefix found in bytecode")
         }
         let data = &data[4..];
         if data.len() < 4 {
-            return Err("bytecode corrupted".to_string())
+            return Err("bytecode corrupted")
         }
         let identifier_len = unsafe {
             u32::from_le(ptr::read_unaligned(data.as_ptr() as *const u32)) as usize
         };
         let data = &data[4..];
         if data.len() < identifier_len {
-            return Err("bytecode corrupted".to_string())
+            return Err("bytecode corrupted")
         }
         let identifier = match str::from_utf8(&data[..identifier_len]) {
             Ok(s) => s,
-            Err(_) => return Err("bytecode corrupted".to_string())
+            Err(_) => return Err("bytecode corrupted")
         };
         let data = &data[identifier_len..];
         if data.len() < 8 {
-            return Err("bytecode corrupted".to_string())
+            return Err("bytecode corrupted")
         }
         let bytecode_len = unsafe {
             u64::from_le(ptr::read_unaligned(data.as_ptr() as *const u64)) as usize
         };
         let data = &data[8..];
         if data.len() < bytecode_len {
-            return Err("bytecode corrupted".to_string())
+            return Err("bytecode corrupted")
         }
         let encoded_bytecode = &data[..bytecode_len];
 

Diff for: src/librustc_codegen_llvm/back/link.rs

@@ -47,8 +47,8 @@ use std::str;
 use syntax::attr;
 
 pub use rustc_codegen_utils::link::{find_crate_name, filename_for_input, default_output_for_target,
-                                  invalid_output_for_target, out_filename, check_file_is_writeable,
-                                  filename_for_metadata};
+                                    invalid_output_for_target, filename_for_metadata,
+                                    out_filename, check_file_is_writeable};
 
 // The third parameter is for env vars, used on windows to set up the
 // path for MSVC to find its DLLs, and gcc to find its bundled
@@ -107,13 +107,10 @@ pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> (PathB
 }
 
 pub fn remove(sess: &Session, path: &Path) {
-    match fs::remove_file(path) {
-        Ok(..) => {}
-        Err(e) => {
-            sess.err(&format!("failed to remove {}: {}",
-                             path.display(),
-                             e));
-        }
+    if let Err(e) = fs::remove_file(path) {
+        sess.err(&format!("failed to remove {}: {}",
+                          path.display(),
+                          e));
     }
 }
 
@@ -147,9 +144,7 @@ pub(crate) fn link_binary(sess: &Session,
 
     // Remove the temporary object file and metadata if we aren't saving temps
     if !sess.opts.cg.save_temps {
-        if sess.opts.output_types.should_codegen() &&
-            !preserve_objects_for_their_debuginfo(sess)
-        {
+        if sess.opts.output_types.should_codegen() && !preserve_objects_for_their_debuginfo(sess) {
             for obj in codegen_results.modules.iter().filter_map(|m| m.object.as_ref()) {
                 remove(sess, obj);
             }
@@ -186,7 +181,7 @@ fn preserve_objects_for_their_debuginfo(sess: &Session) -> bool {
     // the objects as they're losslessly contained inside the archives.
     let output_linked = sess.crate_types.borrow()
         .iter()
-        .any(|x| *x != config::CrateType::Rlib && *x != config::CrateType::Staticlib);
+        .any(|&x| x != config::CrateType::Rlib && x != config::CrateType::Staticlib);
     if !output_linked {
         return false
     }
@@ -270,7 +265,7 @@ pub(crate) fn ignored_for_lto(sess: &Session, info: &CrateInfo, cnum: CrateNum)
     // crates providing these functions don't participate in LTO (e.g.
     // no_builtins or compiler builtins crates).
     !sess.target.target.options.no_builtins &&
-        (info.is_no_builtins.contains(&cnum) || info.compiler_builtins == Some(cnum))
+        (info.compiler_builtins == Some(cnum) || info.is_no_builtins.contains(&cnum))
 }
 
 fn link_binary_output(sess: &Session,
@@ -291,24 +286,19 @@ fn link_binary_output(sess: &Session,
         // final destination, with a `fs::rename` call. In order for the rename to
         // always succeed, the temporary file needs to be on the same filesystem,
         // which is why we create it inside the output directory specifically.
-        let metadata_tmpdir = match TempFileBuilder::new()
+        let metadata_tmpdir = TempFileBuilder::new()
             .prefix("rmeta")
             .tempdir_in(out_filename.parent().unwrap())
-        {
-            Ok(tmpdir) => tmpdir,
-            Err(err) => sess.fatal(&format!("couldn't create a temp dir: {}", err)),
-        };
+            .unwrap_or_else(|err| sess.fatal(&format!("couldn't create a temp dir: {}", err)));
         let metadata = emit_metadata(sess, codegen_results, &metadata_tmpdir);
         if let Err(e) = fs::rename(metadata, &out_filename) {
             sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e));
         }
         out_filenames.push(out_filename);
     }
 
-    let tmpdir = match TempFileBuilder::new().prefix("rustc").tempdir() {
-        Ok(tmpdir) => tmpdir,
-        Err(err) => sess.fatal(&format!("couldn't create a temp dir: {}", err)),
-    };
+    let tmpdir = TempFileBuilder::new().prefix("rustc").tempdir().unwrap_or_else(|err|
+        sess.fatal(&format!("couldn't create a temp dir: {}", err)));
 
     if outputs.outputs.should_codegen() {
         let out_filename = out_filename(sess, crate_type, outputs, crate_name);
@@ -342,7 +332,8 @@ fn archive_search_paths(sess: &Session) -> Vec<PathBuf> {
     sess.target_filesearch(PathKind::Native).for_each_lib_search_path(|path, _| {
         search.push(path.to_path_buf());
     });
-    return search;
+
+    search
 }
 
 fn archive_config<'a>(sess: &'a Session,
@@ -814,8 +805,8 @@ fn link_natively(sess: &Session,
                     .unwrap_or_else(|_| {
                         let mut x = "Non-UTF-8 output: ".to_string();
                         x.extend(s.iter()
-                                 .flat_map(|&b| ascii::escape_default(b))
-                                 .map(|b| char::from_u32(b as u32).unwrap()));
+                                  .flat_map(|&b| ascii::escape_default(b))
+                                  .map(char::from));
                         x
                     })
             }
@@ -870,9 +861,8 @@ fn link_natively(sess: &Session,
         sess.opts.debuginfo != DebugInfo::None &&
         !preserve_objects_for_their_debuginfo(sess)
     {
-        match Command::new("dsymutil").arg(out_filename).output() {
-            Ok(..) => {}
-            Err(e) => sess.fatal(&format!("failed to run dsymutil: {}", e)),
+        if let Err(e) = Command::new("dsymutil").arg(out_filename).output() {
+            sess.fatal(&format!("failed to run dsymutil: {}", e))
         }
     }
 
@@ -1012,8 +1002,7 @@ fn exec_linker(sess: &Session, cmd: &mut Command, out_filename: &Path, tmpdir: &
                 // ensure the line is interpreted as one whole argument.
                 for c in self.arg.chars() {
                     match c {
-                        '\\' |
-                        ' ' => write!(f, "\\{}", c)?,
+                        '\\' | ' ' => write!(f, "\\{}", c)?,
                         c => write!(f, "{}", c)?,
                     }
                 }
@@ -1426,7 +1415,6 @@ fn add_upstream_rust_crates(cmd: &mut dyn Linker,
         for f in archive.src_files() {
             if f.ends_with(RLIB_BYTECODE_EXTENSION) || f == METADATA_FILENAME {
                 archive.remove_file(&f);
-                continue
             }
         }
 

Diff for: src/librustc_codegen_llvm/back/lto.rs

@@ -205,11 +205,11 @@ pub(crate) fn run(cgcx: &CodegenContext,
         Lto::Fat => {
             assert!(cached_modules.is_empty());
             let opt_jobs = fat_lto(cgcx,
-                                  &diag_handler,
-                                  modules,
-                                  upstream_modules,
-                                  &symbol_white_list,
-                                  timeline);
+                                   &diag_handler,
+                                   modules,
+                                   upstream_modules,
+                                   &symbol_white_list,
+                                   timeline);
             opt_jobs.map(|opt_jobs| (opt_jobs, vec![]))
         }
         Lto::Thin |
@@ -296,7 +296,7 @@ fn fat_lto(cgcx: &CodegenContext,
                 let data = bc_decoded.data();
                 linker.add(&data).map_err(|()| {
                     let msg = format!("failed to load bc of {:?}", name);
-                    write::llvm_err(&diag_handler, msg)
+                    write::llvm_err(&diag_handler, &msg)
                 })
             })?;
             timeline.record(&format!("link {:?}", name));
@@ -310,8 +310,8 @@ fn fat_lto(cgcx: &CodegenContext,
         unsafe {
             let ptr = symbol_white_list.as_ptr();
             llvm::LLVMRustRunRestrictionPass(llmod,
-                                            ptr as *const *const libc::c_char,
-                                            symbol_white_list.len() as libc::size_t);
+                                             ptr as *const *const libc::c_char,
+                                             symbol_white_list.len() as libc::size_t);
             cgcx.save_temp_bitcode(&module, "lto.after-restriction");
         }
 
@@ -490,7 +490,7 @@ fn thin_lto(cgcx: &CodegenContext,
             symbol_white_list.as_ptr(),
             symbol_white_list.len() as u32,
         ).ok_or_else(|| {
-            write::llvm_err(&diag_handler, "failed to prepare thin LTO context".to_string())
+            write::llvm_err(&diag_handler, "failed to prepare thin LTO context")
         })?;
 
         info!("thin LTO data created");
@@ -617,8 +617,7 @@ fn run_pass_manager(cgcx: &CodegenContext,
             llvm::LLVMRustAddPass(pm, pass.unwrap());
         }
 
-        time_ext(cgcx.time_passes, None, "LTO passes", ||
-             llvm::LLVMRunPassManager(pm, llmod));
+        time_ext(cgcx.time_passes, None, "LTO passes", || llvm::LLVMRunPassManager(pm, llmod));
 
         llvm::LLVMDisposePassManager(pm);
     }
@@ -747,7 +746,7 @@ impl ThinModule {
     {
         let diag_handler = cgcx.create_diag_handler();
         let tm = (cgcx.tm_factory)().map_err(|e| {
-            write::llvm_err(&diag_handler, e)
+            write::llvm_err(&diag_handler, &e)
         })?;
 
         // Right now the implementation we've got only works over serialized
@@ -762,7 +761,7 @@ impl ThinModule {
             self.data().len(),
             self.shared.module_names[self.idx].as_ptr(),
         ).ok_or_else(|| {
-            let msg = "failed to parse bitcode for thin LTO module".to_string();
+            let msg = "failed to parse bitcode for thin LTO module";
             write::llvm_err(&diag_handler, msg)
         })? as *const _;
         let module = ModuleCodegen {
@@ -786,7 +785,7 @@ impl ThinModule {
             let mut cu2 = ptr::null_mut();
             llvm::LLVMRustThinLTOGetDICompileUnit(llmod, &mut cu1, &mut cu2);
             if !cu2.is_null() {
-                let msg = "multiple source DICompileUnits found".to_string();
+                let msg = "multiple source DICompileUnits found";
                 return Err(write::llvm_err(&diag_handler, msg))
             }
 
@@ -807,25 +806,25 @@ impl ThinModule {
             // You can find some more comments about these functions in the LLVM
             // bindings we've got (currently `PassWrapper.cpp`)
             if !llvm::LLVMRustPrepareThinLTORename(self.shared.data.0, llmod) {
-                let msg = "failed to prepare thin LTO module".to_string();
+                let msg = "failed to prepare thin LTO module";
                 return Err(write::llvm_err(&diag_handler, msg))
             }
             cgcx.save_temp_bitcode(&module, "thin-lto-after-rename");
             timeline.record("rename");
             if !llvm::LLVMRustPrepareThinLTOResolveWeak(self.shared.data.0, llmod) {
-                let msg = "failed to prepare thin LTO module".to_string();
+                let msg = "failed to prepare thin LTO module";
                 return Err(write::llvm_err(&diag_handler, msg))
             }
             cgcx.save_temp_bitcode(&module, "thin-lto-after-resolve");
             timeline.record("resolve");
             if !llvm::LLVMRustPrepareThinLTOInternalize(self.shared.data.0, llmod) {
-                let msg = "failed to prepare thin LTO module".to_string();
+                let msg = "failed to prepare thin LTO module";
                 return Err(write::llvm_err(&diag_handler, msg))
             }
             cgcx.save_temp_bitcode(&module, "thin-lto-after-internalize");
             timeline.record("internalize");
             if !llvm::LLVMRustPrepareThinLTOImport(self.shared.data.0, llmod) {
-                let msg = "failed to prepare thin LTO module".to_string();
+                let msg = "failed to prepare thin LTO module";
                 return Err(write::llvm_err(&diag_handler, msg))
             }
             cgcx.save_temp_bitcode(&module, "thin-lto-after-import");
@@ -920,12 +919,6 @@ impl ThinLTOImports {
 }
 
 fn module_name_to_str(c_str: &CStr) -> &str {
-    match c_str.to_str() {
-        Ok(s) => s,
-        Err(e) => {
-            bug!("Encountered non-utf8 LLVM module name `{}`: {}",
-                c_str.to_string_lossy(),
-                e)
-        }
-    }
+    c_str.to_str().unwrap_or_else(|e|
+        bug!("Encountered non-utf8 LLVM module name `{}`: {}", c_str.to_string_lossy(), e))
 }