Arbitrary inner type in named impl key

[anforowicz]

PTAL?

If this PR looks okay, then the next commit/PR (anforowicz@8ab4ecc) should be able to add support for Vec<Box<T>> (because of how Box<T> is special when evaluating the orphan rule.

/cc @zetafunction who has kindly provided initial feedback/review at anforowicz#4

[anforowicz]

Rebased + minor tweak after a self-review: s/ stringify! / ::core::stringify! in generated code

[dtolnay]

Thank you — I am merging this with some changes in #1669.

[dtolnay]

The mangle module is already imported and used elsewhere in this file.

Suggested change

	let instance = crate::syntax::mangle::type_(ty)
	let instance = mangle::type_(ty)

[dtolnay]

Suggested change

	self.content.borrow_mut().write(s);
	self.content.get_mut().write(s);

[dtolnay]

This is going to give conflicting symbols when an identical Rust name exists in multiple namespaces. The original code before this PR was mangling using the C++ namespace and name.

// src/lib.rs

mod two;

#[cxx::bridge(namespace = "one")]
pub mod one {
    struct Context { x: i32 }
    impl Box<Context> {}
}

// src/two.rs

#[cxx::bridge(namespace = "two")]
pub mod two {
    struct Context { x: i8 }
    impl Box<Context> {}
}

error: symbol `cxxbridge1$box$Context$alloc` is already defined
 --> src/two.rs:6:23
  |
6 |     impl Box<Context> {}
  |                       ^^

or for a C++ type:

  = note: rust-lld: error: duplicate symbol: cxxbridge1$unique_ptr$Context$null
          >>> defined at main.rs.cc:64 (target/debug/build/example-aff44aa21d738c2a/out/cxxbridge/sources/example/src/main.rs.cc:64)
          >>>            4be10217b215df70-main.rs.o:(cxxbridge1$unique_ptr$Context$null) in archive target/debug/build/example-aff44aa21d738c2a/out/libexample-cxx.a
          >>> defined at two.rs.cc:64 (target/debug/build/example-aff44aa21d738c2a/out/cxxbridge/sources/example/src/two.rs.cc:64)
          >>>            4be10217b215df70-two.rs.o:(.text.cxxbridge1$unique_ptr$Context$null+0x0) in archive target/debug/build/example-aff44aa21d738c2a/out/libexample-cxx.a

[dtolnay]

This is not an adequate replacement for what was here before.

#pragma once
#include <rust/cxx.h>

class Thing;

inline void take_thing(rust::Box<Thing>) {}

#[cxx::bridge]
mod ffi {
    extern "Rust" {
        type Thing;
    }
    unsafe extern "C++" {
        include!("example/include/header.h");
        fn take_thing(_: Box<Thing>);
    }
}

struct Thing;

impl Drop for Thing {
    fn drop(&mut self) {
        panic!("...");
    }
}

fn main() {
    ffi::take_thing(Box::new(Thing));
}

Before:

panic in ffi function <example::ffi::Thing as Drop>::drop, aborting.

After:

panic in ffi function <example::ffiThing, aborting.

[dtolnay]

Comment does not match the behavior of NamedImplKey::new.

Suggested change

	/// Mangled form of the `outer` type.
	/// Mangled form of the `inner` type.

[dtolnay]

Suggested change

	let unique_ptr_prefix = format!("cxxbridge1$unique_ptr$std$vector${}$", key.symbol,);
	let unique_ptr_prefix = format!("cxxbridge1$unique_ptr$std$vector${}$", key.symbol);

[dtolnay]

This is not going to compile without #ty_generics.

#[cxx::bridge]
mod ffi {
    struct Borrowed<'a> {
        s: &'a str,
    }
    extern "Rust" {
        fn f(_: &CxxVector<Borrowed>);
    }
}

fn f(_: &cxx::CxxVector<ffi::Borrowed>) {}

error[E0106]: missing lifetime specifier
 --> src/main.rs:7:28
  |
7 |         fn f(_: &CxxVector<Borrowed>);
  |                            ^^^^^^^^ expected named lifetime parameter
  |
  = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
help: consider using the `'a` lifetime
  |
7 |         fn f(_: &CxxVector<Borrowed<'a>>);
  |                                    ++++

[dtolnay]

Using stringify for this is not a good approach.

#[cxx::bridge]
pub mod ffi {
    struct Context<'a> {
        session: &'a str,
    }
    struct Server<'srv> {
        ctx: SharedPtr<Context<'srv>>,
    }
    struct Client<'clt> {
        ctx: SharedPtr<Context<'clt>>,
    }
}

fn main() {
    let _ = *cxx::SharedPtr::<ffi::Context>::null();
}

Before:

called deref on a null SharedPtr<Context>

After:

called deref on a null SharedPtr<Context < 'srv >>

The spacing is wonky, and this is using a random one of the lifetime names which is going to be misleading for the user if their panic is happening in "client" code that has nothing to do with "server" code.

[dtolnay]

This explicit_generics is never the best set of lifetime names to use for an impl. In this codepath there is no explicit impl, so explicit_generics is populated with lifetime names from some random usage of the generic type in a function signature or struct. In general those lifetime names are not meaningful at all outside the context of that single function or struct. This code should not be propagating them onto an impl block (which is going to become visible in rustdoc).

For example:

struct Context<'sess> {
    session: &'sess str,
}
struct Server<'srv> {
    ctx: SharedPtr<Context<'srv>>,
}
struct Client<'clt> {
    ctx: SharedPtr<Context<'clt>>,
}

It does not make sense for the documentation of Context to be rendered as:

• impl<'sess> ExternType for Context<'sess>
• impl<'srv> SharedPtrTarget for Context<'srv>

The correct impl would be impl<'sess> SharedPtrTarget for Context<'sess> (which was generated before this PR).

[dtolnay]

These are all currently unreachable, right?

Suggested change

	Type::RustBox(ty1) => type_(&ty1.inner).map(|s| join!("box", s)),
	Type::RustVec(ty1) => type_(&ty1.inner).map(|s| join!("rust_vec", s)),
	Type::UniquePtr(ty1) => type_(&ty1.inner).map(|s| join!("unique_ptr", s)),
	Type::SharedPtr(ty1) => type_(&ty1.inner).map(|s| join!("shared_ptr", s)),
	Type::WeakPtr(ty1) => type_(&ty1.inner).map(|s| join!("weak_ptr", s)),

[anforowicz]

Yes, correct.