cxx icon indicating copy to clipboard operation
cxx copied to clipboard

Support for std::string_view (C++17)

Open Imxset21 opened this issue 4 years ago • 8 comments

I'm wondering if it would be possible to support std::string_view in cxx.

I imagine that there might be some lifetime concerns similar to the ones in rust::Str, so I'm not sure what the API should look like.

Imxset21 avatar Mar 08 '21 07:03 Imxset21

There's been some work towards this in #80 and #410.

adetaylor avatar Mar 08 '21 18:03 adetaylor

I am open to this but for now it's easy enough to define downstream.

#80 and #410 are for something different, those appear to be for C++ implicit conversions and conversion operators between string_view and rust::Str / rust::String, not for using string_view in FFI signatures.

use cxx::{type_id, ExternType};
use std::marker::PhantomData;
use std::mem::MaybeUninit;
use std::ops::Deref;
use std::os::raw::{c_char, c_void};

#[derive(Copy, Clone)]
#[repr(C)]
pub struct StringView<'a> {
    repr: MaybeUninit<[*const c_void; 2]>,
    borrow: PhantomData<&'a [c_char]>,
}

unsafe impl<'a> ExternType for StringView<'a> {
    type Id = type_id!("std::string_view");
    type Kind = cxx::kind::Trivial;
}

#[cxx::bridge]
mod ffi {
    unsafe extern "C++" {
        include!("example/include/string_view.h");

        #[namespace = "std"]
        #[cxx_name = "string_view"]
        type StringView<'a> = crate::StringView<'a>;

        fn string_view_from_str(s: &str) -> StringView;
        fn string_view_as_bytes(s: StringView) -> &[c_char];
    }
}

impl<'a> StringView<'a> {
    pub fn new(s: &'a str) -> Self {
        ffi::string_view_from_str(s)
    }

    pub fn as_bytes(self) -> &'a [c_char] {
        ffi::string_view_as_bytes(self)
    }
}

impl<'a> Deref for StringView<'a> {
    type Target = [c_char];
    fn deref(&self) -> &Self::Target {
        self.as_bytes()
    }
}

fn main() {
    let string_view = StringView::new("... :)");
    println!("len={} {:?}", string_view.len(), string_view.as_bytes());
}
// include/string_view.h

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

static_assert(sizeof(std::string_view) == 2 * sizeof(void *), "");
static_assert(alignof(std::string_view) == alignof(void *), "");

inline std::string_view string_view_from_str(rust::Str s) {
  return {s.data(), s.size()};
}

inline rust::Slice<const char> string_view_as_bytes(std::string_view s) {
  return {s.data(), s.size()};
}

dtolnay avatar Apr 23 '21 01:04 dtolnay

I've been working on this in https://github.com/SilensAngelusNex/cxx/commit/996efd965b7116ddf7f0dd4e5eb9ad51c393bc6b, but I've run in to a couple issues.

  1. I'm using cfg attributes to make sure CxxStringView is only present when the C++ standard supports string_view, but the cxx::bridge macro doesn't support cfg attributes. I fixed this by making them pass-through attributes (syntax/attrs.rs:142); is that the right solution?
  2. When trying to write extern functions taking CxxStringView as an argument, I'm getting errors saying that CxxStringView is an unsupported type. (Run cargo test --all --features c++17 on my commit to reproduce.) I'm guessing there's an allowlist somewhere I need to add it to, but I don't see one. How do I mark it as supported?

SilensAngelusNex avatar Oct 15 '21 22:10 SilensAngelusNex

@SilensAngelusNex I'm not an expert on adding new types, but as far as I can tell for #2, in order to make it typecheck, it has to pass check_type_ident in check.rs:

fn check_type_ident(cx: &mut Check, name: &NamedType) {
    let ident = &name.rust;
    if Atom::from(ident).is_none()
        && !cx.types.structs.contains_key(ident)
        && !cx.types.enums.contains_key(ident)
        && !cx.types.cxx.contains(ident)
        && !cx.types.rust.contains(ident)
    {
        let msg = format!("unsupported type: {}", ident);
        cx.error(ident, &msg);
    }
}

If I had to guess, I think you need to ensure that your type identifier is in cx.types.cxx, which is an unordered set in the Types struct initialized here. Not sure how to do that part, though.

Imxset21 avatar Oct 16 '21 00:10 Imxset21

I see that this issue has been open a for a while now. Without it, how do users adopt cxx and properly handle std::string_view?

anonrig avatar May 05 '23 15:05 anonrig

I see that this issue has been open a for a while now. Without it, how do users adopt cxx and properly handle std::string_view?

You could declare an opaque type for std::string_view in Rust similar to the following:

#[cxx::bridge]
mod ffi {
    unsafe extern "C++" {
        #[cxx_name = "std::string_view"]
        type StringView<'a>;

        fn construct<'a>(str: &'a str) -> UniquePtr<StringView<'a>>; 
    }
}

Then on the C++ side define construct method appropriately using the basic_string_view( const CharT* s, size_type count ) constructor and the data and size methods for rust::Str.

Of course you'd still have to declare bindings for all the other string_view methods you intend to use but otherwise it should be relatively straightforward.

silvanshade avatar May 08 '23 17:05 silvanshade

That's right; this is described in https://github.com/dtolnay/cxx/issues/734#issuecomment-825319173. Someone should feel free to publish a library containing the StringView binding.

dtolnay avatar May 08 '23 18:05 dtolnay

@dtolnay so what you're saying is that this should rather be implemented in an external library instead of implementing it directly in cxx? Or did I understand your comment incorrectly?

EDIT: Ah sorry, I overlooked the first sentence in the first paragraph comment you linked. Anyway, if you want to upstream this implementation, I'd be happy to help.

tom-anders avatar May 22 '23 20:05 tom-anders