Convert late instantiated methods on Ptr<Self> instead of self (#334)

After #333, late instantiated methods from templates were not added in
the `impl X for Ptr<Self>` block. For exmaple:

```cpp
// 2 methods, each instantiated in separate TUs. Second TU got it wrong
template <typename T>
struct S {
  T x = 0;

  // visited in first TU: correctly added in `trait SImpl` and `impl SImpl for Ptr<Self>`
  void set(T v) { x = v; }
  // visited in the second TU: not added in `trait SImpl` at all and incorrectly added
  // in `impl S` instead of `impl SImpl for Ptr<Self>`
  T get() { return x; }
};
```

This PR solves it by also deferring trait blocks at the end of the file
and adding late instantiated methods in both `trait SImpl` and `impl
SImpl for Ptr<Self>`.

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -26,8 +26,7 @@ std::unordered_set<std::string> Converter::decl_ids_;
 std::unordered_set<std::string> Converter::globals_;
 std::unordered_set<std::string> Converter::abstract_structs_;
 Converter::RecordIndex Converter::record_decls_;
-std::map<Converter::DeferredImplHeader, Converter::DeferredImplBody>
-    Converter::deferred_impls_;
+std::map<std::string, Converter::MethodsOnPtr> Converter::methods_on_ptr_;
 
 void Converter::ConvertUniquePtrDeref(clang::CXXOperatorCallExpr *expr) {
   bool is_star = expr->getOperator() == clang::OverloadedOperatorKind::OO_Star;
@@ -58,12 +57,16 @@ use std::rc::Rc;
 )");
 }
 
-std::string Converter::EmitDeferredImpls() {
+std::string Converter::EmitMethodsOnPtr() {
   std::string out;
-  for (const auto &[header, body] : deferred_impls_) {
-    out += header;
+  for (const auto &[name, methods] : methods_on_ptr_) {
+    out += methods.trait_header;
     out += " {\n";
-    out += body;
+    out += methods.trait_body;
+    out += "}\n";
+    out += methods.impl_header;
+    out += " {\n";
+    out += methods.impl_body;
     out += "}\n";
   }
   return out;
@@ -855,6 +858,15 @@ void Converter::EmitRustStructOrUnion(clang::RecordDecl *decl) {
   AddByteReprTrait(decl);
 }
 
+void Converter::ConvertLateInstantiatedMethods(clang::CXXRecordDecl *decl) {
+  ConvertCXXMethodDecls(
+      decl, std::format("{} {}", keyword::kImpl, GetRecordName(decl)),
+      [](auto *method) {
+        return IsEmittableMethod(method) && method->hasBody() &&
+               !decl_ids_.contains(GetMethodID(method));
+      });
+}
+
 void Converter::ConvertCXXRecordMethods(clang::CXXRecordDecl *decl) {
   ConvertCXXMethodDecls(
       decl, std::format("{} {}", keyword::kImpl, GetRecordName(decl)),
@@ -941,12 +953,7 @@ bool Converter::VisitCXXRecordDecl(clang::CXXRecordDecl *decl) {
     if (!record_decls_.MarkDefined(GetRecordName(decl))) {
       // Other translation units may instantiate members this one did not.
       if (clang::isa<clang::ClassTemplateSpecializationDecl>(decl)) {
-        ConvertCXXMethodDecls(
-            decl, std::format("{} {}", keyword::kImpl, GetRecordName(decl)),
-            [](auto *method) {
-              return IsEmittableMethod(method) && method->hasBody() &&
-                     !decl_ids_.contains(GetMethodID(method));
-            });
+        ConvertLateInstantiatedMethods(decl);
       }
       return false;
     }

cpp2rust/converter/converter.h

@@ -54,7 +54,7 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
 
   static std::string EmitOpaqueRecords();
 
-  static std::string EmitDeferredImpls();
+  static std::string EmitMethodsOnPtr();
 
   virtual bool VisitBuiltinType(clang::BuiltinType *type);
 
@@ -133,6 +133,7 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
   std::string GetMethodName(const clang::CXXMethodDecl *decl);
   virtual std::string GetSelfMaybeWithMut(const clang::CXXMethodDecl *decl);
   virtual void ConvertCXXRecordMethods(clang::CXXRecordDecl *decl);
+  virtual void ConvertLateInstantiatedMethods(clang::CXXRecordDecl *decl);
   virtual std::string DestroyMembers(const clang::CXXRecordDecl *decl);
   virtual void EmitScopedDestructor(const clang::VarDecl *decl);
   void EmitDeallocation(clang::CXXDeleteExpr *expr,
@@ -846,9 +847,15 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
     std::unordered_map<std::string, bool> entries_;
   };
   static RecordIndex record_decls_;
-  using DeferredImplHeader = std::string;
-  using DeferredImplBody = std::string;
-  static std::map<DeferredImplHeader, DeferredImplBody> deferred_impls_;
+  struct MethodsOnPtr {
+    std::string trait_header;
+    std::string trait_body;
+    std::string impl_header;
+    std::string impl_body;
+  };
+  // record name -> trait and impl for Ptr<record>, emitted after all
+  // translation units.
+  static std::map<std::string, MethodsOnPtr> methods_on_ptr_;
 
   enum class ExprKind : uint8_t {
     Callee,

cpp2rust/converter/models/converter_refcount.cpp

@@ -2604,10 +2604,16 @@ ConverterRefCount::TraitName(const clang::CXXRecordDecl *decl) const {
   return GetRecordName(decl) + "Impl";
 }
 
-std::string
-ConverterRefCount::ImplHeader(const clang::CXXRecordDecl *decl) const {
-  return std::format("impl {} for Ptr<{}>", TraitName(decl),
-                     GetRecordName(decl));
+Converter::MethodsOnPtr &
+ConverterRefCount::MethodsOnPtrFor(const clang::CXXRecordDecl *decl) {
+  auto name = GetRecordName(decl);
+  auto [it, inserted] = methods_on_ptr_.try_emplace(name);
+  if (inserted) {
+    it->second.trait_header = std::format("pub trait {}", TraitName(decl));
+    it->second.impl_header =
+        std::format("impl {} for Ptr<{}>", TraitName(decl), name);
+  }
+  return it->second;
 }
 
 bool ConverterRefCount::ConvertOutOfLineMethod(clang::CXXMethodDecl *decl) {
@@ -2619,10 +2625,49 @@ bool ConverterRefCount::ConvertOutOfLineMethod(clang::CXXMethodDecl *decl) {
     PushMethodTarget push(*this, MethodTarget::PtrImpl);
     ConvertCXXMethodDecl(decl);
   }
-  deferred_impls_[ImplHeader(decl->getParent())] += std::move(buf).str();
+  MethodsOnPtrFor(decl->getParent()).impl_body += std::move(buf).str();
   return false;
 }
 
+void ConverterRefCount::ConvertMethodOnPtr(clang::CXXMethodDecl *method) {
+  auto *record = method->getParent();
+  {
+    Buffer buf(*this);
+    {
+      PushCurrFunction push_fn(*this, method);
+      PushMethodTarget push(*this, MethodTarget::TraitDecl);
+      ConvertCXXMethodDecl(method);
+    }
+    MethodsOnPtrFor(record).trait_body += std::move(buf).str();
+  }
+  if (!method->isThisDeclarationADefinition()) {
+    return;
+  }
+  Buffer buf(*this);
+  {
+    PushMethodTarget push(*this, MethodTarget::PtrImpl);
+    VisitCXXMethodDecl(method);
+  }
+  MethodsOnPtrFor(record).impl_body += std::move(buf).str();
+}
+
+void ConverterRefCount::ConvertLateInstantiatedMethods(
+    clang::CXXRecordDecl *decl) {
+  Converter::ConvertCXXMethodDecls(
+      decl, std::format("{} {}", keyword::kImpl, GetRecordName(decl)),
+      [](auto *method) {
+        return IsEmittableMethod(method) && method->hasBody() &&
+               !IsMethodOnPtr(method) &&
+               !decl_ids_.contains(GetMethodID(method));
+      });
+  for (auto *method : decl->methods()) {
+    if (IsEmittableMethod(method) && method->hasBody() &&
+        IsMethodOnPtr(method) && !decl_ids_.contains(GetMethodID(method))) {
+      ConvertMethodOnPtr(method);
+    }
+  }
+}
+
 void ConverterRefCount::ConvertCXXRecordMethods(clang::CXXRecordDecl *decl) {
   auto struct_name = GetRecordName(decl);
 
@@ -2632,42 +2677,16 @@ void ConverterRefCount::ConvertCXXRecordMethods(clang::CXXRecordDecl *decl) {
                                  !IsMethodOnPtr(method);
                         });
 
-  bool synthesize_dtor =
-      !GetUserDefinedDestructor(decl) && HasFieldsNeedingDestruction(decl);
-  if (!synthesize_dtor &&
-      !std::ranges::any_of(decl->methods(), [](auto *method) {
-        return IsMethodOnPtr(method) && method->getDefinition();
-      })) {
-    return;
-  }
-
-  auto header = ImplHeader(decl);
-  StrCat(keyword::kPub, keyword::kTrait, TraitName(decl));
-  PushBrace trait_brace(*this);
-
   for (auto *method : decl->methods()) {
-    if (!IsMethodOnPtr(method) || !method->getDefinition()) {
-      continue;
-    }
-    {
-      PushCurrFunction push_fn(*this, method);
-      PushMethodTarget push(*this, MethodTarget::TraitDecl);
-      ConvertCXXMethodDecl(method);
-    }
-    if (!method->isThisDeclarationADefinition()) {
-      continue;
-    }
-    Buffer buf(*this);
-    {
-      PushMethodTarget push(*this, MethodTarget::PtrImpl);
-      VisitCXXMethodDecl(method);
+    if (IsMethodOnPtr(method) && method->getDefinition()) {
+      ConvertMethodOnPtr(method);
     }
-    deferred_impls_[header] += std::move(buf).str();
   }
 
-  if (synthesize_dtor) {
-    StrCat(std::format("fn {}(&self)", kDestructorName), token::kSemiColon);
-    deferred_impls_[header] += std::format(
+  if (!GetUserDefinedDestructor(decl) && HasFieldsNeedingDestruction(decl)) {
+    MethodsOnPtrFor(decl).trait_body +=
+        std::format("fn {}(&self);\n", kDestructorName);
+    MethodsOnPtrFor(decl).impl_body += std::format(
         "fn {}(&self) {{ {} }}\n", kDestructorName, DestroyMembers(decl));
   }
 }

cpp2rust/converter/models/converter_refcount.h

@@ -60,6 +60,10 @@ class ConverterRefCount final : public Converter {
 
   void ConvertCXXRecordMethods(clang::CXXRecordDecl *decl) override;
 
+  void ConvertLateInstantiatedMethods(clang::CXXRecordDecl *decl) override;
+
+  void ConvertMethodOnPtr(clang::CXXMethodDecl *method);
+
   bool VisitCXXThisExpr(clang::CXXThisExpr *expr) override;
 
   bool ThisIsRustPtr() const override;
@@ -215,7 +219,7 @@ class ConverterRefCount final : public Converter {
 
 private:
   std::string TraitName(const clang::CXXRecordDecl *decl) const;
-  std::string ImplHeader(const clang::CXXRecordDecl *decl) const;
+  MethodsOnPtr &MethodsOnPtrFor(const clang::CXXRecordDecl *decl);
   std::string DestroyMembers(const clang::CXXRecordDecl *decl) override;
   void EmitScopedDestructor(const clang::VarDecl *decl) override;
 

cpp2rust/cpp2rust_lib.cpp

@@ -31,7 +31,7 @@ std::string TranspileSrc(std::string_view cc_code, Model model,
       cc_code, tool_args, std::filesystem::path(filename).filename().string(),
       filename.ends_with(".c") ? CLANG_C_COMPILER : CLANG_CXX_COMPILER);
   rs_code += Converter::EmitOpaqueRecords();
-  rs_code += Converter::EmitDeferredImpls();
+  rs_code += Converter::EmitMethodsOnPtr();
   return rs_code;
 }
 
@@ -72,7 +72,7 @@ std::string TranspileDir(std::string_view build_dir, Model model,
   FrontendActionFactory factory(rs_code, model, rules_dir);
   Tool.run(&factory);
   rs_code += Converter::EmitOpaqueRecords();
-  rs_code += Converter::EmitDeferredImpls();
+  rs_code += Converter::EmitMethodsOnPtr();
   return rs_code;
 }
 } // namespace cpp2rust

tests/multi-file/template_method_late_instantiation/CMakeLists.txt

@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.16)
+project(template_method_late_instantiation LANGUAGES CXX)
+add_executable(app a.cpp b.cpp)

tests/multi-file/template_method_late_instantiation/a.cpp

@@ -0,0 +1,10 @@
+#include <assert.h>
+
+#include "s.h"
+
+int main() {
+  S<int> p;
+  p.set(3);
+  assert(f(&p) == 3);
+  return 0;
+}

tests/multi-file/template_method_late_instantiation/b.cpp

@@ -0,0 +1,3 @@
+#include "s.h"
+
+int f(S<int> *p) { return p->get(); }

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

@@ -0,0 +1,60 @@
+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 S_int_ {
+    pub x: Value<i32>,
+}
+impl Clone for S_int_ {
+    fn clone(&self) -> Self {
+        let __this: Value<S_int_> = Rc::new(RefCell::new(Self {
+            x: Rc::new(RefCell::new((*self.x.borrow()))),
+        }));
+        let this: Ptr<S_int_> = __this.as_pointer();
+        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+    }
+}
+impl ByteRepr for S_int_ {
+    fn byte_size() -> usize {
+        4
+    }
+    fn to_bytes(&self, buf: &mut [u8]) {
+        (*self.x.borrow()).to_bytes(&mut buf[0..4]);
+    }
+    fn from_bytes(buf: &[u8]) -> Self {
+        Self {
+            x: Rc::new(RefCell::new(<i32>::from_bytes(&buf[0..4]))),
+        }
+    }
+}
+pub fn main() {
+    std::process::exit(main_0());
+}
+fn main_0() -> i32 {
+    let p: Value<S_int_> = Rc::new(RefCell::new(<S_int_>::default()));
+    ({ S_int_Impl::set(&p.as_pointer(), 3) });
+    assert!((({ f_0((p.as_pointer()),) }) == 3));
+    return 0;
+}
+pub fn f_0(p: Ptr<S_int_>) -> i32 {
+    let p: Value<Ptr<S_int_>> = Rc::new(RefCell::new(p));
+    return ({ S_int_Impl::get(&(*p.borrow())) });
+}
+pub trait S_int_Impl {
+    fn set(&self, v: i32);
+    fn get(&self) -> i32;
+}
+impl S_int_Impl for Ptr<S_int_> {
+    fn set(&self, v: i32) {
+        let v: Value<i32> = Rc::new(RefCell::new(v));
+        (*(*(*self).upgrade().deref()).x.borrow_mut()) = (*v.borrow());
+    }
+    fn get(&self) -> i32 {
+        return (*(*(*self).upgrade().deref()).x.borrow());
+    }
+}

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

@@ -0,0 +1,37 @@
+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 S_int_ {
+    pub x: i32,
+}
+impl S_int_ {
+    pub unsafe fn set(&mut self, mut v: i32) {
+        self.x = v;
+    }
+}
+pub fn main() {
+    unsafe {
+        std::process::exit(main_0() as i32);
+    }
+}
+unsafe fn main_0() -> i32 {
+    let mut p: S_int_ = <S_int_>::default();
+    (unsafe { S_int_::set(&mut p, 3) });
+    assert!(((unsafe { f_0((&mut p as *mut S_int_),) }) == (3)));
+    return 0;
+}
+impl S_int_ {
+    pub unsafe fn get(&mut self) -> i32 {
+        return self.x;
+    }
+}
+pub unsafe fn f_0(mut p: *mut S_int_) -> i32 {
+    return (unsafe { S_int_::get(&mut (*p)) });
+}