Repository: dtolnay/cxx (language: rust)

Issue:
Remove Str::Repr nested struct from cxx.h
This should be implemented some other way. It shouldn't be necessary to include this "private" public nested struct which is only supposed to be used by our code generator. Instead the code generator should be built using only public constructors and member functions on rust::Str.


Discussion / hints:

Fix patch:
diff --git a/gen/src/write.rs b/gen/src/write.rs
index 16bb7cdb7..10c6b4071 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -619,7 +619,6 @@ fn write_cxx_function_shim(out: &mut OutFile, efn: &ExternFn, impl_annotations:
     }
     match &efn.ret {
         Some(Type::Ref(_)) => write!(out, "&"),
-        Some(Type::Str(_)) if !indirect_return => write!(out, "::rust::Str::Repr("),
         Some(Type::SliceRefU8(_)) if !indirect_return => {
             write!(out, "::rust::Slice<uint8_t>::Repr(")
         }
@@ -659,7 +658,7 @@ fn write_cxx_function_shim(out: &mut OutFile, efn: &ExternFn, impl_annotations:
     match &efn.ret {
         Some(Type::RustBox(_)) => write!(out, ".into_raw()"),
         Some(Type::UniquePtr(_)) => write!(out, ".release()"),
-        Some(Type::Str(_)) | Some(Type::SliceRefU8(_)) if !indirect_return => write!(out, ")"),
+        Some(Type::SliceRefU8(_)) if !indirect_return => write!(out, ")"),
         _ => {}
     }
     if indirect_return {
@@ -866,7 +865,6 @@ fn write_rust_function_shim_impl(
             write!(out, ", ");
         }
         match &arg.ty {
-            Type::Str(_) => write!(out, "::rust::Str::Repr("),
             Type::SliceRefU8(_) => write!(out, "::rust::Slice<uint8_t>::Repr("),
             ty if out.types.needs_indirect_abi(ty) => write!(out, "&"),
             _ => {}
@@ -875,7 +873,7 @@ fn write_rust_function_shim_impl(
         match &arg.ty {
             Type::RustBox(_) => write!(out, ".into_raw()"),
             Type::UniquePtr(_) => write!(out, ".release()"),
-            Type::Str(_) | Type::SliceRefU8(_) => write!(out, ")"),
+            Type::SliceRefU8(_) => write!(out, ")"),
             ty if ty != RustString && out.types.needs_indirect_abi(ty) => write!(out, "$.value"),
             _ => {}
         }
@@ -939,7 +937,6 @@ fn write_indirect_return_type(out: &mut OutFile, ty: &Type) {
             write_type(out, &ty.inner);
             write!(out, " *");
         }
-        Type::Str(_) => write!(out, "::rust::Str::Repr"),
         Type::SliceRefU8(_) => write!(out, "::rust::Slice<uint8_t>::Repr"),
         _ => write_type(out, ty),
     }
@@ -967,7 +964,6 @@ fn write_extern_return_type_space(out: &mut OutFile, ty: &Option<Type>) {
             write_type(out, &ty.inner);
             write!(out, " *");
         }
-        Some(Type::Str(_)) => write!(out, "::rust::Str::Repr "),
         Some(Type::SliceRefU8(_)) => write!(out, "::rust::Slice<uint8_t>::Repr "),
         Some(ty) if out.types.needs_indirect_abi(ty) => write!(out, "void "),
         _ => write_return_type(out, ty),
@@ -980,7 +976,6 @@ fn write_extern_arg(out: &mut OutFile, arg: &Var) {
             write_type_space(out, &ty.inner);
             write!(out, "*");
         }
-        Type::Str(_) => write!(out, "::rust::Str::Repr "),
         Type::SliceRefU8(_) => write!(out, "::rust::Slice<uint8_t>::Repr "),
         _ => write_type_space(out, &arg.ty),
     }
diff --git a/include/cxx.h b/include/cxx.h
index c545e500a..04a63d6fb 100644
--- a/include/cxx.h
+++ b/include/cxx.h
@@ -79,19 +79,11 @@ class Str final {
   Str(const Str &) noexcept = default;
   ~Str() noexcept = default;
 
-  // Repr is PRIVATE; must not be used other than by our generated code.
-  //
+private:
   // Not necessarily ABI compatible with &str. Codegen will translate to
   // cxx::rust_str::RustStr which matches this layout.
-  struct Repr final {
-    const char *ptr;
-    size_t len;
-  };
-  Str(Repr) noexcept;
-  explicit operator Repr() const noexcept;
-
-private:
-  Repr repr;
+  const char *ptr;
+  size_t len;
 };
 #endif // CXXBRIDGE05_RUST_STR
 
diff --git a/src/cxx.cc b/src/cxx.cc
index 722a03409..7e9f57e4d 100644
--- a/src/cxx.cc
+++ b/src/cxx.cc
@@ -115,33 +115,33 @@ std::ostream &operator<<(std::ostream &os, const String &s) {
   return os;
 }
 
-Str::Str() noexcept : repr(Repr{reinterpret_cast<const char *>(1), 0}) {}
+Str::Str() noexcept : ptr(reinterpret_cast<const char *>(1)), len(0) {}
 
-static void initStr(Str::Repr repr) {
-  if (!cxxbridge05$str$valid(repr.ptr, repr.len)) {
+static void initStr(const char *ptr, size_t len) {
+  if (!cxxbridge05$str$valid(ptr, len)) {
     panic<std::invalid_argument>("data for rust::Str is not utf-8");
   }
 }
 
-Str::Str(const std::string &s) : repr(Repr{s.data(), s.length()}) {
-  initStr(this->repr);
+Str::Str(const std::string &s) : ptr(s.data()), len(s.length()) {
+  initStr(this->ptr, this->len);
 }
 
-Str::Str(const char *s) : repr(Repr{s, std::strlen(s)}) {
+Str::Str(const char *s) : ptr(s), len(std::strlen(s)) {
   assert(s != nullptr);
-  initStr(this->repr);
+  initStr(this->ptr, this->len);
 }
 
 Str::Str(const char *s, size_t len)
-    : repr(
-          Repr{s == nullptr && len == 0 ? reinterpret_cast<const char *>(1) : s,
-               len}) {
+    : ptr(s == nullptr && len == 0 ? reinterpret_cast<const char *>(1) : s),
+      len(len) {
   assert(s != nullptr || len == 0);
-  initStr(this->repr);
+  initStr(this->ptr, this->len);
 }
 
 Str &Str::operator=(Str other) noexcept {
-  this->repr = other.repr;
+  this->ptr = other.ptr;
+  this->len = other.len;
   return *this;
 }
 
@@ -149,15 +149,11 @@ Str::operator std::string() const {
   return std::string(this->data(), this->size());
 }
 
-const char *Str::data() const noexcept { return this->repr.ptr; }
-
-size_t Str::size() const noexcept { return this->repr.len; }
-
-size_t Str::length() const noexcept { return this->repr.len; }
+const char *Str::data() const noexcept { return this->ptr; }
 
-Str::Str(Repr repr_) noexcept : repr(repr_) {}
+size_t Str::size() const noexcept { return this->len; }
 
-Str::operator Repr() const noexcept { return this->repr; }
+size_t Str::length() const noexcept { return this->len; }
 
 std::ostream &operator<<(std::ostream &os, const Str &s) {
   os.write(s.data(), s.size());


Test patch:
diff --git a/tests/cxx_gen.rs b/tests/cxx_gen.rs
index 527d7090c..3d3bf0f31 100644
--- a/tests/cxx_gen.rs
+++ b/tests/cxx_gen.rs
@@ -18,7 +18,7 @@ fn test_extern_c_function() {
     let output = str::from_utf8(&generated.implementation).unwrap();
     // To avoid continual breakage we won't test every byte.
     // Let's look for the major features.
-    assert!(output.contains("void cxxbridge05$do_cpp_thing(::rust::Str::Repr foo)"));
+    assert!(output.contains("void cxxbridge05$do_cpp_thing(::rust::Str foo)"));
 }
 
 #[test]
@@ -28,5 +28,5 @@ fn test_impl_annotation() {
     let source = BRIDGE0.parse().unwrap();
     let generated = generate_header_and_cc(source, &opt).unwrap();
     let output = str::from_utf8(&generated.implementation).unwrap();
-    assert!(output.contains("ANNOTATION void cxxbridge05$do_cpp_thing(::rust::Str::Repr foo)"));
+    assert!(output.contains("ANNOTATION void cxxbridge05$do_cpp_thing(::rust::Str foo)"));
 }