transpile: handle module name collisions

Author: randomPoison

c2rust-transpile/src/build_files/mod.rs

@@ -1,6 +1,7 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 use std::fs::{self, File};
 use std::io::Write;
+use std::ops::Bound::{Excluded, Unbounded};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
@@ -107,7 +108,7 @@ pub fn emit_build_files<'lcmd>(
     })
 }
 
-#[derive(Serialize)]
+#[derive(Clone, Debug, Serialize)]
 struct Module {
     path: Option<String>,
     name: String,
@@ -116,19 +117,23 @@ struct Module {
 }
 
 #[derive(Debug, Default)]
-struct ModuleTree(BTreeMap<String, ModuleTree>);
+struct ModuleTree {
+    children: BTreeMap<String, ModuleTree>,
+    path_modules: Vec<Module>,
+}
 
 impl ModuleTree {
     /// Convert the tree representation into a linear vector
     /// and push it into `res`
     fn linearize(&self, res: &mut Vec<Module>) {
-        for (name, child) in self.0.iter() {
+        res.extend(self.path_modules.iter().cloned());
+        for (name, child) in self.children.iter() {
             child.linearize_internal(name, res);
         }
     }
 
     fn linearize_internal(&self, name: &str, res: &mut Vec<Module>) {
-        if self.0.is_empty() {
+        if self.path_modules.is_empty() && self.children.is_empty() {
             res.push(Module {
                 name: name.to_string(),
                 path: None,
@@ -151,6 +156,55 @@ impl ModuleTree {
             });
         }
     }
+
+    fn insert_nested(&mut self, path: Vec<String>) {
+        let mut cur = self;
+        for name in path {
+            cur = cur.children.entry(name).or_default();
+        }
+    }
+
+    fn insert_path_module(
+        &mut self,
+        parent_path: &[String],
+        file_name: String,
+        module_name: String,
+    ) {
+        let mut cur = self;
+        for name in parent_path {
+            cur = cur.children.entry(name.clone()).or_default();
+        }
+        let name = cur.unique_child_name(&module_name);
+        cur.path_modules.push(Module {
+            path: Some(file_name),
+            name,
+            open: false,
+            close: false,
+        });
+    }
+
+    fn unique_child_name(&self, module_name: &str) -> String {
+        if !self.child_name_exists(module_name) {
+            return module_name.to_owned();
+        }
+
+        for i in 1.. {
+            let candidate = format!("{module_name}_{i}");
+            if !self.child_name_exists(&candidate) {
+                return candidate;
+            }
+        }
+
+        unreachable!("We tried all the numbers and couldn't find one that didn't collide")
+    }
+
+    fn child_name_exists(&self, module_name: &str) -> bool {
+        self.children.contains_key(module_name)
+            || self
+                .path_modules
+                .iter()
+                .any(|module| module.name == module_name)
+    }
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -182,17 +236,51 @@ fn convert_module_list(
     });
 
     let mut res = vec![];
-    let mut module_tree = ModuleTree(BTreeMap::new());
+    let mut module_tree = ModuleTree::default();
+    // A C file can have the same stem as a sibling directory, such as
+    // `hash.c` and `hash/sha1.c`. The current nested module emitter cannot
+    // represent both at `mod hash`, so emit the file module with an explicit
+    // path and keep the directory in the nested tree.
+    let internal_modules = modules
+        .iter()
+        .filter_map(|m| {
+            if tcfg.is_binary(m) {
+                return None;
+            }
+            let relpath = m.strip_prefix(build_dir).ok()?;
+            let path = module_path(relpath);
+            Some((m, path))
+        })
+        .collect::<Vec<_>>();
+    // A module collides when another module's path strictly extends it, i.e. a
+    // file whose stem matches a sibling directory (`hash.c` next to `hash/`).
+    // Every strict extension of a path sorts immediately after it, so checking
+    // each path's immediate successor in the sorted set settles it without
+    // comparing every pair.
+    let module_paths = internal_modules
+        .iter()
+        .map(|(_, path)| path.clone())
+        .collect::<BTreeSet<_>>();
+    let collision_modules = internal_modules
+        .iter()
+        .filter(|(_, path)| {
+            module_paths
+                .range::<Vec<String>, _>((Excluded(path), Unbounded))
+                .next()
+                .is_some_and(|next| next.len() > path.len() && next.starts_with(path))
+        })
+        .map(|(module, _)| *module)
+        .collect::<BTreeSet<_>>();
+
+    let mut collision_relpaths = vec![];
     for m in &modules {
         match m.strip_prefix(build_dir) {
-            Ok(relpath) if !tcfg.is_binary(m) => {
+            Ok(relpath) if !tcfg.is_binary(m) && !collision_modules.contains(m) => {
                 // The module is inside the build directory, use nested modules
-                let mut cur = &mut module_tree;
-                for sm in relpath.iter() {
-                    let path = Path::new(sm);
-                    let name = get_module_name(path, true, false, false).unwrap();
-                    cur = cur.0.entry(name).or_default();
-                }
+                module_tree.insert_nested(module_path(relpath));
+            }
+            Ok(relpath) if !tcfg.is_binary(m) => {
+                collision_relpaths.push(relpath.to_path_buf());
             }
             _ => {
                 let relpath = diff_paths(m, build_dir).unwrap();
@@ -207,10 +295,28 @@ fn convert_module_list(
             }
         }
     }
+    for relpath in collision_relpaths {
+        let mut path = module_path(&relpath);
+        // Insert the shadowed file as an explicit `#[path]` module in its
+        // parent. `insert_path_module` renames it if the stem collides with the
+        // sibling directory (or another module), so pass the plain stem.
+        let module_name = path.pop().unwrap();
+        let file_name = relpath.file_name().unwrap().to_str().unwrap().to_string();
+        module_tree.insert_path_module(&path, file_name, module_name);
+    }
     module_tree.linearize(&mut res);
     res
 }
 
+fn module_path(path: &Path) -> Vec<String> {
+    path.iter()
+        .map(|component| {
+            let path = Path::new(component);
+            get_module_name(path, true, false, false).unwrap()
+        })
+        .collect()
+}
+
 fn emit_thin_binaries(
     tcfg: &TranspilerConfig,
     build_dir: &Path,

c2rust-transpile/src/lib.rs

@@ -1,3 +1,4 @@
+#![feature(is_some_and)]
 #![allow(clippy::too_many_arguments)]
 
 mod diagnostics;

c2rust-transpile/tests/module_paths.rs

@@ -0,0 +1,106 @@
+use std::fs;
+use std::path::PathBuf;
+use std::process::Command;
+
+use c2rust_rust_tools::RustEdition;
+use c2rust_transpile::{ReplaceMode, TranspilerConfig};
+use tempfile::TempDir;
+
+fn config(output_dir: PathBuf) -> TranspilerConfig {
+    TranspilerConfig {
+        dump_untyped_context: false,
+        dump_typed_context: false,
+        pretty_typed_context: false,
+        dump_function_cfgs: false,
+        json_function_cfgs: false,
+        dump_cfg_liveness: false,
+        dump_structures: false,
+        verbose: false,
+        debug_ast_exporter: false,
+        emit_c_decl_map: false,
+        incremental_relooper: true,
+        fail_on_multiple: false,
+        filter: None,
+        debug_relooper_labels: false,
+        cross_checks: false,
+        cross_check_backend: Default::default(),
+        cross_check_configs: Default::default(),
+        prefix_function_names: None,
+        translate_asm: true,
+        use_c_loop_info: true,
+        use_c_multiple_info: true,
+        simplify_structures: true,
+        panic_on_translator_failure: false,
+        emit_modules: true,
+        fail_on_error: true,
+        replace_unsupported_decls: ReplaceMode::Extern,
+        translate_valist: true,
+        overwrite_existing: true,
+        reduce_type_annotations: false,
+        reorganize_definitions: false,
+        enabled_warnings: Default::default(),
+        emit_no_std: false,
+        output_dir: Some(output_dir),
+        translate_const_macros: Default::default(),
+        translate_fn_macros: Default::default(),
+        disable_rustfmt: false,
+        disable_refactoring: false,
+        preserve_unused_functions: false,
+        log_level: log::LevelFilter::Warn,
+        edition: RustEdition::Edition2021,
+        deny_unsafe_op_in_unsafe_fn: false,
+        postprocess: false,
+        emit_build_files: true,
+        binaries: vec!["main".to_owned()],
+        thin_binaries: true,
+        c2rust_dir: Some(
+            PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+                .parent()
+                .unwrap()
+                .to_path_buf(),
+        ),
+    }
+}
+
+#[test]
+fn sibling_file_and_directory_modules_do_not_collide() {
+    let td = TempDir::new().unwrap();
+    let src = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+        .join("tests/module_paths/sibling_file_and_directory");
+    let output_dir = td.path().join("translated");
+
+    let (_compile_commands_dir, compile_commands_path) =
+        c2rust_transpile::create_temp_compile_commands(&[
+            src.join("main.c"),
+            src.join("hash.c"),
+            src.join("hash_.c"),
+            src.join("hash/sha1.c"),
+        ]);
+
+    c2rust_transpile::transpile(config(output_dir.clone()), &compile_commands_path, &["-w"]);
+
+    let lib_rs = fs::read_to_string(output_dir.join("lib.rs")).unwrap();
+    assert!(
+        lib_rs.contains("pub mod hash_;"),
+        "generated module tree should include the non-colliding hash_.rs \
+         module normally:\n{lib_rs}"
+    );
+    assert!(
+        lib_rs.contains("#[path = \"hash.rs\"]\n    pub mod hash_1;"),
+        "generated module tree should include the sibling hash.rs module via an \
+         explicit path module inside its parent instead of shadowing it with \
+         the hash/ directory:\n{lib_rs}"
+    );
+    assert!(
+        lib_rs.contains("pub mod sha1;"),
+        "generated lib.rs should include the nested hash/sha1.rs module:\n{lib_rs}"
+    );
+
+    let status = Command::new("cargo")
+        .arg("build")
+        .env("CARGO_TARGET_DIR", td.path().join("target"))
+        .current_dir(&output_dir)
+        .status()
+        .unwrap();
+    assert!(status.success(), "generated crate should build");
+}

c2rust-transpile/tests/module_paths/sibling_file_and_directory/hash.c

@@ -0,0 +1,5 @@
+#include "hash/sha1.h"
+
+int hash_value(int x) {
+    return sha1_mix(x) + 1;
+}

c2rust-transpile/tests/module_paths/sibling_file_and_directory/hash/sha1.c

@@ -0,0 +1,3 @@
+int sha1_mix(int x) {
+    return x * 3;
+}

c2rust-transpile/tests/module_paths/sibling_file_and_directory/hash/sha1.h

@@ -0,0 +1 @@
+int sha1_mix(int x);

c2rust-transpile/tests/module_paths/sibling_file_and_directory/hash_.c

@@ -0,0 +1,3 @@
+int hash_utility(void) {
+    return 7;
+}

c2rust-transpile/tests/module_paths/sibling_file_and_directory/main.c

@@ -0,0 +1,5 @@
+int hash_value(int);
+
+int main(void) {
+    return hash_value(13) == 40 ? 0 : 1;
+}