Skip to content

Conversation

@anforowicz
Copy link
Contributor

PTAL?

This PR fixes #1222. I've split this into two separate commits to get some minor refactoring out of the way in the 1st comment (see the commit description for more details).

This commit replaces `fn local_type(ty: &Type) -> &NamedType` with
two more granular functions:

* `fn get_impl_generics(ty: &Type, types: &Types) -> &Lifetimes`
* `fn format_for_prevent_unwind_label(ty: &Type) -> TokenStream`

This refactoring is desirable, because it makes it easier to expand
these functions to support more than just `Type::Ident` types.  For
example `Type::RustVec(ty1)` may not have a `NamedType` of its own, but
it can still be translated into `&Lifetimes` and/or `TokenStream`.
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


match ty {
Type::RustBox(ty)
| Type::RustVec(ty)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit impls also need to allow this.

error: unsupported Self type of explicit impl
 --> src/lib.rs:8:5
  |
8 |     impl Vec<Box<Thing>> {}
  |     ^^^^^^^^^^^^^^^^^^^^^^^

ASSERT(r_return_enum(2021) == Enum::CVal);
ASSERT(Shared::r_static_method_on_shared() == 2023);
ASSERT(R::r_static_method() == 2024);
ASSERT(r_return_rust_vec_box()[0]->get() == 2020);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link fails with undefined symbols if the element type isn't local.

--- a/tests/ffi/lib.rs
+++ b/tests/ffi/lib.rs
@@ -309,3 +309,3 @@ pub mod ffi {
         #[allow(clippy::vec_box)]
-        fn r_return_rust_vec_box() -> Vec<Box<R>>;
+        fn r_return_rust_vec_box() -> Vec<Box<OpaqueRust>>;
         fn r_return_ref_rust_vec(shared: &Shared) -> &Vec<u8>;
@@ -605,4 +605,4 @@ fn r_return_rust_vec_extern_struct() -> Vec<ffi::Job> {
 #[allow(clippy::vec_box)]
-fn r_return_rust_vec_box() -> Vec<Box<R>> {
-    vec![Box::new(R(2020))]
+fn r_return_rust_vec_box() -> Vec<Box<ffi::OpaqueRust>> {
+    vec![Box::new(module::OpaqueRust(2020))]
 }

--- a/tests/ffi/tests.cc
+++ b/tests/ffi/tests.cc
@@ -814,3 +814,3 @@ extern "C" const char *cxx_run_test() noexcept {
   ASSERT(R::r_static_method() == 2024);
-  ASSERT(r_return_rust_vec_box()[0]->get() == 2020);
+  (void)r_return_rust_vec_box()[0];
error: linking with `cc` failed: exit status: 1
  |
  = note:  "cc" "-m64" "/tmp/rustcg2TIqF/symbols.o" "<66 object files omitted>" "-Wl,--as-needed" "-Wl,-Bstatic" "<sysroot>/lib/rustlib/x86_64-unknown-linux-gnu/lib/{libtest-*,libgetopts-*,librustc_std_workspace_std-*}.rlib" "target/debug/deps/{libcxx_test_suite-c4d078ad888d39fd,libserde-b6e809e7a0962b36,libserde_core-c5bad1a02a584930,libcxx-d4a8956b010c4c1e,libfoldhash-9ce79a62902e55c0,liblink_cplusplus-e0bb3070bed730d2}.rlib" "<sysroot>/lib/rustlib/x86_64-unknown-linux-gnu/lib/{libstd-*,libpanic_unwind-*,libobject-*,libmemchr-*,libaddr2line-*,libgimli-*,libcfg_if-*,librustc_demangle-*,libstd_detect-*,libhashbrown-*,librustc_std_workspace_alloc-*,libminiz_oxide-*,libadler2-*,libunwind-*,liblibc-*,librustc_std_workspace_core-*,liballoc-*,libcore-*,libcompiler_builtins-*}.rlib" "-Wl,-Bdynamic" "-lstdc++" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-L" "/tmp/rustcg2TIqF/raw-dylibs" "-B<sysroot>/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld" "-fuse-ld=lld" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "target/debug/build/cxx-659a5d2b44721952/out" "-L" "target/debug/build/link-cplusplus-df767bd78ea2acc8/out" "-L" "target/debug/build/cxx-test-suite-a29a153f94c9babe/out" "-L" "<sysroot>/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "target/debug/deps/test-ea50607a5284f55c" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs"
  = note: some arguments are omitted. use `--verbose` to show all linker arguments
  = note: rust-lld: error: undefined symbol: rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::Vec()
          >>> referenced by cxx.h:882 (target/debug/build/cxx-test-suite-a29a153f94c9babe/out/cxxbridge/include/rust/cxx.h:882)
          >>>               5d751b5c4ba69b04-lib.rs.o:(rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::Vec(rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>&&)) in archive target/debug/deps/libcxx_test_suite-c4d078ad888d39fd.rlib
          >>> did you mean: rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::~Vec()
          >>> defined in: target/debug/deps/libcxx_test_suite-c4d078ad888d39fd.rlib(a1edd97dd51cd48d-tests.o)
          
          rust-lld: error: undefined symbol: rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::drop()
          >>> referenced by cxx.h:887 (target/debug/build/cxx-test-suite-a29a153f94c9babe/out/cxxbridge/include/rust/cxx.h:887)
          >>>               a1edd97dd51cd48d-tests.o:(rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::~Vec()) in archive target/debug/deps/libcxx_test_suite-c4d078ad888d39fd.rlib
          
          rust-lld: error: undefined symbol: rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::size() const
          >>> referenced by cxx.h:946 (target/debug/build/cxx-test-suite-a29a153f94c9babe/out/cxxbridge/include/rust/cxx.h:946)
          >>>               a1edd97dd51cd48d-tests.o:(rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::operator[](unsigned long)) in archive target/debug/deps/libcxx_test_suite-c4d078ad888d39fd.rlib
          
          rust-lld: error: undefined symbol: rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::data() const
          >>> referenced by cxx.h:914 (target/debug/build/cxx-test-suite-a29a153f94c9babe/out/cxxbridge/include/rust/cxx.h:914)
          >>>               a1edd97dd51cd48d-tests.o:(rust::cxxbridge1::Vec<rust::cxxbridge1::Box<tests::OpaqueRust>>::data()) in archive target/debug/deps/libcxx_test_suite-c4d078ad888d39fd.rlib
          collect2: error: ld returned 1 exit status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unsupported element type of Vec

2 participants