Fix opaque then defined pattern

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -19,11 +19,10 @@
 
 namespace cpp2rust {
 std::unordered_map<std::string, std::string> Converter::inner_structs_;
-std::unordered_set<std::string> Converter::record_decls_;
 std::unordered_set<std::string> Converter::decl_ids_;
 std::unordered_set<std::string> Converter::globals_;
 std::unordered_set<std::string> Converter::abstract_structs_;
-std::unordered_map<std::string, std::string> Converter::referenced_records_;
+Converter::RecordIndex Converter::record_decls_;
 
 void Converter::ConvertUniquePtrDeref(clang::CXXOperatorCallExpr *expr) {
   bool is_star = expr->getOperator() == clang::OverloadedOperatorKind::OO_Star;
@@ -56,14 +55,11 @@ use std::rc::Rc;
 
 std::string Converter::EmitOpaqueRecordMarkers() {
   std::string out;
-  for (const auto &[id, name] : referenced_records_) {
-    if (record_decls_.contains(id)) {
-      continue;
-    }
+  record_decls_.ForEachUndefined([&](const std::string &name) {
     out += "#[repr(C)] pub struct ";
     out += name;
     out += " { _opaque: [u8; 0] }\n";
-  }
+  });
   return out;
 }
 
@@ -177,7 +173,7 @@ bool Converter::VisitRecordType(clang::RecordType *type) {
   auto name = GetRecordName(decl);
   StrCat(name);
   if (!ctx_.getSourceManager().isInSystemHeader(decl->getLocation())) {
-    referenced_records_.try_emplace(GetID(decl), std::move(name));
+    record_decls_.MarkReferenced(std::move(name));
   }
   Mapper::AddRuleForUserDefinedType(decl);
   return false;
@@ -635,7 +631,7 @@ bool Converter::VisitRecordDecl(clang::RecordDecl *decl) {
     return false;
   }
 
-  if (!record_decls_.insert(GetID(decl)).second) {
+  if (!record_decls_.MarkDefined(GetRecordName(decl))) {
     return false;
   }
 
@@ -752,7 +748,7 @@ bool Converter::VisitCXXRecordDecl(clang::CXXRecordDecl *decl) {
       }
     }
 
-    if (!record_decls_.insert(GetID(decl)).second) {
+    if (!record_decls_.MarkDefined(GetRecordName(decl))) {
       return false;
     }
 

cpp2rust/converter/converter.h

@@ -51,8 +51,6 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
 
   virtual void EmitFilePreamble();
 
-  // Returns Rust text emitting `pub struct N { _opaque: [u8; 0] }` for each
-  // record name referenced by some TU but never defined by any.
   static std::string EmitOpaqueRecordMarkers();
 
   virtual bool VisitBuiltinType(clang::BuiltinType *type);
@@ -648,9 +646,37 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
     ~ScopedMapIterDecl() { c.map_iter_decls_.erase(decl); }
   };
   static std::unordered_set<std::string> decl_ids_;
-  static std::unordered_set<std::string> record_decls_;
   static std::unordered_set<std::string> abstract_structs_;
-  static std::unordered_map<std::string, std::string> referenced_records_;
+
+  class RecordIndex {
+  public:
+    void MarkReferenced(std::string name) {
+      entries_.try_emplace(std::move(name), false);
+    }
+    // Returns false if `name` is already defined; otherwise marks it and
+    // returns true.
+    bool MarkDefined(const std::string &name) {
+      bool &defined = entries_[name];
+      if (defined) {
+        return false;
+      }
+      defined = true;
+      return true;
+    }
+    template <typename F> void ForEachUndefined(F &&f) const {
+      for (const auto &[name, defined] : entries_) {
+        if (!defined) {
+          f(name);
+        }
+      }
+    }
+
+  private:
+    // record name -> true if a definition has been emitted, false if only
+    // referenced.
+    std::unordered_map<std::string, bool> entries_;
+  };
+  static RecordIndex record_decls_;
 
   enum class ExprKind : uint8_t {
     Callee,

tests/multi-file/opaque_forward_decl/b.c

@@ -1,6 +1,3 @@
 #include "header.h"
 
-void touch(struct container *c) {
-  /* read the opaque pointer field without dereferencing the opaque type */
-  (void)c->p;
-}
+void touch(struct container *c) { (void)c->p; }

tests/multi-file/opaque_forward_decl/header.h

@@ -1,6 +1,5 @@
 #pragma once
 
-/* Forward declaration only; no full definition exists anywhere in the build. */
 struct opaque;
 
 struct container {

tests/unit/opaque_forward_decl.c

@@ -1,6 +1,5 @@
 #include <stddef.h>
 
-/* Forward declaration only; no body for `opaque` anywhere in this TU. */
 struct opaque;
 
 struct container {

tests/unit/opaque_then_defined.c

@@ -0,0 +1,21 @@
+#include <assert.h>
+
+struct node;
+
+struct list {
+  struct node *head;
+  int size;
+};
+
+struct node {
+  int value;
+  struct node *next;
+};
+
+int main(void) {
+  struct node n = {42, 0};
+  struct list l = {&n, 1};
+  assert(l.head->value == 42);
+  assert(l.size == 1);
+  return 0;
+}

tests/unit/out/refcount/opaque_then_defined.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};
+#[derive(Default)]
+pub struct list {
+    pub head: Value<Ptr<node>>,
+    pub size: Value<i32>,
+}
+impl ByteRepr for list {}
+#[derive(Default)]
+pub struct node {
+    pub value: Value<i32>,
+    pub next: Value<Ptr<node>>,
+}
+impl ByteRepr for node {}
+pub fn main() {
+    std::process::exit(main_0());
+}
+fn main_0() -> i32 {
+    let n: Value<node> = Rc::new(RefCell::new(node {
+        value: Rc::new(RefCell::new(42)),
+        next: Rc::new(RefCell::new(Ptr::<node>::null())),
+    }));
+    let l: Value<list> = Rc::new(RefCell::new(list {
+        head: Rc::new(RefCell::new((n.as_pointer()))),
+        size: Rc::new(RefCell::new(1)),
+    }));
+    assert!(
+        ((((*(*(*(*l.borrow()).head.borrow()).upgrade().deref())
+            .value
+            .borrow())
+            == 42) as i32)
+            != 0)
+    );
+    assert!(((((*(*l.borrow()).size.borrow()) == 1) as i32) != 0));
+    return 0;
+}

tests/unit/out/unsafe/opaque_then_defined.rs

@@ -0,0 +1,38 @@
+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;
+#[repr(C)]
+#[derive(Copy, Clone, Default)]
+pub struct list {
+    pub head: *mut node,
+    pub size: i32,
+}
+#[repr(C)]
+#[derive(Copy, Clone, Default)]
+pub struct node {
+    pub value: i32,
+    pub next: *mut node,
+}
+pub fn main() {
+    unsafe {
+        std::process::exit(main_0() as i32);
+    }
+}
+unsafe fn main_0() -> i32 {
+    let mut n: node = node {
+        value: 42,
+        next: std::ptr::null_mut(),
+    };
+    let mut l: list = list {
+        head: (&mut n as *mut node),
+        size: 1,
+    };
+    assert!((((((*l.head).value) == (42)) as i32) != 0));
+    assert!(((((l.size) == (1)) as i32) != 0));
+    return 0;
+}