Only translate used methods of a template class (#329)

cpp2rust materializes template specializations in order to have access
to defaulted copy or move constructors.

However, materializing all the methods of a class is wrong. Some methods
might not satisfy the constraints of the type that specializes the
class, for example:

```cpp
struct Point {
  int x;
};

template <typename T> struct Box {
  T val;
  T get() { return val; }

  // This only works for T's that have operator+.
  // int is fine, Point is not.
  T twice() { return val + val; }
};
```

Specializing `Box` with `int` satisfies both get and twice, however
`Point` only satisfies get.

This PR only materializes methods that are used by the specialization.

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -626,22 +626,6 @@ static bool hasUserDefinedNonDefaultCopyOrMoveCtor(clang::CXXRecordDecl *decl) {
   return false;
 }
 
-void Converter::materializeTemplateSpecialization(clang::CXXRecordDecl *decl) {
-  for (auto method : decl->methods()) {
-    const clang::FunctionDecl *definition = nullptr;
-    if (method->isDefined(definition)) {
-      continue;
-    }
-
-    if (auto pattern = method->getTemplateInstantiationPattern()) {
-      if (pattern->doesThisDeclarationHaveABody()) {
-        sema_->InstantiateFunctionDefinition(method->getLocation(), method,
-                                             /*Recursive=*/true);
-      }
-    }
-  }
-}
-
 bool IsPointerType(clang::QualType qual_type) {
   return qual_type->isPointerType() ||
          (qual_type->isArrayType() &&
@@ -750,6 +734,23 @@ bool Converter::VisitRecordDecl(clang::RecordDecl *decl) {
   return false;
 }
 
+static bool IsEmittableMethod(clang::CXXMethodDecl *method) {
+  // Virtual methods go into the base trait impl, destructors into Drop
+  if (method->isVirtual() || clang::isa<clang::CXXDestructorDecl>(method)) {
+    return false;
+  }
+  // Compiler-generated members are covered by derived traits
+  if (method->isImplicit()) {
+    return false;
+  }
+  if (auto *definition = method->getDefinition();
+      definition && definition->isDefaulted()) {
+    return false;
+  }
+  return method->isThisDeclarationADefinition() ||
+         clang::isa<clang::CXXConstructorDecl>(method);
+}
+
 void Converter::EmitRustStructOrUnion(clang::RecordDecl *decl) {
   // Enums and static variables. In rust they live outside the record
   for (auto *d : decl->decls()) {
@@ -809,17 +810,9 @@ void Converter::EmitRustStructOrUnion(clang::RecordDecl *decl) {
   if (auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl)) {
     auto struct_name = GetRecordName(cxx);
 
-    ConvertCXXMethodDecls(
-        cxx, std::format("{} {}", keyword::kImpl, struct_name),
-        [](auto *method) {
-          return !method->isImplicit() &&
-                 !(method->getDefinition() &&
-                   method->getDefinition()->isDefaulted()) &&
-                 (method->isThisDeclarationADefinition() ||
-                  clang::isa<clang::CXXConstructorDecl>(method)) &&
-                 !method->isVirtual() &&
-                 !clang::isa<clang::CXXDestructorDecl>(method);
-        });
+    ConvertCXXMethodDecls(cxx,
+                          std::format("{} {}", keyword::kImpl, struct_name),
+                          IsEmittableMethod);
 
     if (cxx->bases_begin() != cxx->bases_end()) {
       ConvertCXXMethodDecls(
@@ -867,10 +860,6 @@ void Converter::EmitRustUnion(clang::RecordDecl *decl) {
 }
 
 bool Converter::VisitCXXRecordDecl(clang::CXXRecordDecl *decl) {
-  if (clang::isa<clang::ClassTemplateSpecializationDecl>(decl)) {
-    materializeTemplateSpecialization(decl);
-  }
-
   decl->dump(log());
 
   Mapper::AddRuleForUserDefinedType(decl);
@@ -888,6 +877,15 @@ 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));
+            });
+      }
       return false;
     }
 
@@ -927,6 +925,12 @@ bool Converter::VisitCXXMethodDecl(clang::CXXMethodDecl *decl) {
   if (!IsConvertibleCXXMethodDecl(decl)) {
     return false;
   }
+  if (!decl->isPureVirtual() && !decl->hasBody()) {
+    return false;
+  }
+  if (!decl_ids_.insert(GetMethodID(decl)).second) {
+    return false;
+  }
   curr_function_ = decl;
 
   if (decl->isOutOfLine() && !decl->overridden_methods().empty()) {

cpp2rust/converter/converter.h

@@ -931,8 +931,6 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
   bool IsCastRedundantInRust(clang::Expr *expr, clang::QualType target_type);
 
 private:
-  void materializeTemplateSpecialization(clang::CXXRecordDecl *decl);
-
   std::string getIntegerLiteral(clang::IntegerLiteral *expr, bool incl_type,
                                 const clang::QualType *type = nullptr);
   const char *keyword_unsafe_;

cpp2rust/converter/converter_lib.cpp

@@ -262,7 +262,9 @@ bool IsConvertibleCXXRecordDecl(const clang::CXXRecordDecl *decl) {
   return decl->isThisDeclarationADefinition() &&
          std::all_of(
              decl->method_begin(), decl->method_end(), [](auto *method) {
-               return method->getDefinition() || method->isPureVirtual();
+               return method->getDefinition() || method->isPureVirtual() ||
+                      method->getTemplateInstantiationPattern() ||
+                      method->getDescribedFunctionTemplate();
              });
 }
 
@@ -427,6 +429,10 @@ std::string GetID(const clang::Decl *decl) {
   return GetLocationID(decl) + GetParamSignature(decl);
 }
 
+std::string GetMethodID(const clang::CXXMethodDecl *decl) {
+  return decl->getQualifiedNameAsString() + GetID(decl);
+}
+
 std::string DisambiguateAnonymousTag(const clang::TagDecl *tag) {
   if (!tag) {
     return "";

cpp2rust/converter/converter_lib.h

@@ -99,6 +99,7 @@ unsigned GetColumnNumber(const clang::Decl *decl);
 unsigned GetArraySize(clang::QualType array_type);
 
 std::string GetID(const clang::Decl *decl);
+std::string GetMethodID(const clang::CXXMethodDecl *decl);
 
 std::string GetNamedDeclAsString(const clang::NamedDecl *decl);
 

tests/unit/class_template_lazy_member.cpp

@@ -0,0 +1,31 @@
+#include <cassert>
+
+// This tests that cpp2rust respects the lazy instantiation of templates. On
+// Box<int> and Box<Point> declarations, only the class template and data
+// memebers are instantiated.
+//
+// Then, each used method is lazyly instantiated when it's used, i.e. twice()
+// is not instantiated for Box<Point>.
+
+struct Point {
+  int x;
+};
+
+template <typename T> struct Box {
+  T val;
+  T get() { return val; }
+
+  // This only works for T's that have operator+.
+  // int is fine, Point is not.
+  T twice() { return val + val; }
+};
+
+int main() {
+  Box<int> i = {3};
+  assert(i.twice() == 6);
+
+  Box<Point> p = {{4}};
+  assert(p.get().x == 4);
+
+  return 0;
+}

tests/unit/out/refcount/class_template_lazy_member.rs

@@ -0,0 +1,109 @@
+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 Point {
+    pub x: Value<i32>,
+}
+impl Clone for Point {
+    fn clone(&self) -> Self {
+        let mut this = Self {
+            x: Rc::new(RefCell::new((*self.x.borrow()))),
+        };
+        this
+    }
+}
+impl ByteRepr for Point {
+    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]))),
+        }
+    }
+}
+#[derive(Default)]
+pub struct Box_int_ {
+    pub val: Value<i32>,
+}
+impl Box_int_ {
+    pub fn twice(&self) -> i32 {
+        return ((*self.val.borrow()) + (*self.val.borrow()));
+    }
+}
+impl Clone for Box_int_ {
+    fn clone(&self) -> Self {
+        let mut this = Self {
+            val: Rc::new(RefCell::new((*self.val.borrow()))),
+        };
+        this
+    }
+}
+impl ByteRepr for Box_int_ {
+    fn byte_size() -> usize {
+        4
+    }
+    fn to_bytes(&self, buf: &mut [u8]) {
+        (*self.val.borrow()).to_bytes(&mut buf[0..4]);
+    }
+    fn from_bytes(buf: &[u8]) -> Self {
+        Self {
+            val: Rc::new(RefCell::new(<i32>::from_bytes(&buf[0..4]))),
+        }
+    }
+}
+#[derive(Default)]
+pub struct Box_Point_ {
+    pub val: Value<Point>,
+}
+impl Box_Point_ {
+    pub fn get(&self) -> Point {
+        return (*self.val.borrow()).clone();
+    }
+}
+impl Clone for Box_Point_ {
+    fn clone(&self) -> Self {
+        let mut this = Self {
+            val: Rc::new(RefCell::new((*self.val.borrow()).clone())),
+        };
+        this
+    }
+}
+impl ByteRepr for Box_Point_ {
+    fn byte_size() -> usize {
+        4
+    }
+    fn to_bytes(&self, buf: &mut [u8]) {
+        (*self.val.borrow()).to_bytes(&mut buf[0..4]);
+    }
+    fn from_bytes(buf: &[u8]) -> Self {
+        Self {
+            val: Rc::new(RefCell::new(<Point>::from_bytes(&buf[0..4]))),
+        }
+    }
+}
+pub fn main() {
+    std::process::exit(main_0());
+}
+fn main_0() -> i32 {
+    let i: Value<Box_int_> = Rc::new(RefCell::new(Box_int_ {
+        val: Rc::new(RefCell::new(3)),
+    }));
+    assert!((({ (*i.borrow()).twice() }) == 6));
+    let p: Value<Box_Point_> = Rc::new(RefCell::new(Box_Point_ {
+        val: Rc::new(RefCell::new(Point {
+            x: Rc::new(RefCell::new(4)),
+        })),
+    }));
+    assert!(((*({ (*p.borrow()).get() }).x.borrow()) == 4));
+    return 0;
+}

tests/unit/out/refcount/class_templates.rs

@@ -17,9 +17,6 @@ impl MyContainer_int_ {
     pub fn size(&self) -> usize {
         return (*self.vec_.borrow()).len();
     }
-    pub fn back_const(&self) -> Ptr<i32> {
-        return (self.vec_.as_pointer() as Ptr<i32>).to_last();
-    }
     pub fn back(&self) -> Ptr<i32> {
         return (self.vec_.as_pointer() as Ptr<i32>).to_last();
     }
@@ -66,9 +63,6 @@ impl MyContainer_char_ {
     pub fn size(&self) -> usize {
         return (*self.vec_.borrow()).len();
     }
-    pub fn back_const(&self) -> Ptr<u8> {
-        return (self.vec_.as_pointer() as Ptr<u8>).to_last();
-    }
     pub fn back(&self) -> Ptr<u8> {
         return (self.vec_.as_pointer() as Ptr<u8>).to_last();
     }
@@ -115,9 +109,6 @@ impl MyContainer_float_ {
     pub fn size(&self) -> usize {
         return (*self.vec_.borrow()).len();
     }
-    pub fn back_const(&self) -> Ptr<f32> {
-        return (self.vec_.as_pointer() as Ptr<f32>).to_last();
-    }
     pub fn back(&self) -> Ptr<f32> {
         return (self.vec_.as_pointer() as Ptr<f32>).to_last();
     }

tests/unit/out/unsafe/class_template_lazy_member.rs

@@ -0,0 +1,47 @@
+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 Point {
+    pub x: i32,
+}
+#[repr(C)]
+#[derive(Copy, Clone, Default)]
+pub struct Box_int_ {
+    pub val: i32,
+}
+impl Box_int_ {
+    pub unsafe fn twice(&mut self) -> i32 {
+        return ((self.val) + (self.val));
+    }
+}
+#[repr(C)]
+#[derive(Copy, Clone, Default)]
+pub struct Box_Point_ {
+    pub val: Point,
+}
+impl Box_Point_ {
+    pub unsafe fn get(&mut self) -> Point {
+        return self.val.clone();
+    }
+}
+pub fn main() {
+    unsafe {
+        std::process::exit(main_0() as i32);
+    }
+}
+unsafe fn main_0() -> i32 {
+    let mut i: Box_int_ = Box_int_ { val: 3 };
+    assert!(((unsafe { i.twice() }) == (6)));
+    let mut p: Box_Point_ = Box_Point_ {
+        val: Point { x: 4 },
+    };
+    assert!((((unsafe { p.get() }).x) == (4)));
+    return 0;
+}

tests/unit/out/unsafe/class_templates.rs

@@ -18,9 +18,6 @@ impl MyContainer_int_ {
     pub unsafe fn size(&self) -> usize {
         return self.vec_.len();
     }
-    pub unsafe fn back_const(&self) -> *const i32 {
-        return ((self.vec_).last().unwrap());
-    }
     pub unsafe fn back(&mut self) -> *mut i32 {
         return ((self.vec_).last_mut().unwrap());
     }
@@ -47,9 +44,6 @@ impl MyContainer_char_ {
     pub unsafe fn size(&self) -> usize {
         return self.vec_.len();
     }
-    pub unsafe fn back_const(&self) -> *const libc::c_char {
-        return ((self.vec_).last().unwrap());
-    }
     pub unsafe fn back(&mut self) -> *mut libc::c_char {
         return ((self.vec_).last_mut().unwrap());
     }
@@ -76,9 +70,6 @@ impl MyContainer_float_ {
     pub unsafe fn size(&self) -> usize {
         return self.vec_.len();
     }
-    pub unsafe fn back_const(&self) -> *const f32 {
-        return ((self.vec_).last().unwrap());
-    }
     pub unsafe fn back(&mut self) -> *mut f32 {
         return ((self.vec_).last_mut().unwrap());
     }