Avoid name collision of static linkage variables (#154)

This PR extends GetNamedDeclAsString to handle variables with static
linkage. This fixes:
* shadowing between global variables and local variables
(tests/unit/static_shadow.c)
* name collisions beween static variables declared in different
functions (tests/unit/static_function_vars_same_name.cpp)
* name collisions between global static variables declared in different
TUs (tests/multi-file/static_name_collision)

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -403,7 +403,6 @@ bool Converter::ConvertVarDeclSkipInit(clang::VarDecl *decl) {
   }
 
   if (decl->isFileVarDecl()) {
-    name = ReplaceAll(Mapper::ToString(decl), "::", "_");
     if ((decl->isThisDeclarationADefinition() ==
              clang::VarDecl::DeclarationOnly &&
          !decl->hasInit()) ||
@@ -2378,7 +2377,7 @@ std::string Converter::ConvertDeclRefExpr(clang::DeclRefExpr *expr) {
   }
 
   if (IsGlobalVar(expr)) {
-    return ReplaceAll(Mapper::ToString(expr->getDecl()), "::", "_");
+    return GetNamedDeclAsString(expr->getDecl());
   }
 
   return GetNamedDeclAsString(decl);

cpp2rust/converter/converter_lib.cpp

@@ -369,6 +369,16 @@ std::string GetID(const clang::Decl *decl) {
 
 static std::unordered_map<std::string, size_t> type_mapping;
 
+static size_t GetDeclId(const clang::NamedDecl *decl, bool internal) {
+  std::string key =
+      clang::ASTNameGenerator(decl->getASTContext()).getName(decl) +
+      GetParamSignature(decl);
+  if (internal) {
+    key += GetLocationID(decl);
+  }
+  return type_mapping.try_emplace(key, type_mapping.size()).first->second;
+}
+
 std::string GetNamedDeclAsString(const clang::NamedDecl *decl) {
   auto name = decl->getDeclName().isIdentifier() ? decl->getName().str()
                                                  : decl->getNameAsString();
@@ -384,19 +394,19 @@ std::string GetNamedDeclAsString(const clang::NamedDecl *decl) {
     return std::format("anon_{}", GetAnonIndex(target));
   }
 
+  std::optional<size_t> id;
   if (auto *fn = clang::dyn_cast<clang::FunctionDecl>(decl)) {
     if (!clang::isa<clang::CXXMethodDecl>(fn)) {
-      auto mangled =
-          clang::ASTNameGenerator(decl->getASTContext()).getName(decl) +
-          GetParamSignature(decl);
-      if (fn->getFormalLinkage() == clang::Linkage::Internal) {
-        mangled += GetLocationID(decl);
-      }
-      auto id =
-          type_mapping.try_emplace(mangled, type_mapping.size()).first->second;
-      name += '_';
-      name += std::to_string(id);
+      id = GetDeclId(decl, fn->getFormalLinkage() == clang::Linkage::Internal);
     }
+  } else if (auto *var = clang::dyn_cast<clang::VarDecl>(decl);
+             var && (var->isFileVarDecl() || var->isStaticLocal())) {
+    id = GetDeclId(var->getCanonicalDecl(),
+                   var->getFormalLinkage() != clang::Linkage::External);
+  }
+  if (id) {
+    name += '_';
+    name += std::to_string(*id);
   }
 
   // transform decl names that are rust keywords:

tests/multi-file/static_name_collision/CMakeLists.txt

@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.16)
+project(static_name_collision LANGUAGES C)
+add_executable(app a.c b.c)

tests/multi-file/static_name_collision/a.c

@@ -0,0 +1,18 @@
+#include <assert.h>
+
+static int same_name_different_type = 1;
+static int same_name_same_type = 5;
+
+int a_foo() { return same_name_different_type; }
+int a_bar() { return same_name_same_type; }
+
+float b_foo();
+int b_bar();
+
+int main(void) {
+  assert(a_foo() == 1);
+  assert(b_foo() == 1.0f);
+  assert(a_bar() == 5);
+  assert(b_bar() == 6);
+  return 0;
+}

tests/multi-file/static_name_collision/b.c

@@ -0,0 +1,5 @@
+static float same_name_different_type = 1.0f;
+static int same_name_same_type = 6;
+
+float b_foo() { return same_name_different_type; }
+int b_bar() { return same_name_same_type; }

tests/multi-file/static_name_collision/out/refcount/static_name_collision.rs

@@ -0,0 +1,42 @@
+extern crate libcc2rs;
+use libcc2rs::*;
+use std::cell::RefCell;
+use std::collections::BTreeMap;
+use std::io::prelude::*;
+use std::io::{Read, Seek, Write};
+use std::os::fd::AsFd;
+use std::rc::{Rc, Weak};
+thread_local!(
+    pub static same_name_different_type_0: Value<i32> = Rc::new(RefCell::new(1));
+);
+thread_local!(
+    pub static same_name_same_type_1: Value<i32> = Rc::new(RefCell::new(5));
+);
+pub fn a_foo_2() -> i32 {
+    return (*same_name_different_type_0.with(Value::clone).borrow());
+}
+pub fn a_bar_3() -> i32 {
+    return (*same_name_same_type_1.with(Value::clone).borrow());
+}
+pub fn main() {
+    std::process::exit(main_0());
+}
+fn main_0() -> i32 {
+    assert!((((({ a_foo_2() }) == 1) as i32) != 0));
+    assert!((((({ b_foo_4() }) == 1.0E+0) as i32) != 0));
+    assert!((((({ a_bar_3() }) == 5) as i32) != 0));
+    assert!((((({ b_bar_5() }) == 6) as i32) != 0));
+    return 0;
+}
+thread_local!(
+    pub static same_name_different_type_6: Value<f32> = Rc::new(RefCell::new(1.0E+0));
+);
+thread_local!(
+    pub static same_name_same_type_7: Value<i32> = Rc::new(RefCell::new(6));
+);
+pub fn b_foo_4() -> f32 {
+    return (*same_name_different_type_6.with(Value::clone).borrow());
+}
+pub fn b_bar_5() -> i32 {
+    return (*same_name_same_type_7.with(Value::clone).borrow());
+}

tests/multi-file/static_name_collision/out/unsafe/static_name_collision.rs

@@ -0,0 +1,36 @@
+extern crate libc;
+use libc::*;
+extern crate libcc2rs;
+use libcc2rs::*;
+use std::collections::BTreeMap;
+use std::io::{Read, Seek, Write};
+use std::os::fd::{AsFd, FromRawFd, IntoRawFd};
+use std::rc::Rc;
+pub static mut same_name_different_type_0: i32 = unsafe { 1 };
+pub static mut same_name_same_type_1: i32 = unsafe { 5 };
+pub unsafe fn a_foo_2() -> i32 {
+    return same_name_different_type_0;
+}
+pub unsafe fn a_bar_3() -> i32 {
+    return same_name_same_type_1;
+}
+pub fn main() {
+    unsafe {
+        std::process::exit(main_0() as i32);
+    }
+}
+unsafe fn main_0() -> i32 {
+    assert!(((((unsafe { a_foo_2() }) == (1)) as i32) != 0));
+    assert!(((((unsafe { b_foo_4() }) == (1.0E+0)) as i32) != 0));
+    assert!(((((unsafe { a_bar_3() }) == (5)) as i32) != 0));
+    assert!(((((unsafe { b_bar_5() }) == (6)) as i32) != 0));
+    return 0;
+}
+pub static mut same_name_different_type_6: f32 = unsafe { 1.0E+0 };
+pub static mut same_name_same_type_7: i32 = unsafe { 6 };
+pub unsafe fn b_foo_4() -> f32 {
+    return same_name_different_type_6;
+}
+pub unsafe fn b_bar_5() -> i32 {
+    return same_name_same_type_7;
+}

tests/unit/out/refcount/addr_of_global.rs

@@ -42,75 +42,75 @@ impl Clone for Outer {
 }
 impl ByteRepr for Outer {}
 thread_local!(
-    pub static alpha: Value<Inner> = Rc::new(RefCell::new(Inner {
+    pub static alpha_0: Value<Inner> = Rc::new(RefCell::new(Inner {
         value: Rc::new(RefCell::new(1)),
     }));
 );
 thread_local!(
-    pub static beta: Value<Inner> = Rc::new(RefCell::new(Inner {
+    pub static beta_1: Value<Inner> = Rc::new(RefCell::new(Inner {
         value: Rc::new(RefCell::new(2)),
     }));
 );
 thread_local!(
-    pub static shared: Value<Inner> = Rc::new(RefCell::new(Inner {
+    pub static shared_2: Value<Inner> = Rc::new(RefCell::new(Inner {
         value: Rc::new(RefCell::new(42)),
     }));
 );
 thread_local!(
-    pub static items: Value<Box<[Ptr<Inner>]>> = Rc::new(RefCell::new(Box::new([
-        (alpha.with(Value::clone).as_pointer()),
-        (beta.with(Value::clone).as_pointer()),
+    pub static items_3: Value<Box<[Ptr<Inner>]>> = Rc::new(RefCell::new(Box::new([
+        (alpha_0.with(Value::clone).as_pointer()),
+        (beta_1.with(Value::clone).as_pointer()),
     ])));
 );
 thread_local!(
-    pub static obj: Value<Outer> = Rc::new(RefCell::new(Outer {
-        p: Rc::new(RefCell::new((shared.with(Value::clone).as_pointer()))),
+    pub static obj_4: Value<Outer> = Rc::new(RefCell::new(Outer {
+        p: Rc::new(RefCell::new((shared_2.with(Value::clone).as_pointer()))),
     }));
 );
 pub fn main() {
     std::process::exit(main_0());
 }
 fn main_0() -> i32 {
     assert!(
-        ((*(*(*items.with(Value::clone).borrow())[(0) as usize]
+        ((*(*(*items_3.with(Value::clone).borrow())[(0) as usize]
             .upgrade()
             .deref())
         .value
         .borrow())
             == 1)
     );
     assert!(
-        ((*(*(*items.with(Value::clone).borrow())[(1) as usize]
+        ((*(*(*items_3.with(Value::clone).borrow())[(1) as usize]
             .upgrade()
             .deref())
         .value
         .borrow())
             == 2)
     );
     assert!(
-        ((*(*(*(*obj.with(Value::clone).borrow()).p.borrow())
+        ((*(*(*(*obj_4.with(Value::clone).borrow()).p.borrow())
             .upgrade()
             .deref())
         .value
         .borrow())
             == 42)
     );
     thread_local!(
-        static cache: Value<Box<[Ptr<Inner>]>> = Rc::new(RefCell::new(Box::new([
-            (alpha.with(Value::clone).as_pointer()),
-            (beta.with(Value::clone).as_pointer()),
+        static cache_5: Value<Box<[Ptr<Inner>]>> = Rc::new(RefCell::new(Box::new([
+            (alpha_0.with(Value::clone).as_pointer()),
+            (beta_1.with(Value::clone).as_pointer()),
         ])));
     );
     assert!(
-        ((*(*(*cache.with(Value::clone).borrow())[(0) as usize]
+        ((*(*(*cache_5.with(Value::clone).borrow())[(0) as usize]
             .upgrade()
             .deref())
         .value
         .borrow())
             == 1)
     );
     assert!(
-        ((*(*(*cache.with(Value::clone).borrow())[(1) as usize]
+        ((*(*(*cache_5.with(Value::clone).borrow())[(1) as usize]
             .upgrade()
             .deref())
         .value

tests/unit/out/refcount/bool_condition_logical.rs

@@ -25,17 +25,17 @@ impl From<i32> for Code {
 }
 libcc2rs::impl_enum_inc_dec!(Code);
 thread_local!(
-    pub static side_effect: Value<i32> = Rc::new(RefCell::new(0));
+    pub static side_effect_0: Value<i32> = Rc::new(RefCell::new(0));
 );
-pub fn observe_0(v: i32) -> i32 {
+pub fn observe_1(v: i32) -> i32 {
     let v: Value<i32> = Rc::new(RefCell::new(v));
-    (*side_effect.with(Value::clone).borrow_mut()).prefix_inc();
+    (*side_effect_0.with(Value::clone).borrow_mut()).prefix_inc();
     return (*v.borrow());
 }
-pub fn returns_one_1() -> i32 {
+pub fn returns_one_2() -> i32 {
     return 1;
 }
-pub fn returns_zero_2() -> i32 {
+pub fn returns_zero_3() -> i32 {
     return 0;
 }
 pub fn main() {
@@ -66,25 +66,25 @@ fn main_0() -> i32 {
     {
         assert!(true);
     }
-    (*side_effect.with(Value::clone).borrow_mut()) = 0;
+    (*side_effect_0.with(Value::clone).borrow_mut()) = 0;
     if ((*zero.borrow()) != 0)
         && (({
             let _v: i32 = 1;
-            observe_0(_v)
+            observe_1(_v)
         }) != 0)
     {
         assert!(false);
     }
-    assert!(((*side_effect.with(Value::clone).borrow()) == 0));
+    assert!(((*side_effect_0.with(Value::clone).borrow()) == 0));
     if ((*n.borrow()) != 0)
         || (({
             let _v: i32 = 1;
-            observe_0(_v)
+            observe_1(_v)
         }) != 0)
     {
         assert!(true);
     }
-    assert!(((*side_effect.with(Value::clone).borrow()) == 0));
+    assert!(((*side_effect_0.with(Value::clone).borrow()) == 0));
     let x: Value<i32> = Rc::new(RefCell::new(5));
     let y: Value<i32> = Rc::new(RefCell::new(3));
     let flags: Value<u32> = Rc::new(RefCell::new(2_u32));
@@ -138,19 +138,19 @@ fn main_0() -> i32 {
     if ((*x.borrow()) > (*y.borrow())) && (((*n.borrow()) != 0) && (!(*cp.borrow()).is_null())) {
         assert!(true);
     }
-    if ((*x.borrow()) > (*y.borrow())) && (({ returns_one_1() }) != 0) {
+    if ((*x.borrow()) > (*y.borrow())) && (({ returns_one_2() }) != 0) {
         assert!(true);
     }
-    if ((*x.borrow()) > (*y.borrow())) && (!(({ returns_zero_2() }) != 0)) {
+    if ((*x.borrow()) > (*y.borrow())) && (!(({ returns_zero_3() }) != 0)) {
         assert!(true);
     }
-    if ((*x.borrow()) < (*y.borrow())) || (({ returns_one_1() }) != 0) {
+    if ((*x.borrow()) < (*y.borrow())) || (({ returns_one_2() }) != 0) {
         assert!(true);
     }
-    if ((*x.borrow()) < (*y.borrow())) || (!(({ returns_one_1() }) != 0)) {
+    if ((*x.borrow()) < (*y.borrow())) || (!(({ returns_one_2() }) != 0)) {
         assert!(false);
     }
-    if ((!((*p.borrow()).is_null())) && (({ returns_one_1() }) != 0)) && ((*n.borrow()) != 0) {
+    if ((!((*p.borrow()).is_null())) && (({ returns_one_2() }) != 0)) && ((*n.borrow()) != 0) {
         assert!(true);
     }
     return 0;