Incorporated review feedback:

Renamed the struct to make it a little clearer that it doesn't just hold one
imports map. (I couldn't bring myself to write it as `ThinLTOImportsExports`
though, mainly since the exports map is literally derived from the imports map
data.) Added some doc to the struct too.

Revised comments to add link to the newer issue that discusses why the exports
are relevant.

Renamed a few of the methods so that the two character difference is more
apparent (because 1. the method name is shorter and, perhaps more importantly,
the changed characters now lie at the beginning of the method name.)

Author: pnkfelix

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

@@ -463,15 +463,18 @@ fn thin_lto(
                 // If previous imports have been deleted, or we get an IO error
                 // reading the file storing them, then we'll just use `None` as the
                 // prev_import_map, which will force the code to be recompiled.
-                let prev =
-                    if path.exists() { ThinLTOImports::load_from_file(&path).ok() } else { None };
-                let curr = ThinLTOImports::from_thin_lto_data(data);
+                let prev = if path.exists() {
+                    ThinLTOImportMaps::load_from_file(&path).ok()
+                } else {
+                    None
+                };
+                let curr = ThinLTOImportMaps::from_thin_lto_data(data);
                 (Some(path), prev, curr)
             } else {
                 // If we don't compile incrementally, we don't need to load the
                 // import data from LLVM.
                 assert!(green_modules.is_empty());
-                let curr = ThinLTOImports::default();
+                let curr = ThinLTOImportMaps::default();
                 (None, None, curr)
             };
         info!("thin LTO import map loaded");
@@ -497,10 +500,14 @@ fn thin_lto(
             let module_name = module_name_to_str(module_name);
 
             // If (1.) the module hasn't changed, and (2.) none of the modules
-            // it imports from has changed, *and* (3.) the import-set itself has
-            // not changed from the previous compile when it was last
-            // ThinLTO'ed, then we can re-use the post-ThinLTO version of the
-            // module. Otherwise, freshly perform LTO optimization.
+            // it imports from nor exports to have changed, *and* (3.) the
+            // import and export sets themselves have not changed from the
+            // previous compile when it was last ThinLTO'ed, then we can re-use
+            // the post-ThinLTO version of the module. Otherwise, freshly
+            // perform LTO optimization.
+            //
+            // (Note that globally, the export set is just the inverse of the
+            // import set.)
             //
             // This strategy means we can always save the computed imports as
             // canon: when we reuse the post-ThinLTO version, condition (3.)
@@ -509,16 +516,18 @@ fn thin_lto(
             // version, the current import set *is* the correct one, since we
             // are doing the ThinLTO in this current compilation cycle.)
             //
-            // See rust-lang/rust#59535.
+            // For more discussion, see rust-lang/rust#59535 (where the import
+            // issue was discovered) and rust-lang/rust#69798 (where the
+            // analogous export issue was discovered).
             if let (Some(prev_import_map), true) =
                 (prev_import_map.as_ref(), green_modules.contains_key(module_name))
             {
                 assert!(cgcx.incr_comp_session_dir.is_some());
 
-                let prev_imports = prev_import_map.modules_imported_by(module_name);
-                let curr_imports = curr_import_map.modules_imported_by(module_name);
-                let prev_exports = prev_import_map.modules_exported_by(module_name);
-                let curr_exports = curr_import_map.modules_exported_by(module_name);
+                let prev_imports = prev_import_map.imports_of(module_name);
+                let curr_imports = curr_import_map.imports_of(module_name);
+                let prev_exports = prev_import_map.exports_of(module_name);
+                let curr_exports = curr_import_map.exports_of(module_name);
                 let imports_all_green = curr_imports
                     .iter()
                     .all(|imported_module| green_modules.contains_key(imported_module));
@@ -890,20 +899,29 @@ pub unsafe fn optimize_thin_module(
     Ok(module)
 }
 
+/// Summarizes module import/export relationships used by LLVM's ThinLTO pass.
+///
+/// Note that we tend to have two such instances of `ThinLTOImportMaps` in use:
+/// one loaded from a file that represents the relationships used during the
+/// compilation associated with the incremetnal build artifacts we are
+/// attempting to reuse, and another constructed via `from_thin_lto_data`, which
+/// captures the relationships of ThinLTO in the current compilation.
 #[derive(Debug, Default)]
-pub struct ThinLTOImports {
+pub struct ThinLTOImportMaps {
     // key = llvm name of importing module, value = list of modules it imports from
     imports: FxHashMap<String, Vec<String>>,
     // key = llvm name of exporting module, value = list of modules it exports to
     exports: FxHashMap<String, Vec<String>>,
 }
 
-impl ThinLTOImports {
-    fn modules_imported_by(&self, llvm_module_name: &str) -> &[String] {
+impl ThinLTOImportMaps {
+    /// Returns modules imported by `llvm_module_name` during some ThinLTO pass.
+    fn imports_of(&self, llvm_module_name: &str) -> &[String] {
         self.imports.get(llvm_module_name).map(|v| &v[..]).unwrap_or(&[])
     }
 
-    fn modules_exported_by(&self, llvm_module_name: &str) -> &[String] {
+    /// Returns modules exported by `llvm_module_name` during some ThinLTO pass.
+    fn exports_of(&self, llvm_module_name: &str) -> &[String] {
         self.exports.get(llvm_module_name).map(|v| &v[..]).unwrap_or(&[])
     }
 
@@ -921,7 +939,7 @@ impl ThinLTOImports {
         Ok(())
     }
 
-    fn load_from_file(path: &Path) -> io::Result<ThinLTOImports> {
+    fn load_from_file(path: &Path) -> io::Result<ThinLTOImportMaps> {
         use std::io::BufRead;
         let mut imports = FxHashMap::default();
         let mut exports: FxHashMap<_, Vec<_>> = FxHashMap::default();
@@ -946,17 +964,17 @@ impl ThinLTOImports {
                 current_module = Some(line.trim().to_string());
             }
         }
-        Ok(ThinLTOImports { imports, exports })
+        Ok(ThinLTOImportMaps { imports, exports })
     }
 
     /// Loads the ThinLTO import map from ThinLTOData.
-    unsafe fn from_thin_lto_data(data: *const llvm::ThinLTOData) -> ThinLTOImports {
+    unsafe fn from_thin_lto_data(data: *const llvm::ThinLTOData) -> ThinLTOImportMaps {
         unsafe extern "C" fn imported_module_callback(
             payload: *mut libc::c_void,
             importing_module_name: *const libc::c_char,
             imported_module_name: *const libc::c_char,
         ) {
-            let map = &mut *(payload as *mut ThinLTOImports);
+            let map = &mut *(payload as *mut ThinLTOImportMaps);
             let importing_module_name = CStr::from_ptr(importing_module_name);
             let importing_module_name = module_name_to_str(&importing_module_name);
             let imported_module_name = CStr::from_ptr(imported_module_name);
@@ -981,7 +999,7 @@ impl ThinLTOImports {
                 .push(importing_module_name.to_owned());
         }
 
-        let mut map = ThinLTOImports::default();
+        let mut map = ThinLTOImportMaps::default();
         llvm::LLVMRustGetThinLTOModuleImports(
             data,
             imported_module_callback,