From 9059daecf0de8b1a46a11aa4c28248d6dd2eb055 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 11:25:27 +0100 Subject: [PATCH 01/37] first draft --- demo/build.rs | 2 +- demo/include/blobstore.h | 8 + demo/src/blobstore.cc | 17 ++ demo/src/main.rs | 25 +++ gen/src/cfg.rs | 6 +- gen/src/mod.rs | 1 + gen/src/namespace.rs | 3 +- gen/src/write.rs | 66 ++++-- include/cxx.h | 458 +++++++++++++++++++++++++++++++++++++++ macro/src/derive.rs | 32 +++ macro/src/expand.rs | 63 +++++- syntax/attrs.rs | 2 +- syntax/cfg.rs | 6 + syntax/check.rs | 53 ++++- syntax/doc.rs | 1 + syntax/ident.rs | 3 +- syntax/improper.rs | 6 + syntax/mod.rs | 9 +- syntax/names.rs | 2 +- syntax/parse.rs | 182 ++++++++++++++-- syntax/pod.rs | 12 +- syntax/tokens.rs | 14 +- syntax/types.rs | 63 ++++-- 23 files changed, 964 insertions(+), 70 deletions(-) diff --git a/demo/build.rs b/demo/build.rs index c1b55cc2b..a1fccb722 100644 --- a/demo/build.rs +++ b/demo/build.rs @@ -1,7 +1,7 @@ fn main() { cxx_build::bridge("src/main.rs") .file("src/blobstore.cc") - .flag_if_supported("-std=c++14") + .flag_if_supported("-std=c++17") .compile("cxxbridge-demo"); println!("cargo:rerun-if-changed=src/main.rs"); diff --git a/demo/include/blobstore.h b/demo/include/blobstore.h index d89583aa9..05862e626 100644 --- a/demo/include/blobstore.h +++ b/demo/include/blobstore.h @@ -7,6 +7,8 @@ namespace blobstore { struct MultiBuf; struct BlobMetadata; +struct BlobWrapper; +struct Foo; class BlobstoreClient { public: @@ -22,5 +24,11 @@ class BlobstoreClient { std::unique_ptr new_blobstore_client(); +Foo make_foo(); +void take_foo(const Foo&); + +BlobWrapper make_metadata(); +void take_metadata(const BlobWrapper&); + } // namespace blobstore } // namespace org diff --git a/demo/src/blobstore.cc b/demo/src/blobstore.cc index 7cf40dfe3..f5700d23a 100644 --- a/demo/src/blobstore.cc +++ b/demo/src/blobstore.cc @@ -4,6 +4,7 @@ #include #include #include +#include #include namespace org { @@ -67,5 +68,21 @@ std::unique_ptr new_blobstore_client() { return std::make_unique(); } +Foo make_foo() { + return Foo{false}; +} + +void take_foo(const Foo& foo) { + std::cout << "The index of foo is " << foo.index() << std::endl; +} + +void take_metadata(const BlobWrapper&) { + +} + +BlobWrapper make_metadata() { + return {}; +} + } // namespace blobstore } // namespace org diff --git a/demo/src/main.rs b/demo/src/main.rs index 458f1f211..28733ce0e 100644 --- a/demo/src/main.rs +++ b/demo/src/main.rs @@ -6,6 +6,19 @@ mod ffi { tags: Vec, } + struct BlobWrapper { + pub inner: BlobMetadata + } + + /// A classic. + // #[derive(Debug)] + enum Foo { + /// This is my doc + Bar(i32), + Baz(bool), + Bam(BlobMetadata), + } + // Rust types and signatures exposed to C++. extern "Rust" { type MultiBuf; @@ -23,6 +36,9 @@ mod ffi { fn put(&self, parts: &mut MultiBuf) -> u64; fn tag(&self, blobid: u64, tag: &str); fn metadata(&self, blobid: u64) -> BlobMetadata; + + fn make_foo() -> Foo; + fn take_foo(foo: &Foo); } } @@ -42,6 +58,15 @@ pub fn next_chunk(buf: &mut MultiBuf) -> &[u8] { } fn main() { + let f = ffi::Foo::Bar(1); + ffi::take_foo(&f); + let f = ffi::make_foo(); + match f { + ffi::Foo::Bar(val) => println!("The value is {val}"), + ffi::Foo::Baz(val) => println!("The value is {val}"), + _ => {}, + } + let client = ffi::new_blobstore_client(); // Upload a blob. diff --git a/gen/src/cfg.rs b/gen/src/cfg.rs index adab6e5c2..a3d64c3ea 100644 --- a/gen/src/cfg.rs +++ b/gen/src/cfg.rs @@ -29,9 +29,10 @@ pub(super) fn strip( Api::Struct(strct) => strct .fields .retain(|field| eval(cx, cfg_errors, cfg_evaluator, &field.cfg)), - Api::Enum(enm) => enm + Api::Enum(enm, _) => enm .variants .retain(|variant| eval(cx, cfg_errors, cfg_evaluator, &variant.cfg)), + // TODO What is this doing?? _ => {} } } @@ -113,7 +114,8 @@ impl Api { match self { Api::Include(include) => &include.cfg, Api::Struct(strct) => &strct.cfg, - Api::Enum(enm) => &enm.cfg, + Api::Enum(enm, _) => &enm.cfg, + Api::EnumUnnamed(enm) => &enm.cfg, Api::CxxType(ety) | Api::RustType(ety) => &ety.cfg, Api::CxxFunction(efn) | Api::RustFunction(efn) => &efn.cfg, Api::TypeAlias(alias) => &alias.cfg, diff --git a/gen/src/mod.rs b/gen/src/mod.rs index f24846a7e..1f6386fa5 100644 --- a/gen/src/mod.rs +++ b/gen/src/mod.rs @@ -159,6 +159,7 @@ pub(super) fn generate(syntax: File, opt: &Opt) -> Result { )); } } + // TODO here is the c++ side - maybe I have to add here something. cfg::strip(errors, cfg_errors, opt.cfg_evaluator.as_ref(), apis); errors.propagate()?; diff --git a/gen/src/namespace.rs b/gen/src/namespace.rs index 424e9d8e2..f61563ee2 100644 --- a/gen/src/namespace.rs +++ b/gen/src/namespace.rs @@ -6,7 +6,8 @@ impl Api { match self { Api::CxxFunction(efn) | Api::RustFunction(efn) => &efn.name.namespace, Api::CxxType(ety) | Api::RustType(ety) => &ety.name.namespace, - Api::Enum(enm) => &enm.name.namespace, + Api::Enum(enm, _) => &enm.name.namespace, + Api::EnumUnnamed(enm) => &enm.name.namespace, Api::Struct(strct) => &strct.name.namespace, Api::Impl(_) | Api::Include(_) | Api::TypeAlias(_) => Default::default(), } diff --git a/gen/src/write.rs b/gen/src/write.rs index 8eef0a76b..e741ac6d8 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -9,8 +9,8 @@ use crate::syntax::set::UnorderedSet; use crate::syntax::symbol::{self, Symbol}; use crate::syntax::trivial::{self, TrivialReason}; use crate::syntax::{ - derive, mangle, Api, Doc, Enum, EnumRepr, ExternFn, ExternType, Pair, Signature, Struct, Trait, - Type, TypeAlias, Types, Var, + derive, mangle, Api, CEnumOpts, Doc, Enum, EnumRepr, ExternFn, ExternType, Pair, Signature, + Struct, Trait, Type, TypeAlias, Types, Var, }; use proc_macro2::Ident; @@ -35,7 +35,8 @@ pub(super) fn gen(apis: &[Api], types: &Types, opt: &Opt, header: bool) -> Vec true, - Api::Enum(enm) => !out.types.cxx.contains(&enm.name.rust), + Api::Enum(enm, _) => !out.types.cxx.contains(&enm.name.rust), + Api::EnumUnnamed(_) => true, _ => false, }; @@ -46,12 +47,12 @@ fn write_forward_declarations(out: &mut OutFile, apis: &[Api]) { fn write(out: &mut OutFile, ns_entries: &NamespaceEntries, indent: usize) { let apis = ns_entries.direct_content(); - for api in apis { write!(out, "{:1$}", "", indent); match api { Api::Struct(strct) => write_struct_decl(out, &strct.name), - Api::Enum(enm) => write_enum_decl(out, enm), + Api::Enum(enm, opts) => write_enum_decl(out, enm, opts), + Api::EnumUnnamed(enm) => write_struct_decl(out, &enm.name), Api::CxxType(ety) => write_struct_using(out, &ety.name), Api::RustType(ety) => write_struct_decl(out, &ety.name), _ => unreachable!(), @@ -99,14 +100,18 @@ fn write_data_structures<'a>(out: &mut OutFile<'a>, apis: &'a [Api]) { } } } - Api::Enum(enm) => { + Api::Enum(enm, opts) => { out.next_section(); if !out.types.cxx.contains(&enm.name.rust) { - write_enum(out, enm); - } else if !enm.variants_from_header { - check_enum(out, enm); + write_enum(out, enm, opts); + } else if !opts.variants_from_header { + check_enum(out, enm, opts); } } + Api::EnumUnnamed(enm) => { + out.next_section(); + write_enum_using(out, &enm); + } Api::RustType(ety) => { out.next_section(); let methods = methods_for_type @@ -328,8 +333,8 @@ fn write_struct_decl(out: &mut OutFile, ident: &Pair) { writeln!(out, "struct {};", ident.cxx); } -fn write_enum_decl(out: &mut OutFile, enm: &Enum) { - let repr = match &enm.repr { +fn write_enum_decl(out: &mut OutFile, enm: &Enum, opts: &CEnumOpts) { + let repr = match &opts.repr { #[cfg(feature = "experimental-enum-variants-from-header")] EnumRepr::Foreign { .. } => return, EnumRepr::Native { atom, .. } => *atom, @@ -339,6 +344,37 @@ fn write_enum_decl(out: &mut OutFile, enm: &Enum) { writeln!(out, ";"); } +fn write_enum_using(out: &mut OutFile, enm: &Enum) { + write!( + out, + "struct {} final : public ::rust::variant<", + enm.name.cxx + ); + + let mut iter = enm.variants.iter().peekable(); + while let Some(value) = iter.next() { + write_type(out, value.ty.as_ref().unwrap()); + if iter.peek().is_some() { + write!(out, ", "); + } + } + + writeln!(out, "> {{"); + write!(out, "using base = ::rust::variant<"); + let mut iter = enm.variants.iter().peekable(); + while let Some(value) = iter.next() { + write_type(out, value.ty.as_ref().unwrap()); + if iter.peek().is_some() { + write!(out, ", "); + } + } + + writeln!(out, ">;"); + writeln!(out, " using base::base;"); + writeln!(out, " using base::operator=;"); + writeln!(out, "}};"); +} + fn write_struct_using(out: &mut OutFile, ident: &Pair) { writeln!(out, "using {} = {};", ident.cxx, ident.to_fully_qualified()); } @@ -388,8 +424,8 @@ fn write_opaque_type<'a>(out: &mut OutFile<'a>, ety: &'a ExternType, methods: &[ writeln!(out, "#endif // {}", guard); } -fn write_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum) { - let repr = match &enm.repr { +fn write_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum, opts: &'a CEnumOpts) { + let repr = match &opts.repr { #[cfg(feature = "experimental-enum-variants-from-header")] EnumRepr::Foreign { .. } => return, EnumRepr::Native { atom, .. } => *atom, @@ -410,8 +446,8 @@ fn write_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum) { writeln!(out, "#endif // {}", guard); } -fn check_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum) { - let repr = match &enm.repr { +fn check_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum, opts: & 'a CEnumOpts) { + let repr = match &opts.repr { #[cfg(feature = "experimental-enum-variants-from-header")] EnumRepr::Foreign { .. } => return, EnumRepr::Native { atom, .. } => *atom, diff --git a/include/cxx.h b/include/cxx.h index 002282551..baddcfdfd 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -13,6 +14,9 @@ #include #include #include +#if __cplusplus >= 201703L +#include +#endif #include #if defined(_WIN32) #include @@ -364,6 +368,460 @@ class Vec final { }; #endif // CXXBRIDGE1_RUST_VEC +#ifndef CXXBRIDGE1_RUST_VARIANT +#define CXXBRIDGE1_RUST_VARIANT + +#if __cplusplus >= 201703L + +template +struct type_from_index_impl { + using type = Type; +}; + +template +struct type_from_index; + +template +struct type_from_index {}; + +template +struct type_from_index + : std::conditional_t, + type_from_index> {}; + +template +constexpr size_t compile_time_count() noexcept { + return 0 + (static_cast(Values) + ...); +} + +template +struct count + : std::integral_constant()> {}; + +template +struct exactly_once : std::conditional_t::value == 1, + std::true_type, std::false_type> {}; + +template +constexpr std::size_t compile_time_index() noexcept { + static_assert(sizeof...(Values) != 0, "Provide at least one value"); + static_assert(exactly_once::value, "Ambiguous index"); + std::size_t ii = 0; + // Note: This might be a too simplistic implementation. The order of `ii++` + // should be guaranteed by the standard but I haven't found where. + return ((ii++ * static_cast(Values)) + ...); +} + +template +struct index_from_type + : std::integral_constant< + std::size_t, + compile_time_index, Ts>...>()> {}; + +template +struct visitor_type { + /// @brief The visit method which will pick the right type depending on the + /// `index` value. + template + constexpr static auto visit(Visitor &&visitor, std::size_t index, + std::byte *data) + -> decltype(visitor(*reinterpret_cast(data))) { + if (index == 0) { + return visitor(*reinterpret_cast(data)); + } + if constexpr (sizeof...(Remainder) != 0) { + return visitor_type::visit(std::forward(visitor), + --index, data); + } + throw std::out_of_range("invalid"); + } + + template + constexpr static auto visit(Visitor &&visitor, std::size_t index, + const std::byte *data) + -> decltype(visitor(*reinterpret_cast(data))) { + if (index == 0) { + return visitor(*reinterpret_cast(data)); + } + if constexpr (sizeof...(Remainder) != 0) { + return visitor_type::visit(std::forward(visitor), + --index, data); + } + throw std::out_of_range("invalid"); + } +}; + +template +struct attempt; + +template +constexpr decltype(auto) get(attempt &); + +template +constexpr decltype(auto) get(const attempt &); + +template +constexpr auto visit(Visitor &&visitor, attempt &); + +template +constexpr auto visit(Visitor &&visitor, const attempt &); + +template +struct attempt { + static_assert(sizeof...(Ts) > 0, + "attempt must hold at least one alternative"); + + /// @brief Delete the default constructor since we cannot be in an + /// uninitialized state (if the first alternative throws). Corresponds to the + /// (1) constructor in std::variant. + attempt() = delete; + + constexpr static bool all_copy_constructible_v = + std::conjunction_v...>; + + /// @brief Copy constructor. Participates only in the resolution if all types + /// are copy constructable. Corresponds to (2) constructor of std::variant. + attempt(const attempt &other) { + static_assert( + all_copy_constructible_v, + "Copy constructor requires that all types are copy constructable"); + + m_Type = other.m_Type; + visit( + [this](const auto &value) { + using type = std::decay_t; + new (static_cast(t_buff)) type(value); + }, + other); + }; + + /// @brief Delete the move constructor since if we move this container it's + /// unclear in which state it is. Corresponds to (3) constructor of + /// std::variant. + attempt(attempt &&other) = delete; + + template + constexpr static bool is_unique_v = + exactly_once>...>::value; + + template + constexpr static std::size_t index_from_type_v = + index_from_type::value; + + /// @brief Converting constructor. Corresponds to (4) constructor of + /// std::variant. + template , + typename = std::enable_if_t && + std::is_constructible_v>> + attempt(T &&other) noexcept(std::is_nothrow_constructible_v) { + m_Type = index_from_type_v; + new (static_cast(t_buff)) D(std::forward(other)); + } + + /// @brief Participates in the resolution only if we can construct T from Args + /// and if T is unique in Ts. Corresponds to (5) constructor of std::variant. + template >, + typename = std::enable_if_t>> + explicit attempt(std::in_place_type_t type, Args &&...args) noexcept( + std::is_nothrow_constructible_v) + : attempt{std::in_place_index>, + std::forward(args)...} {} + + template + using type_from_index_t = typename type_from_index::type; + + /// @brief Participates in the resolution only if the index is within range + /// and if the type can be constructor from Args. Corresponds to (7) of + /// std::variant. + template , + typename = std::enable_if_t>> + explicit attempt(std::in_place_index_t index, Args &&...args) noexcept( + std::is_nothrow_constructible_v) { + m_Type = I; + new (static_cast(t_buff)) T(std::forward(args)...); + } + + template + constexpr static bool all_same_v = + std::conjunction_v...>; + + /// @brief Converts the std::variant to our variant. Participates only in + /// the resolution if all types in Ts are copy constructable. + template && all_copy_constructible_v>> + attempt(const std::variant &other) { + m_Type = other.index(); + std::visit( + [this](const auto &value) { + using type = std::decay_t; + new (static_cast(t_buff)) type(value); + }, + other); + } + + constexpr static bool all_move_constructible_v = + std::conjunction_v...>; + + /// @brief Converts the std::variant to our variant. Participates only in + /// the resolution if all types in Ts are move constructable. + template && all_move_constructible_v>> + attempt(std::variant &&other) { + m_Type = other.index(); + std::visit( + [this](auto &&value) { + using type = std::decay_t; + new (static_cast(t_buff)) type(std::move(value)); + }, + other); + } + + ~attempt() { destroy(); } + + /// @brief Copy assignment. Staticly fails if not every type in Ts is copy + /// constructable. Corresponds to (1) assignment of std::variant. + attempt &operator=(const attempt &other) { + static_assert( + all_copy_constructible_v, + "Copy assignment requires that all types are copy constructable"); + + visit([this](const auto &value) { *this = value; }, other); + + return *this; + }; + + /// @brief Deleted move assignment. Same as for the move constructor. + /// Would correspond to (2) assignment of std::variant. + attempt &operator=(attempt &&other) = delete; + + /// @brief Converting assignment. Corresponds to (3) assignment of + /// std::variant. + template && std::is_constructible_v>> + attempt &operator=(T &&other) { + constexpr auto index = index_from_type_v; + + if (m_Type == index) { + if constexpr (std::is_nothrow_assignable_v) { + get(*this) = std::forward(other); + return *this; + } + } + this->emplace>(std::forward(other)); + return *this; + } + + /// @brief Converting assignment from std::variant. Participates only in the + /// resolution if all types in Ts are copy constructable. + template && all_copy_constructible_v>> + attempt &operator=(const std::variant &other) { + // TODO this is not really clean since we fail if std::variant has + // duplicated types. + std::visit( + [this](const auto &value) { + using type = decltype(value); + emplace>(value); + }, + other); + return *this; + } + + /// @brief Converting assignment from std::variant. Participates only in the + /// resolution if all types in Ts are move constructable. + template && all_move_constructible_v>> + attempt &operator=(std::variant &&other) { + // TODO this is not really clean since we fail if std::variant has + // duplicated types. + std::visit( + [this](auto &&value) { + using type = decltype(value); + emplace>(std::move(value)); + }, + other); + return *this; + } + + /// @brief Emplace function. Participates in the resolution only if T is + /// unique in Ts and if T can be constructed from Args. Offers strong + /// exception guarantee. Corresponds to the (1) emplace function of + /// std::variant. + template >, + typename = std::enable_if_t>> + T &emplace(Args &&...args) { + constexpr std::size_t index = index_from_type_v; + return this->emplace(std::forward(args)...); + } + + /// @brief Emplace function. Participates in the resolution only if T can be + /// constructed from Args. Offers strong exception guarantee. Corresponds to + /// the (2) emplace function of std::variant. + template , + typename = std::enable_if_t>> + T &emplace(Args &&...args) { + if constexpr (std::is_nothrow_constructible_v) { + destroy(); + new (static_cast(t_buff)) T(std::forward(args)...); + } else if constexpr (std::is_nothrow_move_constructible_v) { + // This operation may throw, but we know that the move does not. + const T tmp{std::forward(args)...}; + + // The operations below are save. + destroy(); + new (static_cast(t_buff)) T(std::move(tmp)); + } else { + // Backup the old data. + alignas(Ts...) std::byte old_buff[std::max({sizeof(Ts)...})]; + std::memcpy(old_buff, t_buff, sizeof(t_buff)); + + try { + // Try to construct the new object + new (static_cast(t_buff)) T(std::forward(args)...); + } catch (...) { + // Restore the old buffer + std::memcpy(t_buff, old_buff, sizeof(t_buff)); + throw; + } + // Fetch the old buffer and detroy it. + std::swap_ranges(t_buff, t_buff + sizeof(t_buff), old_buff); + + destroy(); + std::memcpy(t_buff, old_buff, sizeof(t_buff)); + } + + m_Type = I; + return get(*this); + } + + constexpr std::size_t index() const noexcept { return m_Type; } + void swap(attempt &other) { + // TODO + } + + struct my_bad_variant_access : std::runtime_error { + my_bad_variant_access(std::size_t index) + : std::runtime_error{"The index should be " + std::to_string(index)} {} + }; + + public: + template + friend constexpr decltype(auto) get(attempt &variant) { + variant.throw_if_invalid(); + return *reinterpret_cast *>(variant.t_buff); + } + + template + friend constexpr decltype(auto) get(const attempt &variant) { + variant.throw_if_invalid(); + return *reinterpret_cast *>(variant.t_buff); + } + + private: + template + void throw_if_invalid() const { + static_assert(I < (sizeof...(Ts)), "Invalid index"); + + if (m_Type != I) throw my_bad_variant_access(m_Type); + } + + void destroy() { + visit( + [this](const auto &value) { + using type = std::decay_t; + value.~type(); + }, + *this); + } + + // Until c++23 enums are represented as ints. + int m_Type; + // https://stackoverflow.com/questions/71828288/why-is-stdaligned-storage-to-be-deprecated-in-c23-and-what-to-use-instead + alignas(Ts...) std::byte t_buff[std::max({sizeof(Ts)...})]; + + public: + using this_visitor_type = visitor_type; + + /// @brief Applies the visitor to the variant. Corresponds to the (3) + /// std::visit defintion. + template + friend constexpr auto visit(Visitor &&visitor, attempt &variant) + -> decltype(this_visitor_type::visit(std::forward(visitor), + variant.m_Type, variant.t_buff)) { + return this_visitor_type::visit(std::forward(visitor), + variant.m_Type, variant.t_buff); + } + + /// @brief Applies the visitor to the variant. Corresponds to the (4) + /// std::visit defintion. + template + friend constexpr auto visit(Visitor &&visitor, const attempt &variant) + -> decltype(this_visitor_type::visit(std::forward(visitor), + variant.m_Type, variant.t_buff)) { + return this_visitor_type::visit(std::forward(visitor), + variant.m_Type, variant.t_buff); + } +}; + +template >...>::value>> +constexpr const T &get(const attempt &variant) { + constexpr auto index = index_from_type::value; + return get(variant); +} + +template >...>::value>> +constexpr T &get(attempt &variant) { + constexpr auto index = index_from_type::value; + return get(variant); +} + +template +struct copy_control; + +template <> +struct copy_control { + copy_control() = default; + copy_control(const copy_control &other) = default; + copy_control &operator=(const copy_control &) = default; +}; + +template <> +struct copy_control { + copy_control() = default; + copy_control(const copy_control &other) = delete; + copy_control &operator=(const copy_control &) = delete; +}; + +template +using allow_copy = + copy_control...>>; + +template +struct variant : public attempt, private allow_copy { + using base = attempt; + + variant() = delete; + variant(const variant &) = default; + variant(variant &&) = delete; + + using base::base; + + variant &operator=(const variant &) = default; + variant &operator=(variant &&) = delete; + + using base::operator=; +}; + +#endif + +#endif + #ifndef CXXBRIDGE1_RUST_FN // https://cxx.rs/binding/fn.html template diff --git a/macro/src/derive.rs b/macro/src/derive.rs index 1c06ad892..538488a57 100644 --- a/macro/src/derive.rs +++ b/macro/src/derive.rs @@ -97,6 +97,38 @@ pub(crate) fn expand_enum(enm: &Enum, actual_derives: &mut Option) expanded } +pub(crate) fn expand_enum_unnamed(enm: &Enum, actual_derives: &mut Option) -> TokenStream { + let mut expanded = TokenStream::new(); + let mut traits = Vec::new(); + + for derive in &enm.derives { + let span = derive.span; + match derive.what { + // The traits below will always be implemented. This is consistent + // with the logic for c-like enums. + Trait::Copy | Trait::Clone | Trait::Eq | Trait::PartialEq => {} + Trait::Debug => expanded.extend(enum_debug(enm, span)), + Trait::Default => unreachable!(), + Trait::ExternType => unreachable!(), + Trait::Hash => traits.push(quote_spanned!(span=> ::cxx::core::hash::Hash)), + Trait::Ord => expanded.extend(enum_ord(enm, span)), + Trait::PartialOrd => expanded.extend(enum_partial_ord(enm, span)), + Trait::Serialize => traits.push(quote_spanned!(span=> ::serde::Serialize)), + Trait::Deserialize => traits.push(quote_spanned!(span=> ::serde::Deserialize)), + } + } + + let span = enm.name.rust.span(); + expanded.extend(enum_copy(enm, span)); + expanded.extend(enum_clone(enm, span)); + traits.push(quote_spanned!(span=> ::cxx::core::cmp::Eq)); + traits.push(quote_spanned!(span=> ::cxx::core::cmp::PartialEq)); + + *actual_derives = Some(quote!(#[derive(#(#traits),*)])); + + expanded +} + fn struct_copy(strct: &Struct, span: Span) -> TokenStream { let ident = &strct.name.rust; let generics = &strct.generics; diff --git a/macro/src/expand.rs b/macro/src/expand.rs index ff1ed2076..e5868b26e 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -7,8 +7,8 @@ use crate::syntax::qualified::QualifiedName; use crate::syntax::report::Errors; use crate::syntax::symbol::Symbol; use crate::syntax::{ - self, check, mangle, Api, Doc, Enum, ExternFn, ExternType, Impl, Lifetimes, Pair, Signature, - Struct, Trait, Type, TypeAlias, Types, + self, check, mangle, Api, Doc, Enum, ExternFn, ExternType, Impl, Lifetimes, Pair, + Signature, Struct, Trait, Type, TypeAlias, Types, CEnumOpts, }; use crate::type_id::Crate; use crate::{derive, generics}; @@ -35,6 +35,8 @@ pub(crate) fn bridge(mut ffi: Module) -> Result { let content = mem::take(&mut ffi.content); let trusted = ffi.unsafety.is_some(); let namespace = &ffi.namespace; + // TODO this generates my token stream for rust! I probably don't need to + // do anything here. let ref mut apis = syntax::parse_items(errors, content, trusted, namespace); #[cfg(feature = "experimental-enum-variants-from-header")] crate::load::load(errors, apis); @@ -68,7 +70,10 @@ fn expand(ffi: Module, doc: Doc, attrs: OtherAttrs, apis: &[Api], types: &Types) hidden.extend(expand_struct_operators(strct)); forbid.extend(expand_struct_forbid_drop(strct)); } - Api::Enum(enm) => expanded.extend(expand_enum(enm)), + Api::Enum(enm, opts) => expanded.extend(expand_enum(enm, opts)), + Api::EnumUnnamed(enm) => { + expanded.extend(expand_enum_unnamed(enm)); + } Api::CxxType(ety) => { let ident = &ety.name.rust; if !types.structs.contains_key(ident) && !types.enums.contains_key(ident) { @@ -316,11 +321,59 @@ fn expand_struct_forbid_drop(strct: &Struct) -> TokenStream { } } -fn expand_enum(enm: &Enum) -> TokenStream { +fn expand_enum_unnamed(enm: &Enum) -> TokenStream { + let ident = &enm.name.rust; + let doc = &enm.doc; + let attrs = &enm.attrs; + let generics = &enm.generics; + + let variants = enm.variants.iter().map(|variant| { + let doc = &variant.doc; + let attrs = &variant.attrs; + let ty = &variant.ty.as_ref().unwrap(); + let variant = &variant.name.rust; + // TODO add here the visibility and let it crash since the doc says + // that the syntax allows Visibilty specifiers but they should be + // rejected https://doc.rust-lang.org/reference/items/enumerations.html#variant-visibility + quote!(#doc #attrs #variant(#ty)) + }); + // TODO I don't understand the custom derives. + // let mut derives = None; + // let derived_traits = derive::expand_struct(strct, &mut derives); + + let span = ident.span(); + let visibility = enm.visibility; + let enum_token = enm.enum_token; + let type_id = type_id(&enm.name); + let enum_def = quote_spanned! {span=> + #visibility #enum_token #ident #generics { + #(#variants,)* + } + }; + + quote! { + #doc + // #derives + #attrs + #[repr(C)] + #enum_def + + unsafe impl #generics ::cxx::ExternType for #ident #generics { + #[allow(unused_attributes)] // incorrect lint + #[doc(hidden)] + type Id = #type_id; + type Kind = ::cxx::kind::Trivial; + } + + // #derived_traits + } +} + +fn expand_enum(enm: &Enum, opts:& CEnumOpts) -> TokenStream { let ident = &enm.name.rust; let doc = &enm.doc; let attrs = &enm.attrs; - let repr = &enm.repr; + let repr = &opts.repr; let type_id = type_id(&enm.name); let variants = enm.variants.iter().map(|variant| { let doc = &variant.doc; diff --git a/syntax/attrs.rs b/syntax/attrs.rs index 894b82b83..6e2dab832 100644 --- a/syntax/attrs.rs +++ b/syntax/attrs.rs @@ -282,7 +282,7 @@ fn parse_rust_name_attribute(meta: &Meta) -> Result { Err(Error::new_spanned(meta, "unsupported rust_name attribute")) } -#[derive(Clone)] +#[derive(Clone, Default)] pub(crate) struct OtherAttrs(Vec); impl OtherAttrs { diff --git a/syntax/cfg.rs b/syntax/cfg.rs index 83511d734..0542adbcd 100644 --- a/syntax/cfg.rs +++ b/syntax/cfg.rs @@ -12,6 +12,12 @@ pub(crate) enum CfgExpr { Not(Box), } +impl Default for CfgExpr { + fn default() -> Self { + Self::Unconditional + } +} + impl CfgExpr { pub(crate) fn merge(&mut self, expr: CfgExpr) { if let CfgExpr::Unconditional = self { diff --git a/syntax/check.rs b/syntax/check.rs index b5fd45e11..7a89af8e8 100644 --- a/syntax/check.rs +++ b/syntax/check.rs @@ -2,8 +2,9 @@ use crate::syntax::atom::Atom::{self, *}; use crate::syntax::report::Errors; use crate::syntax::visit::{self, Visit}; use crate::syntax::{ - error, ident, trivial, Api, Array, Enum, ExternFn, ExternType, Impl, Lang, Lifetimes, - NamedType, Ptr, Receiver, Ref, Signature, SliceRef, Struct, Trait, Ty1, Type, TypeAlias, Types, + error, ident, trivial, Api, Array, CEnumOpts, Enum, ExternFn, ExternType, Impl, Lang, + Lifetimes, NamedType, Ptr, Receiver, Ref, Signature, SliceRef, Struct, Trait, Ty1, Type, + TypeAlias, Types, }; use proc_macro2::{Delimiter, Group, Ident, TokenStream}; use quote::{quote, ToTokens}; @@ -64,7 +65,8 @@ fn do_typecheck(cx: &mut Check) { match api { Api::Include(_) => {} Api::Struct(strct) => check_api_struct(cx, strct), - Api::Enum(enm) => check_api_enum(cx, enm), + Api::Enum(enm, opts) => check_api_enum(cx, enm, opts), + Api::EnumUnnamed(enm) => check_api_enum_unnamed(cx, enm), Api::CxxType(ety) | Api::RustType(ety) => check_api_type(cx, ety), Api::CxxFunction(efn) | Api::RustFunction(efn) => check_api_fn(cx, efn), Api::TypeAlias(alias) => check_api_type_alias(cx, alias), @@ -353,11 +355,11 @@ fn check_api_struct(cx: &mut Check, strct: &Struct) { } } -fn check_api_enum(cx: &mut Check, enm: &Enum) { +fn check_api_enum(cx: &mut Check, enm: &Enum, opts: &CEnumOpts) { check_reserved_name(cx, &enm.name.rust); check_lifetimes(cx, &enm.generics); - if enm.variants.is_empty() && !enm.explicit_repr && !enm.variants_from_header { + if enm.variants.is_empty() && !opts.explicit_repr && !opts.variants_from_header { let span = span_for_enum_error(enm); cx.error( span, @@ -373,6 +375,40 @@ fn check_api_enum(cx: &mut Check, enm: &Enum) { } } +fn check_api_enum_unnamed(cx: &mut Check, enm: &Enum) { + let name = &enm.name.rust; + check_reserved_name(cx, name); + check_lifetimes(cx, &enm.generics); + + if enm.variants.is_empty() { + let span = span_for_enum_unnamed_error(enm); + cx.error(span, "enums without any variants are not supported"); + } + + // TODO how do I check if cxx has the variant. + // TODO I think the derives below make sense. + for derive in &enm.derives { + if derive.what == Trait::Default || derive.what == Trait::ExternType { + let msg = format!("derive({}) on shared struct is not supported", derive); + cx.error(derive, msg); + } + } + + for variant in &enm.variants { + let ty = variant.ty.as_ref().unwrap(); + if let Type::Fn(_) = ty { + cx.error( + variant, + "function pointers in a enum variant are not implemented yet", + ); + } else if is_unsized(cx, ty) { + let desc = describe(cx, ty); + let msg = format!("using {} by value is not supported", desc); + cx.error(variant, msg); + } + } +} + fn check_api_type(cx: &mut Check, ety: &ExternType) { check_reserved_name(cx, &ety.name.rust); check_lifetimes(cx, &ety.generics); @@ -682,6 +718,13 @@ fn span_for_enum_error(enm: &Enum) -> TokenStream { quote!(#enum_token #brace_token) } +fn span_for_enum_unnamed_error(enm: &Enum) -> TokenStream { + let enum_token = enm.enum_token; + let mut brace_token = Group::new(Delimiter::Brace, TokenStream::new()); + brace_token.set_span(enm.brace_token.span.join()); + quote!(#enum_token #brace_token) +} + fn span_for_receiver_error(receiver: &Receiver) -> TokenStream { let ampersand = receiver.ampersand; let lifetime = &receiver.lifetime; diff --git a/syntax/doc.rs b/syntax/doc.rs index bd8111eaf..9de9b4b93 100644 --- a/syntax/doc.rs +++ b/syntax/doc.rs @@ -2,6 +2,7 @@ use proc_macro2::TokenStream; use quote::{quote, ToTokens}; use syn::LitStr; +#[derive(Default)] pub(crate) struct Doc { pub hidden: bool, fragments: Vec, diff --git a/syntax/ident.rs b/syntax/ident.rs index bb2281e72..9b9b73116 100644 --- a/syntax/ident.rs +++ b/syntax/ident.rs @@ -34,12 +34,13 @@ pub(crate) fn check_all(cx: &mut Check, apis: &[Api]) { check(cx, &field.name); } } - Api::Enum(enm) => { + Api::Enum(enm, _) => { check(cx, &enm.name); for variant in &enm.variants { check(cx, &variant.name); } } + Api::EnumUnnamed(_) => {} // TODO Api::CxxType(ety) | Api::RustType(ety) => { check(cx, &ety.name); } diff --git a/syntax/improper.rs b/syntax/improper.rs index a19f5b7d6..bd2a8f336 100644 --- a/syntax/improper.rs +++ b/syntax/improper.rs @@ -18,6 +18,12 @@ impl<'a> Types<'a> { Definite(atom == RustString) } else if let Some(strct) = self.structs.get(ident) { Depends(&strct.name.rust) // iterate to fixed-point + } else if let Some(enm) = self.enums.get(ident) { + if enm.variants.iter().all(|variant| variant.ty.is_some()) { + Depends(&enm.name.rust) // iterate to fixed-point + } else { + Definite(self.rust.contains(ident) || self.aliases.contains_key(ident)) + } } else { Definite(self.rust.contains(ident) || self.aliases.contains_key(ident)) } diff --git a/syntax/mod.rs b/syntax/mod.rs index 5ff343b4d..d49cdf976 100644 --- a/syntax/mod.rs +++ b/syntax/mod.rs @@ -51,7 +51,8 @@ pub(crate) use self::types::Types; pub(crate) enum Api { Include(Include), Struct(Struct), - Enum(Enum), + Enum(Enum, CEnumOpts), + EnumUnnamed(Enum), CxxType(ExternType), CxxFunction(ExternFn), RustType(ExternType), @@ -130,6 +131,9 @@ pub(crate) struct Enum { pub generics: Lifetimes, pub brace_token: Brace, pub variants: Vec, +} + +pub(crate) struct CEnumOpts { pub variants_from_header: bool, #[allow(dead_code)] pub variants_from_header_attr: Option, @@ -253,10 +257,13 @@ pub(crate) struct Variant { pub doc: Doc, #[allow(dead_code)] // only used by cxxbridge-macro, not cxx-build pub attrs: OtherAttrs, + // #[allow(dead_code)] // only used by cxxbridge-macro, not cxx-build + // pub visibility: Token![pub], pub name: Pair, pub discriminant: Discriminant, #[allow(dead_code)] pub expr: Option, + pub ty: Option, } pub(crate) enum Type { diff --git a/syntax/names.rs b/syntax/names.rs index 7afa5a9e3..b28bd04ac 100644 --- a/syntax/names.rs +++ b/syntax/names.rs @@ -7,7 +7,7 @@ use syn::ext::IdentExt; use syn::parse::{Error, Parser, Result}; use syn::punctuated::Punctuated; -#[derive(Clone)] +#[derive(Clone, Default)] pub(crate) struct ForeignName { text: String, } diff --git a/syntax/parse.rs b/syntax/parse.rs index 850dcc8d1..f116a13b9 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -1,13 +1,13 @@ use crate::syntax::attrs::OtherAttrs; use crate::syntax::cfg::CfgExpr; -use crate::syntax::discriminant::DiscriminantSet; +use crate::syntax::discriminant::{Discriminant, DiscriminantSet}; use crate::syntax::file::{Item, ItemForeignMod}; use crate::syntax::report::Errors; use crate::syntax::Atom::*; use crate::syntax::{ - attrs, error, Api, Array, Derive, Doc, Enum, EnumRepr, ExternFn, ExternType, ForeignName, Impl, - Include, IncludeKind, Lang, Lifetimes, NamedType, Namespace, Pair, Ptr, Receiver, Ref, - Signature, SliceRef, Struct, Ty1, Type, TypeAlias, Var, Variant, + attrs, error, Api, Array, CEnumOpts, Derive, Doc, Enum, EnumRepr, ExternFn, ExternType, + ForeignName, Impl, Include, IncludeKind, Lang, Lifetimes, NamedType, Namespace, Pair, Ptr, + Receiver, Ref, Signature, SliceRef, Struct, Ty1, Type, TypeAlias, Var, Variant, }; use proc_macro2::{Delimiter, Group, Span, TokenStream, TokenTree}; use quote::{format_ident, quote, quote_spanned}; @@ -40,7 +40,22 @@ pub(crate) fn parse_items( Ok(strct) => apis.push(strct), Err(err) => cx.push(err), }, - Item::Enum(item) => apis.push(parse_enum(cx, item, namespace)), + Item::Enum(item) => { + // Check if we have any unnamed field - in this ase we have to make sure + // that there is no field with an determinant. + if item + .variants + .iter() + .any(|variant| matches!(variant.fields, Fields::Unnamed(_))) + { + match parse_enum_unnamed(cx, item, namespace) { + Ok(enumct) => apis.push(enumct), + Err(err) => cx.push(err), + } + } else { + apis.push(parse_enum(cx, item, namespace)); + } + } Item::ForeignMod(foreign_mod) => { parse_foreign_mod(cx, foreign_mod, &mut apis, trusted, namespace); } @@ -187,6 +202,126 @@ fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) -> })) } +/// Parses an "Rust" enum with unnamed fields. +fn parse_enum_unnamed(cx: &mut Errors, mut item: ItemEnum, namespace: &Namespace) -> Result { + let mut cfg = CfgExpr::Unconditional; + let mut doc = Doc::new(); + let mut derives = Vec::new(); + let mut namespace = namespace.clone(); + let mut cxx_name = None; + let mut rust_name = None; + let attrs = attrs::parse( + cx, + item.attrs.clone(), + attrs::Parser { + cfg: Some(&mut cfg), + doc: Some(&mut doc), + derives: Some(&mut derives), + namespace: Some(&mut namespace), + cxx_name: Some(&mut cxx_name), + rust_name: Some(&mut rust_name), + ..Default::default() + }, + ); + + // Generate the variants. + let mut variants = Vec::new(); + variants.reserve(item.variants.len()); + + for variant in &mut item.variants { + // Having both, Rust typed variants and c-style value variants is + // illegal. + if variant.discriminant.is_some() { + return Err(Error::new_spanned( + item, + "Mixed c-style enums with Rust enums", + )); + } + + // Get the unnamed field of the variant. + let field = match variant.fields { + // TODO(ddo) This is consistent with the struct but it makes the + // feature way less useful. + Fields::Unit => { + return Err(Error::new_spanned(item, "Unit variants are not supported")) + } + Fields::Named(_) => { + return Err(Error::new_spanned(item, "Named variants are not supported")) + } + Fields::Unnamed(ref unnamed_variant) => { + // Having move than one unnamed field is also illegal since we can't + // represent it in c++. + if unnamed_variant.unnamed.len() != 1 { + return Err(Error::new_spanned( + item, + "More than one unnamed field is not supported", + )); + } + unnamed_variant.unnamed.first().unwrap() + } + }; + + let ty = match parse_type(&field.ty) { + Ok(ty) => ty, + Err(err) => { + cx.push(err); + continue; + } + }; + + let mut cfg = CfgExpr::Unconditional; + let mut doc = Doc::new(); + + // This is kind of stupid - there are no "names" in c++ for the variant + // but just indices... + let name = pair(Namespace::default(), &variant.ident, None, None); + let variant_attrs = attrs::parse( + cx, + mem::take(&mut variant.attrs), + attrs::Parser { + cfg: Some(&mut cfg), + doc: Some(&mut doc), + ..Default::default() + }, + ); + variants.push(Variant { + cfg, + doc, + attrs: variant_attrs, + name, + discriminant: Discriminant::zero(), + expr: None, + ty: Some(ty), + }); + } + + let enum_token = item.enum_token; + let visibility = visibility_pub(&item.vis, enum_token.span); + let brace_token = item.brace_token; + let name = pair(namespace, &item.ident, cxx_name, rust_name); + + // TODO check for generics and discard them or use. + let generics = Lifetimes { + lt_token: None, + lifetimes: Punctuated::new(), + gt_token: None, + }; + + Ok(Api::EnumUnnamed(Enum { + cfg, + doc, + derives, + attrs, + visibility, + enum_token, + name, + generics, + brace_token, + variants, + })) +} + +/// Parses a "C-like" enum where all fields are units. fn parse_enum(cx: &mut Errors, item: ItemEnum, namespace: &Namespace) -> Api { let mut cfg = CfgExpr::Unconditional; let mut doc = Doc::new(); @@ -262,22 +397,26 @@ fn parse_enum(cx: &mut Errors, item: ItemEnum, namespace: &Namespace) -> Api { let variants_from_header_attr = variants_from_header; let variants_from_header = variants_from_header_attr.is_some(); - Api::Enum(Enum { - cfg, - doc, - derives, - attrs, - visibility, - enum_token, - name, - generics, - brace_token, - variants, - variants_from_header, - variants_from_header_attr, - repr, - explicit_repr, - }) + Api::Enum( + Enum { + cfg, + doc, + derives, + attrs, + visibility, + enum_token, + name, + generics, + brace_token, + variants, + }, + CEnumOpts { + variants_from_header, + variants_from_header_attr, + repr, + explicit_repr, + }, + ) } fn parse_variant( @@ -329,6 +468,7 @@ fn parse_variant( name, discriminant, expr, + ty: None, }) } diff --git a/syntax/pod.rs b/syntax/pod.rs index 506e53cb5..d29d8baf6 100644 --- a/syntax/pod.rs +++ b/syntax/pod.rs @@ -18,8 +18,18 @@ impl<'a> Types<'a> { .fields .iter() .all(|field| self.is_guaranteed_pod(&field.ty)) + } else if let Some(enm) = self.enums.get(ident) { + if enm.variants.iter().all(|variant| variant.ty.is_some()) { + enm.variants + .iter() + .all(|variant| self.is_guaranteed_pod(variant.ty.as_ref().unwrap())) + } else { + // This assumes that every variant has no ty set which + // means we're in the "c-style" branch. + true + } } else { - self.enums.contains_key(ident) + false } } Type::RustBox(_) diff --git a/syntax/tokens.rs b/syntax/tokens.rs index 05eddc703..f12b0b623 100644 --- a/syntax/tokens.rs +++ b/syntax/tokens.rs @@ -1,7 +1,7 @@ use crate::syntax::atom::Atom::*; use crate::syntax::{ - Array, Atom, Derive, Enum, EnumRepr, ExternFn, ExternType, Impl, Lifetimes, NamedType, Ptr, - Ref, Signature, SliceRef, Struct, Ty1, Type, TypeAlias, Var, + Array, Atom, Derive, Enum, EnumRepr, ExternFn, ExternType, Impl, Lifetimes, + NamedType, Ptr, Ref, Signature, SliceRef, Struct, Ty1, Type, TypeAlias, Var, Variant, }; use proc_macro2::{Ident, Span, TokenStream}; use quote::{quote_spanned, ToTokens}; @@ -56,6 +56,16 @@ impl ToTokens for Var { } } +impl ToTokens for Variant { + fn to_tokens(&self, tokens: &mut TokenStream) { + let name = &self.name; + name.rust.to_tokens(tokens); + if let Some(ref ty) = self.ty { + ty.to_tokens(tokens); + } + } +} + impl ToTokens for Ty1 { fn to_tokens(&self, tokens: &mut TokenStream) { let Ty1 { diff --git a/syntax/types.rs b/syntax/types.rs index 623a8b8d6..5b5610ddb 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -59,6 +59,7 @@ impl<'a> Types<'a> { }; let mut type_names = UnorderedSet::new(); + let mut function_names = UnorderedSet::new(); for api in apis { // The same identifier is permitted to be declared as both a shared @@ -71,11 +72,7 @@ impl<'a> Types<'a> { Api::Include(_) => {} Api::Struct(strct) => { let ident = &strct.name.rust; - if !type_names.insert(ident) - && (!cxx.contains(ident) - || structs.contains_key(ident) - || enums.contains_key(ident)) - { + if !type_names.insert(ident) && !cxx.contains(ident) { // If already declared as a struct or enum, or if // colliding with something other than an extern C++ // type, then error. @@ -87,8 +84,8 @@ impl<'a> Types<'a> { } add_resolution(&strct.name, &strct.generics); } - Api::Enum(enm) => { - match &enm.repr { + Api::Enum(enm, opts) => { + match &opts.repr { EnumRepr::Native { atom: _, repr_type } => { all.insert(repr_type); } @@ -96,24 +93,35 @@ impl<'a> Types<'a> { EnumRepr::Foreign { rust_type: _ } => {} } let ident = &enm.name.rust; - if !type_names.insert(ident) - && (!cxx.contains(ident) - || structs.contains_key(ident) - || enums.contains_key(ident)) - { + if !type_names.insert(ident) && !cxx.contains(ident) { // If already declared as a struct or enum, or if // colliding with something other than an extern C++ // type, then error. duplicate_name(cx, enm, ident); } enums.insert(ident, enm); - if enm.variants_from_header { + if opts.variants_from_header { // #![variants_from_header] enums are implicitly extern // C++ type. cxx.insert(&enm.name.rust); } add_resolution(&enm.name, &enm.generics); } + Api::EnumUnnamed(enm) => { + let ident = &enm.name.rust; + if !type_names.insert(ident) && !cxx.contains(ident) { + // If already declared as a struct or enum, or if + // colliding with something other than an extern C++ + // type, then error. + duplicate_name(cx, enm, ident); + } + + enums.insert(ident, enm); + for variant in &enm.variants { + visit(&mut all, variant.ty.as_ref().unwrap()); + } + add_resolution(&enm.name, &enm.generics); + } Api::CxxType(ety) => { let ident = &ety.name.rust; if !type_names.insert(ident) @@ -238,6 +246,35 @@ impl<'a> Types<'a> { }); } + let mut unresolved_enums = types.enums.keys(); + new_information = true; + while new_information { + new_information = false; + unresolved_enums.retain(|ident| { + let mut retain = false; + for var in &types.enums[ident].variants { + // If the ty is missing we're dealing with the C-style enum. + let Some(ty) = &var.ty else { + new_information = true; + return false; + }; + if match types.determine_improper_ctype(ty) { + ImproperCtype::Depends(inner) => { + retain = true; + types.struct_improper_ctypes.contains(inner) + } + ImproperCtype::Definite(improper) => improper, + } { + types.struct_improper_ctypes.insert(ident); + new_information = true; + return false; + } + } + // If all fields definite false, remove from unresolved_structs. + retain + }); + } + types } From 405edf87f555d95e4ebdc3acce0d6784af341264 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 16:07:54 +0100 Subject: [PATCH 02/37] fix visit --- .vscode/settings.json | 3 + demo/include/blobstore.h | 9 +-- demo/src/blobstore.cc | 28 +++++--- demo/src/main.rs | 19 ++++-- include/cxx.h | 139 +++++++++++++++++++++------------------ 5 files changed, 112 insertions(+), 86 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 8a1c2c130..7839a864b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,8 @@ { "search.exclude": { "**/target": true + }, + "files.associations": { + "variant": "cpp" } } diff --git a/demo/include/blobstore.h b/demo/include/blobstore.h index 05862e626..cd46c58f4 100644 --- a/demo/include/blobstore.h +++ b/demo/include/blobstore.h @@ -7,7 +7,6 @@ namespace blobstore { struct MultiBuf; struct BlobMetadata; -struct BlobWrapper; struct Foo; class BlobstoreClient { @@ -24,11 +23,9 @@ class BlobstoreClient { std::unique_ptr new_blobstore_client(); -Foo make_foo(); -void take_foo(const Foo&); - -BlobWrapper make_metadata(); -void take_metadata(const BlobWrapper&); +Foo make_enum(); +void take_enum(const Foo&); +void take_mut_enum(Foo&); } // namespace blobstore } // namespace org diff --git a/demo/src/blobstore.cc b/demo/src/blobstore.cc index f5700d23a..c8065725d 100644 --- a/demo/src/blobstore.cc +++ b/demo/src/blobstore.cc @@ -2,9 +2,9 @@ #include "demo/src/main.rs.h" #include #include +#include #include #include -#include #include namespace org { @@ -68,20 +68,28 @@ std::unique_ptr new_blobstore_client() { return std::make_unique(); } -Foo make_foo() { - return Foo{false}; -} +Foo make_enum() { return Foo{false}; } -void take_foo(const Foo& foo) { - std::cout << "The index of foo is " << foo.index() << std::endl; +std::ostream &operator<<(std::ostream &os, const BlobMetadata &md) { + return os; } -void take_metadata(const BlobWrapper&) { - +void take_enum(const Foo &foo) { + std::cout << "The index of foo is " << foo.index() << std::endl; + rust::visit( + [](const auto &v) { + std::cout << "The value of foo is " << v << std::endl; + }, + foo); } -BlobWrapper make_metadata() { - return {}; +void take_mut_enum(Foo &foo) { + take_enum(foo); + if (foo.index() == 0) { + foo = false; + } else { + foo = 111; + } } } // namespace blobstore diff --git a/demo/src/main.rs b/demo/src/main.rs index 28733ce0e..a534252f0 100644 --- a/demo/src/main.rs +++ b/demo/src/main.rs @@ -7,7 +7,7 @@ mod ffi { } struct BlobWrapper { - pub inner: BlobMetadata + pub inner: BlobMetadata, } /// A classic. @@ -37,8 +37,9 @@ mod ffi { fn tag(&self, blobid: u64, tag: &str); fn metadata(&self, blobid: u64) -> BlobMetadata; - fn make_foo() -> Foo; - fn take_foo(foo: &Foo); + fn make_enum() -> Foo; + fn take_enum(foo: &Foo); + fn take_mut_enum(foo: &mut Foo); } } @@ -59,12 +60,18 @@ pub fn next_chunk(buf: &mut MultiBuf) -> &[u8] { fn main() { let f = ffi::Foo::Bar(1); - ffi::take_foo(&f); - let f = ffi::make_foo(); + ffi::take_enum(&f); + let mut f = ffi::make_enum(); match f { ffi::Foo::Bar(val) => println!("The value is {val}"), ffi::Foo::Baz(val) => println!("The value is {val}"), - _ => {}, + _ => {} + } + ffi::take_mut_enum(&mut f); + match f { + ffi::Foo::Bar(val) => println!("The value is {val}"), + ffi::Foo::Baz(val) => println!("The value is {val}"), + _ => {} } let client = ffi::new_blobstore_client(); diff --git a/include/cxx.h b/include/cxx.h index baddcfdfd..880dc9cd7 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -57,8 +57,8 @@ class String final { static String lossy(const char16_t *) noexcept; static String lossy(const char16_t *, std::size_t) noexcept; - String &operator=(const String &) &noexcept; - String &operator=(String &&) &noexcept; + String &operator=(const String &) & noexcept; + String &operator=(String &&) & noexcept; explicit operator std::string() const; @@ -117,7 +117,7 @@ class Str final { Str(const char *); Str(const char *, std::size_t); - Str &operator=(const Str &) &noexcept = default; + Str &operator=(const Str &) & noexcept = default; explicit operator std::string() const; @@ -165,8 +165,8 @@ template <> struct copy_assignable_if { copy_assignable_if() noexcept = default; copy_assignable_if(const copy_assignable_if &) noexcept = default; - copy_assignable_if &operator=(const copy_assignable_if &) &noexcept = delete; - copy_assignable_if &operator=(copy_assignable_if &&) &noexcept = default; + copy_assignable_if &operator=(const copy_assignable_if &) & noexcept = delete; + copy_assignable_if &operator=(copy_assignable_if &&) & noexcept = default; }; } // namespace detail @@ -180,8 +180,8 @@ class Slice final Slice() noexcept; Slice(T *, std::size_t count) noexcept; - Slice &operator=(const Slice &) &noexcept = default; - Slice &operator=(Slice &&) &noexcept = default; + Slice &operator=(const Slice &) & noexcept = default; + Slice &operator=(Slice &&) & noexcept = default; T *data() const noexcept; std::size_t size() const noexcept; @@ -269,7 +269,7 @@ class Box final { explicit Box(const T &); explicit Box(T &&); - Box &operator=(Box &&) &noexcept; + Box &operator=(Box &&) & noexcept; const T *operator->() const noexcept; const T &operator*() const noexcept; @@ -314,7 +314,7 @@ class Vec final { Vec(Vec &&) noexcept; ~Vec() noexcept; - Vec &operator=(Vec &&) &noexcept; + Vec &operator=(Vec &&) & noexcept; Vec &operator=(const Vec &) &; std::size_t size() const noexcept; @@ -419,37 +419,7 @@ struct index_from_type compile_time_index, Ts>...>()> {}; template -struct visitor_type { - /// @brief The visit method which will pick the right type depending on the - /// `index` value. - template - constexpr static auto visit(Visitor &&visitor, std::size_t index, - std::byte *data) - -> decltype(visitor(*reinterpret_cast(data))) { - if (index == 0) { - return visitor(*reinterpret_cast(data)); - } - if constexpr (sizeof...(Remainder) != 0) { - return visitor_type::visit(std::forward(visitor), - --index, data); - } - throw std::out_of_range("invalid"); - } - - template - constexpr static auto visit(Visitor &&visitor, std::size_t index, - const std::byte *data) - -> decltype(visitor(*reinterpret_cast(data))) { - if (index == 0) { - return visitor(*reinterpret_cast(data)); - } - if constexpr (sizeof...(Remainder) != 0) { - return visitor_type::visit(std::forward(visitor), - --index, data); - } - throw std::out_of_range("invalid"); - } -}; +struct visitor_type; template struct attempt; @@ -461,10 +431,10 @@ template constexpr decltype(auto) get(const attempt &); template -constexpr auto visit(Visitor &&visitor, attempt &); +constexpr decltype(auto) visit(Visitor &&visitor, attempt &); template -constexpr auto visit(Visitor &&visitor, const attempt &); +constexpr decltype(auto) visit(Visitor &&visitor, const attempt &); template struct attempt { @@ -706,7 +676,7 @@ struct attempt { : std::runtime_error{"The index should be " + std::to_string(index)} {} }; - public: +public: template friend constexpr decltype(auto) get(attempt &variant) { variant.throw_if_invalid(); @@ -719,12 +689,13 @@ struct attempt { return *reinterpret_cast *>(variant.t_buff); } - private: +private: template void throw_if_invalid() const { static_assert(I < (sizeof...(Ts)), "Invalid index"); - if (m_Type != I) throw my_bad_variant_access(m_Type); + if (m_Type != I) + throw my_bad_variant_access(m_Type); } void destroy() { @@ -741,30 +712,64 @@ struct attempt { // https://stackoverflow.com/questions/71828288/why-is-stdaligned-storage-to-be-deprecated-in-c23-and-what-to-use-instead alignas(Ts...) std::byte t_buff[std::max({sizeof(Ts)...})]; - public: - using this_visitor_type = visitor_type; + template + friend struct visitor_type; +}; + +template +struct visitor_type { + template + constexpr static decltype(auto) visit(Visitor &&visitor, Variant &&variant) { + return visit(std::forward(visitor), variant.m_Type, + variant.t_buff); + } - /// @brief Applies the visitor to the variant. Corresponds to the (3) - /// std::visit defintion. + /// @brief The visit method which will pick the right type depending on the + /// `index` value. template - friend constexpr auto visit(Visitor &&visitor, attempt &variant) - -> decltype(this_visitor_type::visit(std::forward(visitor), - variant.m_Type, variant.t_buff)) { - return this_visitor_type::visit(std::forward(visitor), - variant.m_Type, variant.t_buff); + constexpr static auto visit(Visitor &&visitor, std::size_t index, + std::byte *data) + -> decltype(visitor(*reinterpret_cast(data))) { + if (index == 0) { + return visitor(*reinterpret_cast(data)); + } + if constexpr (sizeof...(Remainder) != 0) { + return visitor_type::visit(std::forward(visitor), + --index, data); + } + throw std::out_of_range("invalid"); } - /// @brief Applies the visitor to the variant. Corresponds to the (4) - /// std::visit defintion. template - friend constexpr auto visit(Visitor &&visitor, const attempt &variant) - -> decltype(this_visitor_type::visit(std::forward(visitor), - variant.m_Type, variant.t_buff)) { - return this_visitor_type::visit(std::forward(visitor), - variant.m_Type, variant.t_buff); + constexpr static auto visit(Visitor &&visitor, std::size_t index, + const std::byte *data) + -> decltype(visitor(*reinterpret_cast(data))) { + if (index == 0) { + return visitor(*reinterpret_cast(data)); + } + if constexpr (sizeof...(Remainder) != 0) { + return visitor_type::visit(std::forward(visitor), + --index, data); + } + throw std::out_of_range("invalid"); } }; +/// @brief Applies the visitor to the variant. Corresponds to the (3) +/// std::visit defintion. +template +constexpr decltype(auto) visit(Visitor &&visitor, attempt &variant) { + return visitor_type::visit(std::forward(visitor), variant); +} + +/// @brief Applies the visitor to the variant. Corresponds to the (4) +/// std::visit defintion. +template +constexpr decltype(auto) visit(Visitor &&visitor, + const attempt &variant) { + return visitor_type::visit(std::forward(visitor), variant); +} + template >...>::value>> @@ -818,6 +823,12 @@ struct variant : public attempt, private allow_copy { using base::operator=; }; +template +constexpr decltype(auto) visit(Visitor &&visitor, attempt &); + +template +constexpr decltype(auto) visit(Visitor &&visitor, const attempt &); + #endif #endif @@ -849,7 +860,7 @@ class Error final : public std::exception { ~Error() noexcept override; Error &operator=(const Error &) &; - Error &operator=(Error &&) &noexcept; + Error &operator=(Error &&) & noexcept; const char *what() const noexcept override; @@ -1221,7 +1232,7 @@ Box::~Box() noexcept { } template -Box &Box::operator=(Box &&other) &noexcept { +Box &Box::operator=(Box &&other) & noexcept { if (this->ptr) { this->drop(); } @@ -1309,7 +1320,7 @@ Vec::~Vec() noexcept { } template -Vec &Vec::operator=(Vec &&other) &noexcept { +Vec &Vec::operator=(Vec &&other) & noexcept { this->drop(); this->repr = other.repr; new (&other) Vec(); From 2a04047a9788badf72d6526aa11491bdde8dfad6 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 16:18:10 +0100 Subject: [PATCH 03/37] remove data_enums tests --- tests/ui/data_enums.rs | 8 -------- tests/ui/data_enums.stderr | 5 ----- 2 files changed, 13 deletions(-) delete mode 100644 tests/ui/data_enums.rs delete mode 100644 tests/ui/data_enums.stderr diff --git a/tests/ui/data_enums.rs b/tests/ui/data_enums.rs deleted file mode 100644 index aa23200dc..000000000 --- a/tests/ui/data_enums.rs +++ /dev/null @@ -1,8 +0,0 @@ -#[cxx::bridge] -mod ffi { - enum A { - Field(u64), - } -} - -fn main() {} diff --git a/tests/ui/data_enums.stderr b/tests/ui/data_enums.stderr deleted file mode 100644 index d8aa09e39..000000000 --- a/tests/ui/data_enums.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: enums with data are not supported yet - --> tests/ui/data_enums.rs:4:9 - | -4 | Field(u64), - | ^^^^^^^^^^ From 8b0acc17a768ebf85ec16853a0da1516499e707d Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 16:21:57 +0100 Subject: [PATCH 04/37] out-comment derive_enum_unnamed --- macro/src/derive.rs | 62 ++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/macro/src/derive.rs b/macro/src/derive.rs index 538488a57..6d33e81fc 100644 --- a/macro/src/derive.rs +++ b/macro/src/derive.rs @@ -97,37 +97,37 @@ pub(crate) fn expand_enum(enm: &Enum, actual_derives: &mut Option) expanded } -pub(crate) fn expand_enum_unnamed(enm: &Enum, actual_derives: &mut Option) -> TokenStream { - let mut expanded = TokenStream::new(); - let mut traits = Vec::new(); - - for derive in &enm.derives { - let span = derive.span; - match derive.what { - // The traits below will always be implemented. This is consistent - // with the logic for c-like enums. - Trait::Copy | Trait::Clone | Trait::Eq | Trait::PartialEq => {} - Trait::Debug => expanded.extend(enum_debug(enm, span)), - Trait::Default => unreachable!(), - Trait::ExternType => unreachable!(), - Trait::Hash => traits.push(quote_spanned!(span=> ::cxx::core::hash::Hash)), - Trait::Ord => expanded.extend(enum_ord(enm, span)), - Trait::PartialOrd => expanded.extend(enum_partial_ord(enm, span)), - Trait::Serialize => traits.push(quote_spanned!(span=> ::serde::Serialize)), - Trait::Deserialize => traits.push(quote_spanned!(span=> ::serde::Deserialize)), - } - } - - let span = enm.name.rust.span(); - expanded.extend(enum_copy(enm, span)); - expanded.extend(enum_clone(enm, span)); - traits.push(quote_spanned!(span=> ::cxx::core::cmp::Eq)); - traits.push(quote_spanned!(span=> ::cxx::core::cmp::PartialEq)); - - *actual_derives = Some(quote!(#[derive(#(#traits),*)])); - - expanded -} +// pub(crate) fn expand_enum_unnamed(enm: &Enum, actual_derives: &mut Option) -> TokenStream { +// let mut expanded = TokenStream::new(); +// let mut traits = Vec::new(); + +// for derive in &enm.derives { +// let span = derive.span; +// match derive.what { +// // The traits below will always be implemented. This is consistent +// // with the logic for c-like enums. +// Trait::Copy | Trait::Clone | Trait::Eq | Trait::PartialEq => {} +// Trait::Debug => expanded.extend(enum_debug(enm, span)), +// Trait::Default => unreachable!(), +// Trait::ExternType => unreachable!(), +// Trait::Hash => traits.push(quote_spanned!(span=> ::cxx::core::hash::Hash)), +// Trait::Ord => expanded.extend(enum_ord(enm, span)), +// Trait::PartialOrd => expanded.extend(enum_partial_ord(enm, span)), +// Trait::Serialize => traits.push(quote_spanned!(span=> ::serde::Serialize)), +// Trait::Deserialize => traits.push(quote_spanned!(span=> ::serde::Deserialize)), +// } +// } + +// let span = enm.name.rust.span(); +// expanded.extend(enum_copy(enm, span)); +// expanded.extend(enum_clone(enm, span)); +// traits.push(quote_spanned!(span=> ::cxx::core::cmp::Eq)); +// traits.push(quote_spanned!(span=> ::cxx::core::cmp::PartialEq)); + +// *actual_derives = Some(quote!(#[derive(#(#traits),*)])); + +// expanded +// } fn struct_copy(strct: &Struct, span: Span) -> TokenStream { let ident = &strct.name.rust; From a921eafd3337872d6d1a9432ed23aafb5101a1e4 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 16:25:49 +0100 Subject: [PATCH 05/37] fix clippy --- gen/src/write.rs | 2 +- syntax/parse.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index e741ac6d8..9b82f00f5 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -110,7 +110,7 @@ fn write_data_structures<'a>(out: &mut OutFile<'a>, apis: &'a [Api]) { } Api::EnumUnnamed(enm) => { out.next_section(); - write_enum_using(out, &enm); + write_enum_using(out, enm); } Api::RustType(ety) => { out.next_section(); diff --git a/syntax/parse.rs b/syntax/parse.rs index f116a13b9..4ba5ac4a8 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -225,8 +225,7 @@ fn parse_enum_unnamed(cx: &mut Errors, mut item: ItemEnum, namespace: &Namespace ); // Generate the variants. - let mut variants = Vec::new(); - variants.reserve(item.variants.len()); + let mut variants = Vec::with_capacity(item.variants.len()); for variant in &mut item.variants { // Having both, Rust typed variants and c-style value variants is From 3fccd51b9bc5a0557d94606cf3627354ace5c9b4 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 16:29:59 +0100 Subject: [PATCH 06/37] Rename Foo to BlobEnum --- demo/include/blobstore.h | 8 ++++---- demo/src/blobstore.cc | 18 +++++++++--------- demo/src/main.rs | 19 +++++++++---------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/demo/include/blobstore.h b/demo/include/blobstore.h index cd46c58f4..f8634353b 100644 --- a/demo/include/blobstore.h +++ b/demo/include/blobstore.h @@ -7,7 +7,7 @@ namespace blobstore { struct MultiBuf; struct BlobMetadata; -struct Foo; +struct BlobEnum; class BlobstoreClient { public: @@ -23,9 +23,9 @@ class BlobstoreClient { std::unique_ptr new_blobstore_client(); -Foo make_enum(); -void take_enum(const Foo&); -void take_mut_enum(Foo&); +BlobEnum make_enum(); +void take_enum(const BlobEnum&); +void take_mut_enum(BlobEnum&); } // namespace blobstore } // namespace org diff --git a/demo/src/blobstore.cc b/demo/src/blobstore.cc index c8065725d..56616dcea 100644 --- a/demo/src/blobstore.cc +++ b/demo/src/blobstore.cc @@ -68,27 +68,27 @@ std::unique_ptr new_blobstore_client() { return std::make_unique(); } -Foo make_enum() { return Foo{false}; } +BlobEnum make_enum() { return BlobEnum{false}; } std::ostream &operator<<(std::ostream &os, const BlobMetadata &md) { return os; } -void take_enum(const Foo &foo) { - std::cout << "The index of foo is " << foo.index() << std::endl; +void take_enum(const BlobEnum &enm) { + std::cout << "The index of foo is " << enm.index() << std::endl; rust::visit( [](const auto &v) { std::cout << "The value of foo is " << v << std::endl; }, - foo); + enm); } -void take_mut_enum(Foo &foo) { - take_enum(foo); - if (foo.index() == 0) { - foo = false; +void take_mut_enum(BlobEnum &enm) { + take_enum(enm); + if (enm.index() == 0) { + enm = false; } else { - foo = 111; + enm = 111; } } diff --git a/demo/src/main.rs b/demo/src/main.rs index a534252f0..d0396cb23 100644 --- a/demo/src/main.rs +++ b/demo/src/main.rs @@ -11,8 +11,7 @@ mod ffi { } /// A classic. - // #[derive(Debug)] - enum Foo { + enum BlobEnum { /// This is my doc Bar(i32), Baz(bool), @@ -37,9 +36,9 @@ mod ffi { fn tag(&self, blobid: u64, tag: &str); fn metadata(&self, blobid: u64) -> BlobMetadata; - fn make_enum() -> Foo; - fn take_enum(foo: &Foo); - fn take_mut_enum(foo: &mut Foo); + fn make_enum() -> BlobEnum; + fn take_enum(enm: &BlobEnum); + fn take_mut_enum(enm: &mut BlobEnum); } } @@ -59,18 +58,18 @@ pub fn next_chunk(buf: &mut MultiBuf) -> &[u8] { } fn main() { - let f = ffi::Foo::Bar(1); + let f = ffi::BlobEnum::Bar(1); ffi::take_enum(&f); let mut f = ffi::make_enum(); match f { - ffi::Foo::Bar(val) => println!("The value is {val}"), - ffi::Foo::Baz(val) => println!("The value is {val}"), + ffi::BlobEnum::Bar(val) => println!("The value is {val}"), + ffi::BlobEnum::Baz(val) => println!("The value is {val}"), _ => {} } ffi::take_mut_enum(&mut f); match f { - ffi::Foo::Bar(val) => println!("The value is {val}"), - ffi::Foo::Baz(val) => println!("The value is {val}"), + ffi::BlobEnum::Bar(val) => println!("The value is {val}"), + ffi::BlobEnum::Baz(val) => println!("The value is {val}"), _ => {} } From 9be6b245841280ac9d3763d1dea5e907abd3ce27 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 16:36:02 +0100 Subject: [PATCH 07/37] fix rustc 1.60 compilation --- demo/src/blobstore.cc | 1 + syntax/types.rs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/demo/src/blobstore.cc b/demo/src/blobstore.cc index 56616dcea..1d52239fe 100644 --- a/demo/src/blobstore.cc +++ b/demo/src/blobstore.cc @@ -71,6 +71,7 @@ std::unique_ptr new_blobstore_client() { BlobEnum make_enum() { return BlobEnum{false}; } std::ostream &operator<<(std::ostream &os, const BlobMetadata &md) { + os << "The size [" << md.size << "] and some tags..."; return os; } diff --git a/syntax/types.rs b/syntax/types.rs index 5b5610ddb..3b8da0a73 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -254,9 +254,9 @@ impl<'a> Types<'a> { let mut retain = false; for var in &types.enums[ident].variants { // If the ty is missing we're dealing with the C-style enum. - let Some(ty) = &var.ty else { - new_information = true; - return false; + let ty = match &var.ty { + None => return false, + Some(ty) => ty }; if match types.determine_improper_ctype(ty) { ImproperCtype::Depends(inner) => { From a66c29d415a96f29e4e5a7b63a09683b5621d118 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Wed, 27 Dec 2023 16:47:45 +0100 Subject: [PATCH 08/37] try fix windows --- .bazelrc | 1 + demo/build.rs | 2 ++ tools/buck/toolchains/BUCK | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.bazelrc b/.bazelrc index f6ec1aba3..11220b783 100644 --- a/.bazelrc +++ b/.bazelrc @@ -2,3 +2,4 @@ build --enable_platform_specific_config build:linux --@rules_rust//:extra_rustc_flags=-Clink-arg=-fuse-ld=lld build:linux --cxxopt=-std=c++17 build:macos --cxxopt=-std=c++17 +build:windows --cxxopt=/std:c++17 diff --git a/demo/build.rs b/demo/build.rs index a1fccb722..8e1ba23db 100644 --- a/demo/build.rs +++ b/demo/build.rs @@ -2,6 +2,8 @@ fn main() { cxx_build::bridge("src/main.rs") .file("src/blobstore.cc") .flag_if_supported("-std=c++17") + .flag_if_supported("/std:c++17") + .flag_if_supported("/Zc:__cplusplus") .compile("cxxbridge-demo"); println!("cargo:rerun-if-changed=src/main.rs"); diff --git a/tools/buck/toolchains/BUCK b/tools/buck/toolchains/BUCK index e120a29ba..8789f7822 100644 --- a/tools/buck/toolchains/BUCK +++ b/tools/buck/toolchains/BUCK @@ -9,7 +9,7 @@ system_cxx_toolchain( cxx_flags = select({ "config//os:linux": ["-std=c++17"], "config//os:macos": ["-std=c++17"], - "config//os:windows": [], + "config//os:windows": ["/std:c++17"], }), link_flags = select({ "config//os:linux": ["-lstdc++"], From 0c4c2b1652a811e16b477e66657c3e0b27f78a7f Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 09:17:00 +0100 Subject: [PATCH 09/37] try fix windows on buck --- tools/buck/toolchains/BUCK | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buck/toolchains/BUCK b/tools/buck/toolchains/BUCK index 8789f7822..572795095 100644 --- a/tools/buck/toolchains/BUCK +++ b/tools/buck/toolchains/BUCK @@ -9,7 +9,7 @@ system_cxx_toolchain( cxx_flags = select({ "config//os:linux": ["-std=c++17"], "config//os:macos": ["-std=c++17"], - "config//os:windows": ["/std:c++17"], + "config//os:windows": ["/std:c++17", "/Zc:__cplusplus"], }), link_flags = select({ "config//os:linux": ["-lstdc++"], From 279a3c26db087cd2ea2b81c820e59c93f588af9f Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 09:38:05 +0100 Subject: [PATCH 10/37] Add doc for mvsc usage --- .vscode/settings.json | 3 ++- include/cxx.h | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 7839a864b..4e81ec6c6 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,6 +3,7 @@ "**/target": true }, "files.associations": { - "variant": "cpp" + "variant": "cpp", + "string": "cpp" } } diff --git a/include/cxx.h b/include/cxx.h index 880dc9cd7..8f2413bca 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -14,6 +14,14 @@ #include #include #include +// If you're using enums and variants on windows, you need to pass also +// `/Zc:__cplusplus` as a compiler to make __cplusplus work correctly. If users +// ever report that they have a too old compiler to `/Zc:__cplusplus` we still +// may support a `_MSVC_LANG` define. +// +// Sources: +// https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/ +// https://learn.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-170 #if __cplusplus >= 201703L #include #endif From 198073e1ee0175c250cb186db3b2ee110de2d73b Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 09:38:38 +0100 Subject: [PATCH 11/37] rename foo --- demo/src/blobstore.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/demo/src/blobstore.cc b/demo/src/blobstore.cc index 1d52239fe..04920bb2b 100644 --- a/demo/src/blobstore.cc +++ b/demo/src/blobstore.cc @@ -76,10 +76,10 @@ std::ostream &operator<<(std::ostream &os, const BlobMetadata &md) { } void take_enum(const BlobEnum &enm) { - std::cout << "The index of foo is " << enm.index() << std::endl; + std::cout << "The index of enum is " << enm.index() << std::endl; rust::visit( [](const auto &v) { - std::cout << "The value of foo is " << v << std::endl; + std::cout << "The value of enum is " << v << std::endl; }, enm); } From 3747084cd7b094c44bc108c2d4c46b2f6ec1cf71 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 09:50:11 +0100 Subject: [PATCH 12/37] revert unnecessary changes --- gen/src/mod.rs | 1 - gen/src/write.rs | 46 ++++++++++++++++++++++----------------------- macro/src/expand.rs | 2 -- syntax/attrs.rs | 2 +- syntax/cfg.rs | 6 ------ syntax/doc.rs | 1 - syntax/names.rs | 2 +- syntax/types.rs | 1 - 8 files changed, 24 insertions(+), 37 deletions(-) diff --git a/gen/src/mod.rs b/gen/src/mod.rs index 1f6386fa5..f24846a7e 100644 --- a/gen/src/mod.rs +++ b/gen/src/mod.rs @@ -159,7 +159,6 @@ pub(super) fn generate(syntax: File, opt: &Opt) -> Result { )); } } - // TODO here is the c++ side - maybe I have to add here something. cfg::strip(errors, cfg_errors, opt.cfg_evaluator.as_ref(), apis); errors.propagate()?; diff --git a/gen/src/write.rs b/gen/src/write.rs index 9b82f00f5..beef6c0fe 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -110,7 +110,7 @@ fn write_data_structures<'a>(out: &mut OutFile<'a>, apis: &'a [Api]) { } Api::EnumUnnamed(enm) => { out.next_section(); - write_enum_using(out, enm); + write_enum_unnamed(out, enm); } Api::RustType(ety) => { out.next_section(); @@ -344,32 +344,30 @@ fn write_enum_decl(out: &mut OutFile, enm: &Enum, opts: &CEnumOpts) { writeln!(out, ";"); } -fn write_enum_using(out: &mut OutFile, enm: &Enum) { - write!( - out, - "struct {} final : public ::rust::variant<", - enm.name.cxx - ); - - let mut iter = enm.variants.iter().peekable(); - while let Some(value) = iter.next() { - write_type(out, value.ty.as_ref().unwrap()); - if iter.peek().is_some() { - write!(out, ", "); +fn write_enum_unnamed(out: &mut OutFile, enm: &Enum) { + write!(out, "struct {} final : public", enm.name.cxx); + + /// Writes something like `::rust::variant` with type1... + /// being cxx types of the enum's variants. + fn write_variants(out: &mut OutFile, enm: &Enum) { + write!(out, "::rust::variant<"); + let mut iter = enm.variants.iter().peekable(); + while let Some(value) = iter.next() { + write_type(out, value.ty.as_ref().unwrap()); + if iter.peek().is_some() { + write!(out, ", "); + } } + write!(out, ">"); } - writeln!(out, "> {{"); - write!(out, "using base = ::rust::variant<"); - let mut iter = enm.variants.iter().peekable(); - while let Some(value) = iter.next() { - write_type(out, value.ty.as_ref().unwrap()); - if iter.peek().is_some() { - write!(out, ", "); - } - } + write_variants(out, enm); + writeln!(out, "{{"); + + write!(out, "using base = "); + write_variants(out, enm); + writeln!(out, ";"); - writeln!(out, ">;"); writeln!(out, " using base::base;"); writeln!(out, " using base::operator=;"); writeln!(out, "}};"); @@ -446,7 +444,7 @@ fn write_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum, opts: &'a CEnumOpts) { writeln!(out, "#endif // {}", guard); } -fn check_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum, opts: & 'a CEnumOpts) { +fn check_enum<'a>(out: &mut OutFile<'a>, enm: &'a Enum, opts: &'a CEnumOpts) { let repr = match &opts.repr { #[cfg(feature = "experimental-enum-variants-from-header")] EnumRepr::Foreign { .. } => return, diff --git a/macro/src/expand.rs b/macro/src/expand.rs index e5868b26e..1ab58cf88 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -35,8 +35,6 @@ pub(crate) fn bridge(mut ffi: Module) -> Result { let content = mem::take(&mut ffi.content); let trusted = ffi.unsafety.is_some(); let namespace = &ffi.namespace; - // TODO this generates my token stream for rust! I probably don't need to - // do anything here. let ref mut apis = syntax::parse_items(errors, content, trusted, namespace); #[cfg(feature = "experimental-enum-variants-from-header")] crate::load::load(errors, apis); diff --git a/syntax/attrs.rs b/syntax/attrs.rs index 6e2dab832..894b82b83 100644 --- a/syntax/attrs.rs +++ b/syntax/attrs.rs @@ -282,7 +282,7 @@ fn parse_rust_name_attribute(meta: &Meta) -> Result { Err(Error::new_spanned(meta, "unsupported rust_name attribute")) } -#[derive(Clone, Default)] +#[derive(Clone)] pub(crate) struct OtherAttrs(Vec); impl OtherAttrs { diff --git a/syntax/cfg.rs b/syntax/cfg.rs index 0542adbcd..83511d734 100644 --- a/syntax/cfg.rs +++ b/syntax/cfg.rs @@ -12,12 +12,6 @@ pub(crate) enum CfgExpr { Not(Box), } -impl Default for CfgExpr { - fn default() -> Self { - Self::Unconditional - } -} - impl CfgExpr { pub(crate) fn merge(&mut self, expr: CfgExpr) { if let CfgExpr::Unconditional = self { diff --git a/syntax/doc.rs b/syntax/doc.rs index 9de9b4b93..bd8111eaf 100644 --- a/syntax/doc.rs +++ b/syntax/doc.rs @@ -2,7 +2,6 @@ use proc_macro2::TokenStream; use quote::{quote, ToTokens}; use syn::LitStr; -#[derive(Default)] pub(crate) struct Doc { pub hidden: bool, fragments: Vec, diff --git a/syntax/names.rs b/syntax/names.rs index b28bd04ac..7afa5a9e3 100644 --- a/syntax/names.rs +++ b/syntax/names.rs @@ -7,7 +7,7 @@ use syn::ext::IdentExt; use syn::parse::{Error, Parser, Result}; use syn::punctuated::Punctuated; -#[derive(Clone, Default)] +#[derive(Clone)] pub(crate) struct ForeignName { text: String, } diff --git a/syntax/types.rs b/syntax/types.rs index 3b8da0a73..bfd2d7e74 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -59,7 +59,6 @@ impl<'a> Types<'a> { }; let mut type_names = UnorderedSet::new(); - let mut function_names = UnorderedSet::new(); for api in apis { // The same identifier is permitted to be declared as both a shared From 79f114bf0741fb3b457b38ee183eca4560a97a2a Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 10:06:31 +0100 Subject: [PATCH 13/37] add missing checks --- gen/src/cfg.rs | 3 +-- include/cxx.h | 6 +++--- syntax/ident.rs | 3 +-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/gen/src/cfg.rs b/gen/src/cfg.rs index a3d64c3ea..717043a1b 100644 --- a/gen/src/cfg.rs +++ b/gen/src/cfg.rs @@ -29,10 +29,9 @@ pub(super) fn strip( Api::Struct(strct) => strct .fields .retain(|field| eval(cx, cfg_errors, cfg_evaluator, &field.cfg)), - Api::Enum(enm, _) => enm + Api::Enum(enm, _) | Api::EnumUnnamed(enm) => enm .variants .retain(|variant| eval(cx, cfg_errors, cfg_evaluator, &variant.cfg)), - // TODO What is this doing?? _ => {} } } diff --git a/include/cxx.h b/include/cxx.h index 8f2413bca..5785d65d3 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -426,7 +426,7 @@ struct index_from_type std::size_t, compile_time_index, Ts>...>()> {}; -template +template struct visitor_type; template @@ -720,12 +720,12 @@ struct attempt { // https://stackoverflow.com/questions/71828288/why-is-stdaligned-storage-to-be-deprecated-in-c23-and-what-to-use-instead alignas(Ts...) std::byte t_buff[std::max({sizeof(Ts)...})]; - template + template friend struct visitor_type; }; template -struct visitor_type { +struct visitor_type { template constexpr static decltype(auto) visit(Visitor &&visitor, Variant &&variant) { return visit(std::forward(visitor), variant.m_Type, diff --git a/syntax/ident.rs b/syntax/ident.rs index 9b9b73116..7c26f6c07 100644 --- a/syntax/ident.rs +++ b/syntax/ident.rs @@ -34,13 +34,12 @@ pub(crate) fn check_all(cx: &mut Check, apis: &[Api]) { check(cx, &field.name); } } - Api::Enum(enm, _) => { + Api::Enum(enm, _) | Api::EnumUnnamed(enm) => { check(cx, &enm.name); for variant in &enm.variants { check(cx, &variant.name); } } - Api::EnumUnnamed(_) => {} // TODO Api::CxxType(ety) | Api::RustType(ety) => { check(cx, &ety.name); } From 008aea0c1642900192c74613f34bc57cca911323 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 10:15:55 +0100 Subject: [PATCH 14/37] concise match --- gen/src/cfg.rs | 3 +-- gen/src/namespace.rs | 3 +-- gen/src/write.rs | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/gen/src/cfg.rs b/gen/src/cfg.rs index 717043a1b..5f3417ca8 100644 --- a/gen/src/cfg.rs +++ b/gen/src/cfg.rs @@ -113,8 +113,7 @@ impl Api { match self { Api::Include(include) => &include.cfg, Api::Struct(strct) => &strct.cfg, - Api::Enum(enm, _) => &enm.cfg, - Api::EnumUnnamed(enm) => &enm.cfg, + Api::Enum(enm, _) | Api::EnumUnnamed(enm) => &enm.cfg, Api::CxxType(ety) | Api::RustType(ety) => &ety.cfg, Api::CxxFunction(efn) | Api::RustFunction(efn) => &efn.cfg, Api::TypeAlias(alias) => &alias.cfg, diff --git a/gen/src/namespace.rs b/gen/src/namespace.rs index f61563ee2..bbe61cc65 100644 --- a/gen/src/namespace.rs +++ b/gen/src/namespace.rs @@ -6,8 +6,7 @@ impl Api { match self { Api::CxxFunction(efn) | Api::RustFunction(efn) => &efn.name.namespace, Api::CxxType(ety) | Api::RustType(ety) => &ety.name.namespace, - Api::Enum(enm, _) => &enm.name.namespace, - Api::EnumUnnamed(enm) => &enm.name.namespace, + Api::Enum(enm, _) | Api::EnumUnnamed(enm) => &enm.name.namespace, Api::Struct(strct) => &strct.name.namespace, Api::Impl(_) | Api::Include(_) | Api::TypeAlias(_) => Default::default(), } diff --git a/gen/src/write.rs b/gen/src/write.rs index beef6c0fe..6ba7c35c3 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -34,9 +34,8 @@ pub(super) fn gen(apis: &[Api], types: &Types, opt: &Opt, header: bool) -> Vec true, + Api::Struct(_) | Api::CxxType(_) | Api::RustType(_) | Api::EnumUnnamed(_) => true, Api::Enum(enm, _) => !out.types.cxx.contains(&enm.name.rust), - Api::EnumUnnamed(_) => true, _ => false, }; From e30596185f97adcf9a383a60c46293eebe89c772 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 10:29:34 +0100 Subject: [PATCH 15/37] add static_asserts --- src/cxx.cc | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) diff --git a/src/cxx.cc b/src/cxx.cc index 2522d61aa..cc326b699 100644 --- a/src/cxx.cc +++ b/src/cxx.cc @@ -449,6 +449,202 @@ static_assert(!std::is_same::const_iterator, Vec::iterator>::value, "Vec::const_iterator != Vec::iterator"); + +#if __cplusplus >= 201703L +/// @brief Below static_asserts for out variant. +/// +/// We go quite overboard with the tests since the variant has some intricate +/// function resolution based on the types it can hold. +namespace detail { + +/// @brief A type which can only be move-constructed/assigned but not copied. +/// +/// If it's part of the variant's types the variant cannot not be copy +/// constructed. The variant can be constructed/assigned with an rvalue of this +/// type. +struct MoveType { + MoveType() = default; + MoveType(MoveType &&other) = default; + MoveType(const MoveType &other) = delete; + + MoveType &operator=(const MoveType &other) = delete; + MoveType &operator=(MoveType &&other) = default; +}; + +/// @brief A type which can only be copy-constructed/assigned but not moved. +/// +/// If every type in the variant can be copy constructed, then the variant can +/// be copy constructed. The variant it can be constructed/assigned with a +/// lvalue of this type. +struct CopyType { + CopyType() = default; + CopyType(const CopyType &other) = default; + CopyType(CopyType &&other) = delete; + CopyType(int, std::string) {} + + CopyType &operator=(const CopyType &other) = default; + CopyType &operator=(CopyType &&other) = delete; +}; + +/// @brief A type which can be copy and move constructed/assigned. +/// +/// If every type in the variant can be copy constructed, then the variant can +/// be copy constructed. The variant it can be constructed/assigned with this +/// type. +struct CopyAndMoveType { + CopyAndMoveType() = default; + CopyAndMoveType(const CopyAndMoveType &other) = default; + CopyAndMoveType(CopyAndMoveType &&other) = default; + + CopyAndMoveType &operator=(const CopyAndMoveType &other) = default; + CopyAndMoveType &operator=(CopyAndMoveType &&other) = default; +}; + + +/// @brief A type which can be neither copy nor move constructed/assigned. +/// +/// If it's part of the variant's types the variant cannot not be copy +/// constructed. The variant can not constructed/assigned with any value of this +/// type - but you can emplace it. +struct NoCopyAndNoMoveType { + NoCopyAndNoMoveType() = default; + NoCopyAndNoMoveType(const NoCopyAndNoMoveType &other) = delete; + NoCopyAndNoMoveType(NoCopyAndNoMoveType &&other) = delete; + + NoCopyAndNoMoveType &operator=(const NoCopyAndNoMoveType &other) = delete; + NoCopyAndNoMoveType &operator=(NoCopyAndNoMoveType &&other) = delete; +}; + +// Checks with a variant containing all types. +using all_variant = + variant; + +// Check of copy and move construction/assignment. +static_assert(!std::is_default_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(!std::is_copy_assignable_v); +static_assert(!std::is_move_assignable_v); + +// Checks for converting construction/assignment. For every type we check +// construction by value, move, ref, const ref, in_place_type_t and +// in_place_index_t. The last two should always work. + +// Checks for the first type (MoveType). We expect that everything requiring a +// copy will not work. +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert( + std::is_constructible_v>); +static_assert(std::is_constructible_v>); + +// Checks for the second type (CopyType). We expect that everything requiring a +// move will not work. +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert( + std::is_constructible_v>); +static_assert(std::is_constructible_v< + all_variant, std::in_place_type_t, int, std::string>); +static_assert(std::is_constructible_v>); + +// Checks for the third type (CopyAndMoveType). We expect that everything works. +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v>); +static_assert(std::is_constructible_v>); + +// Checks for the fourth type (NoCopyAndNoMoveType). We expect that only +// in_place constructors work. +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert( + !std::is_constructible_v); +static_assert(std::is_constructible_v< + all_variant, std::in_place_type_t>); +static_assert(std::is_constructible_v>); + +// Checks with invalid input. +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); + +// Checks with the construction from std::variant. We can't move and we can't +// copy the types - therefore there is no safe way to construct our variant +// from std::variant. +using std_all_variant = + std::variant; + +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(!std::is_assignable_v); +static_assert(!std::is_assignable_v); + +// Checks with a variant consisting of only movable types. +using move_variant = variant; + +// Check of copy and move construction/assignment. Since we disallow move +// constructors/assignments none of the copy and move constructors should work. +static_assert(!std::is_default_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(!std::is_copy_assignable_v); +static_assert(!std::is_move_assignable_v); + +// Checks with the construction from std::variant. Since we can move every type +// we should be able to move construct from std::variant. Copy constructors +// should not work since MoveType has no copy constructor. +using std_move_variant = std::variant; +static_assert(!std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(!std::is_assignable_v); +static_assert(std::is_assignable_v); + +// Checks with a variant consisting of only copyable types. +using copy_variant = variant; + +// Check of copy and move construction/assignment. Copy constructor/assignment +// should work since every type can be copy constructed/assigned. +static_assert(!std::is_default_constructible_v); +static_assert(!std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(std::is_copy_assignable_v); +static_assert(!std::is_move_assignable_v); + +// Checks with the construction from std::variant. Since we can copy every type +// we should be able to copy construct from std::variant. Move constructors +// should not work since CopyType has no move constructor. +using std_copy_variant = std::variant; +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v); +static_assert(std::is_assignable_v); +static_assert(std::is_assignable_v); + +// Checks with a variant containing duplicate types: Constructors where just +// the type is specified should not work since there is no way of telling which +// index to use. +using duplicate_variant = variant; +static_assert(!std::is_constructible_v); +static_assert(!std::is_constructible_v, int>); +static_assert( + std::is_constructible_v, int>); +static_assert( + std::is_constructible_v, int>); +static_assert( + !std::is_constructible_v, int>); + + +} // namespace detail +#endif + static const char *errorCopy(const char *ptr, std::size_t len) { char *copy = new char[len]; std::memcpy(copy, ptr, len); From da26ad7408e5c9485917b633232af28cd9bc0b98 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 12:04:40 +0100 Subject: [PATCH 16/37] handle visibility --- demo/src/main.rs | 6 ++++++ include/cxx.h | 6 +++--- macro/src/expand.rs | 10 ++++++---- syntax/mod.rs | 6 +++--- syntax/parse.rs | 2 ++ tests/ui/enum_visibility.rs | 9 +++++++++ tests/ui/enum_visibility.stderr | 7 +++++++ 7 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 tests/ui/enum_visibility.rs create mode 100644 tests/ui/enum_visibility.stderr diff --git a/demo/src/main.rs b/demo/src/main.rs index d0396cb23..1b427c1b3 100644 --- a/demo/src/main.rs +++ b/demo/src/main.rs @@ -18,6 +18,12 @@ mod ffi { Bam(BlobMetadata), } + enum BlobCLike { + Bar, + Baz, + Bam, + } + // Rust types and signatures exposed to C++. extern "Rust" { type MultiBuf; diff --git a/include/cxx.h b/include/cxx.h index 5785d65d3..c16d37093 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -16,8 +16,8 @@ #include // If you're using enums and variants on windows, you need to pass also // `/Zc:__cplusplus` as a compiler to make __cplusplus work correctly. If users -// ever report that they have a too old compiler to `/Zc:__cplusplus` we still -// may support a `_MSVC_LANG` define. +// ever report that they have a too old compiler to `/Zc:__cplusplus` we may +// fallback to the `_MSVC_LANG` define. // // Sources: // https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/ @@ -415,7 +415,7 @@ constexpr std::size_t compile_time_index() noexcept { static_assert(sizeof...(Values) != 0, "Provide at least one value"); static_assert(exactly_once::value, "Ambiguous index"); std::size_t ii = 0; - // Note: This might be a too simplistic implementation. The order of `ii++` + // TODO: This might be a too simplistic implementation. The order of `ii++` // should be guaranteed by the standard but I haven't found where. return ((ii++ * static_cast(Values)) + ...); } diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 1ab58cf88..eb5318653 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -329,11 +329,13 @@ fn expand_enum_unnamed(enm: &Enum) -> TokenStream { let doc = &variant.doc; let attrs = &variant.attrs; let ty = &variant.ty.as_ref().unwrap(); + let vis = &variant.vis; let variant = &variant.name.rust; - // TODO add here the visibility and let it crash since the doc says - // that the syntax allows Visibilty specifiers but they should be - // rejected https://doc.rust-lang.org/reference/items/enumerations.html#variant-visibility - quote!(#doc #attrs #variant(#ty)) + // We add the visibility if defined here and let it crash since the doc + // says that the syntax allows visibility specifiers but they should be + // rejected. See + // https://doc.rust-lang.org/reference/items/enumerations.html#variant-visibility + quote!(#doc #attrs #variant(#vis #ty)) }); // TODO I don't understand the custom derives. // let mut derives = None; diff --git a/syntax/mod.rs b/syntax/mod.rs index d49cdf976..9810e6cb3 100644 --- a/syntax/mod.rs +++ b/syntax/mod.rs @@ -38,7 +38,7 @@ use self::symbol::Symbol; use proc_macro2::{Ident, Span}; use syn::punctuated::Punctuated; use syn::token::{Brace, Bracket, Paren}; -use syn::{Attribute, Expr, Generics, Lifetime, LitInt, Token, Type as RustType}; +use syn::{Attribute, Expr, Generics, Lifetime, LitInt, Token, Type as RustType, Visibility}; pub(crate) use self::atom::Atom; pub(crate) use self::derive::{Derive, Trait}; @@ -257,8 +257,8 @@ pub(crate) struct Variant { pub doc: Doc, #[allow(dead_code)] // only used by cxxbridge-macro, not cxx-build pub attrs: OtherAttrs, - // #[allow(dead_code)] // only used by cxxbridge-macro, not cxx-build - // pub visibility: Token![pub], + #[allow(dead_code)] // only used by cxxbridge-macro, not cxx-build + pub vis: Visibility, pub name: Pair, pub discriminant: Discriminant, #[allow(dead_code)] diff --git a/syntax/parse.rs b/syntax/parse.rs index 4ba5ac4a8..73738d5b2 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -288,6 +288,7 @@ fn parse_enum_unnamed(cx: &mut Errors, mut item: ItemEnum, namespace: &Namespace doc, attrs: variant_attrs, name, + vis: field.vis.clone(), discriminant: Discriminant::zero(), expr: None, ty: Some(ty), @@ -464,6 +465,7 @@ fn parse_variant( cfg, doc, attrs, + vis: Visibility::Inherited, name, discriminant, expr, diff --git a/tests/ui/enum_visibility.rs b/tests/ui/enum_visibility.rs new file mode 100644 index 000000000..fe53c5b4a --- /dev/null +++ b/tests/ui/enum_visibility.rs @@ -0,0 +1,9 @@ +#[cxx::bridge] +mod ffi { + enum Bad { + A(pub i32), + B(bool), + } +} + +fn main() {} diff --git a/tests/ui/enum_visibility.stderr b/tests/ui/enum_visibility.stderr new file mode 100644 index 000000000..0a6688557 --- /dev/null +++ b/tests/ui/enum_visibility.stderr @@ -0,0 +1,7 @@ +error[E0449]: visibility qualifiers are not permitted here + --> tests/ui/enum_visibility.rs:4:11 + | +4 | A(pub i32), + | ^^^ + | + = note: enum variants and their fields always share the visibility of the enum they are in From a6449ea954be2fc94f28fa92c050c8a5be9a75c2 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 12:09:44 +0100 Subject: [PATCH 17/37] add mixed enums test --- tests/ui/enum_mixed.rs | 9 +++++++++ tests/ui/enum_mixed.stderr | 8 ++++++++ 2 files changed, 17 insertions(+) create mode 100644 tests/ui/enum_mixed.rs create mode 100644 tests/ui/enum_mixed.stderr diff --git a/tests/ui/enum_mixed.rs b/tests/ui/enum_mixed.rs new file mode 100644 index 000000000..24174b036 --- /dev/null +++ b/tests/ui/enum_mixed.rs @@ -0,0 +1,9 @@ +#[cxx::bridge] +mod ffi { + enum Bad { + A(pub i32), + B = 1, + } +} + +fn main() {} \ No newline at end of file diff --git a/tests/ui/enum_mixed.stderr b/tests/ui/enum_mixed.stderr new file mode 100644 index 000000000..742a6c09e --- /dev/null +++ b/tests/ui/enum_mixed.stderr @@ -0,0 +1,8 @@ +error: Mixed c-style enums with Rust enums + --> tests/ui/enum_mixed.rs:3:5 + | +3 | / enum Bad { +4 | | A(pub i32), +5 | | B = 1, +6 | | } + | |_____^ From 6aa6873c5674a8024e892dd73cc5e00a2054343c Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 12:14:48 +0100 Subject: [PATCH 18/37] extend ui tests --- tests/ui/enum_many_types_tuple.rs | 9 +++++++++ tests/ui/enum_many_types_tuple.stderr | 8 ++++++++ tests/ui/enum_mixed.rs | 4 ++-- tests/ui/enum_mixed.stderr | 2 +- tests/ui/enum_named.rs | 9 +++++++++ tests/ui/enum_named.stderr | 8 ++++++++ 6 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 tests/ui/enum_many_types_tuple.rs create mode 100644 tests/ui/enum_many_types_tuple.stderr create mode 100644 tests/ui/enum_named.rs create mode 100644 tests/ui/enum_named.stderr diff --git a/tests/ui/enum_many_types_tuple.rs b/tests/ui/enum_many_types_tuple.rs new file mode 100644 index 000000000..c845caaca --- /dev/null +++ b/tests/ui/enum_many_types_tuple.rs @@ -0,0 +1,9 @@ +#[cxx::bridge] +mod ffi { + enum Bad { + A(i32, bool), + B(i32), + } +} + +fn main() {} diff --git a/tests/ui/enum_many_types_tuple.stderr b/tests/ui/enum_many_types_tuple.stderr new file mode 100644 index 000000000..e54d535ff --- /dev/null +++ b/tests/ui/enum_many_types_tuple.stderr @@ -0,0 +1,8 @@ +error: More than one unnamed field is not supported + --> tests/ui/enum_many_types_tuple.rs:3:5 + | +3 | / enum Bad { +4 | | A(i32, bool), +5 | | B(i32), +6 | | } + | |_____^ diff --git a/tests/ui/enum_mixed.rs b/tests/ui/enum_mixed.rs index 24174b036..d2dbae0c0 100644 --- a/tests/ui/enum_mixed.rs +++ b/tests/ui/enum_mixed.rs @@ -1,9 +1,9 @@ #[cxx::bridge] mod ffi { enum Bad { - A(pub i32), + A(i32), B = 1, } } -fn main() {} \ No newline at end of file +fn main() {} diff --git a/tests/ui/enum_mixed.stderr b/tests/ui/enum_mixed.stderr index 742a6c09e..ab546bcc0 100644 --- a/tests/ui/enum_mixed.stderr +++ b/tests/ui/enum_mixed.stderr @@ -2,7 +2,7 @@ error: Mixed c-style enums with Rust enums --> tests/ui/enum_mixed.rs:3:5 | 3 | / enum Bad { -4 | | A(pub i32), +4 | | A(i32), 5 | | B = 1, 6 | | } | |_____^ diff --git a/tests/ui/enum_named.rs b/tests/ui/enum_named.rs new file mode 100644 index 000000000..493487f39 --- /dev/null +++ b/tests/ui/enum_named.rs @@ -0,0 +1,9 @@ +#[cxx::bridge] +mod ffi { + enum Bad { + A{age: i32}, + B(i32), + } +} + +fn main() {} diff --git a/tests/ui/enum_named.stderr b/tests/ui/enum_named.stderr new file mode 100644 index 000000000..12a9033cc --- /dev/null +++ b/tests/ui/enum_named.stderr @@ -0,0 +1,8 @@ +error: Named variants are not supported + --> tests/ui/enum_named.rs:3:5 + | +3 | / enum Bad { +4 | | A{age: i32}, +5 | | B(i32), +6 | | } + | |_____^ From 85d1ae45385d91758b690625629a264c85e17353 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 14:02:46 +0100 Subject: [PATCH 19/37] support lifetimes --- demo/src/main.rs | 9 +++-- syntax/parse.rs | 90 +++++++++++++++++++++++------------------------- 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/demo/src/main.rs b/demo/src/main.rs index 1b427c1b3..6c93b0989 100644 --- a/demo/src/main.rs +++ b/demo/src/main.rs @@ -6,8 +6,13 @@ mod ffi { tags: Vec, } - struct BlobWrapper { - pub inner: BlobMetadata, + struct Blob<'a> { + size: &'a usize, + } + + enum BlobB<'a> { + Foo(Blob<'a>), + Bar(&'a usize), } /// A classic. diff --git a/syntax/parse.rs b/syntax/parse.rs index 73738d5b2..72b1a5de0 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -70,38 +70,10 @@ pub(crate) fn parse_items( apis } -fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) -> Result { - let mut cfg = CfgExpr::Unconditional; - let mut doc = Doc::new(); - let mut derives = Vec::new(); - let mut namespace = namespace.clone(); - let mut cxx_name = None; - let mut rust_name = None; - let attrs = attrs::parse( - cx, - mem::take(&mut item.attrs), - attrs::Parser { - cfg: Some(&mut cfg), - doc: Some(&mut doc), - derives: Some(&mut derives), - namespace: Some(&mut namespace), - cxx_name: Some(&mut cxx_name), - rust_name: Some(&mut rust_name), - ..Default::default() - }, - ); - - let named_fields = match item.fields { - Fields::Named(fields) => fields, - Fields::Unit => return Err(Error::new_spanned(item, "unit structs are not supported")), - Fields::Unnamed(_) => { - return Err(Error::new_spanned(item, "tuple structs are not supported")); - } - }; - +fn parse_generics(cx: &mut Errors, generics: Generics) -> Lifetimes { let mut lifetimes = Punctuated::new(); let mut has_unsupported_generic_param = false; - for pair in item.generics.params.into_pairs() { + for pair in generics.params.into_pairs() { let (param, punct) = pair.into_tuple(); match param { GenericParam::Lifetime(param) => { @@ -117,14 +89,14 @@ fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) -> } GenericParam::Type(param) => { if !has_unsupported_generic_param { - let msg = "struct with generic type parameter is not supported yet"; + let msg = "user defined type with generic type parameter is not supported yet"; cx.error(¶m, msg); has_unsupported_generic_param = true; } } GenericParam::Const(param) => { if !has_unsupported_generic_param { - let msg = "struct with const generic parameter is not supported yet"; + let msg = "user defined type with const generic parameter is not supported yet"; cx.error(¶m, msg); has_unsupported_generic_param = true; } @@ -132,13 +104,49 @@ fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) -> } } - if let Some(where_clause) = &item.generics.where_clause { + if let Some(where_clause) = &generics.where_clause { cx.error( where_clause, - "struct with where-clause is not supported yet", + "user defined type with where-clause is not supported yet", ); } + Lifetimes { + lt_token: generics.lt_token, + lifetimes, + gt_token: generics.gt_token, + } +} + +fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) -> Result { + let mut cfg = CfgExpr::Unconditional; + let mut doc = Doc::new(); + let mut derives = Vec::new(); + let mut namespace = namespace.clone(); + let mut cxx_name = None; + let mut rust_name = None; + let attrs = attrs::parse( + cx, + mem::take(&mut item.attrs), + attrs::Parser { + cfg: Some(&mut cfg), + doc: Some(&mut doc), + derives: Some(&mut derives), + namespace: Some(&mut namespace), + cxx_name: Some(&mut cxx_name), + rust_name: Some(&mut rust_name), + ..Default::default() + }, + ); + + let named_fields = match item.fields { + Fields::Named(fields) => fields, + Fields::Unit => return Err(Error::new_spanned(item, "unit structs are not supported")), + Fields::Unnamed(_) => { + return Err(Error::new_spanned(item, "tuple structs are not supported")); + } + }; + let mut fields = Vec::new(); for field in named_fields.named { let ident = field.ident.unwrap(); @@ -181,11 +189,7 @@ fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) -> let struct_token = item.struct_token; let visibility = visibility_pub(&item.vis, struct_token.span); let name = pair(namespace, &item.ident, cxx_name, rust_name); - let generics = Lifetimes { - lt_token: item.generics.lt_token, - lifetimes, - gt_token: item.generics.gt_token, - }; + let generics = parse_generics(cx, item.generics); let brace_token = named_fields.brace_token; Ok(Api::Struct(Struct { @@ -299,13 +303,7 @@ fn parse_enum_unnamed(cx: &mut Errors, mut item: ItemEnum, namespace: &Namespace let visibility = visibility_pub(&item.vis, enum_token.span); let brace_token = item.brace_token; let name = pair(namespace, &item.ident, cxx_name, rust_name); - - // TODO check for generics and discard them or use. - let generics = Lifetimes { - lt_token: None, - lifetimes: Punctuated::new(), - gt_token: None, - }; + let generics = parse_generics(cx, item.generics); Ok(Api::EnumUnnamed(Enum { cfg, From 94d0c96a7d4842d13673fe2d68b88284934c11b0 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 28 Dec 2023 17:14:57 +0100 Subject: [PATCH 20/37] add some tests --- flags/src/impl.rs | 1 + gen/src/write.rs | 12 +++++++----- tests/ffi/Cargo.toml | 2 +- tests/ffi/lib.rs | 12 ++++++++++++ tests/ffi/tests.cc | 13 +++++++++++++ tests/ffi/tests.h | 5 +++++ tests/test.rs | 9 +++++++++ 7 files changed, 48 insertions(+), 6 deletions(-) diff --git a/flags/src/impl.rs b/flags/src/impl.rs index 4f7b8fb4b..e341d2c83 100644 --- a/flags/src/impl.rs +++ b/flags/src/impl.rs @@ -7,6 +7,7 @@ pub const STD: &str = { #[cfg(feature = "c++17")] (flags = ["-std=c++17", "/std:c++17"]); + // TODO if I add a feature for the enum add here /Zc:__cplusplus for windows. #[cfg(feature = "c++20")] (flags = ["-std=c++20", "/std:c++20"]); diff --git a/gen/src/write.rs b/gen/src/write.rs index 6ba7c35c3..fedd7d226 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -344,7 +344,7 @@ fn write_enum_decl(out: &mut OutFile, enm: &Enum, opts: &CEnumOpts) { } fn write_enum_unnamed(out: &mut OutFile, enm: &Enum) { - write!(out, "struct {} final : public", enm.name.cxx); + write!(out, "struct {} final : public ", enm.name.cxx); /// Writes something like `::rust::variant` with type1... /// being cxx types of the enum's variants. @@ -363,12 +363,14 @@ fn write_enum_unnamed(out: &mut OutFile, enm: &Enum) { write_variants(out, enm); writeln!(out, "{{"); - write!(out, "using base = "); + write!(out, " using base = "); write_variants(out, enm); writeln!(out, ";"); - - writeln!(out, " using base::base;"); - writeln!(out, " using base::operator=;"); + writeln!(out, " {}() = delete;", enm.name.cxx); + writeln!(out, " {}(const {}&) = default;", enm.name.cxx, enm.name.cxx); + writeln!(out, " {}({}&&) = default;", enm.name.cxx, enm.name.cxx); + writeln!(out, " using base::base;"); + writeln!(out, " using base::operator=;"); writeln!(out, "}};"); } diff --git a/tests/ffi/Cargo.toml b/tests/ffi/Cargo.toml index 167bbb02d..b6830787d 100644 --- a/tests/ffi/Cargo.toml +++ b/tests/ffi/Cargo.toml @@ -9,7 +9,7 @@ publish = false path = "lib.rs" [dependencies] -cxx = { path = "../..", default-features = false } +cxx = { path = "../..", features = ["c++17"] } [build-dependencies] cxx-build = { path = "../../gen/build" } diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index ef8d5b371..17a1b6210 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -92,6 +92,16 @@ pub mod ffi { s: &'a str, } + enum EnumSimple { + AVal(bool), + BVal(Shared), + } + + enum EnumImproper { + AVal(i32), + BVal(SharedString), + } + unsafe extern "C++" { include!("tests/ffi/tests.h"); @@ -131,6 +141,8 @@ pub mod ffi { fn c_return_nested_ns_enum(n: u16) -> ABEnum; fn c_return_const_ptr(n: usize) -> *const C; fn c_return_mut_ptr(n: usize) -> *mut C; + fn c_return_enum_simple(first: bool) -> EnumSimple; + fn c_return_enum_improper(first: bool) -> EnumImproper; fn c_take_primitive(n: usize); fn c_take_shared(shared: Shared); diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc index 8cf74bebb..9616ec34d 100644 --- a/tests/ffi/tests.cc +++ b/tests/ffi/tests.cc @@ -229,6 +229,19 @@ std::unique_ptr c_return_borrow(const std::string &s) { return std::unique_ptr(new Borrow(s)); } +EnumSimple c_return_enum_simple(bool first) { + if(first) + return false; + return Shared{123}; +} + +EnumImproper c_return_enum_improper(bool first) { + if (first) { + return 2; + } + return SharedString{rust::String{"Some string"}}; +} + void c_take_primitive(size_t n) { if (n == 2020) { cxx_test_suite_set_correct(); diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h index dc02e4ff8..786b88625 100644 --- a/tests/ffi/tests.h +++ b/tests/ffi/tests.h @@ -39,6 +39,9 @@ struct Shared; struct SharedString; enum class Enum : uint16_t; +struct EnumSimple; +struct EnumImproper; + class C { public: C(size_t n); @@ -124,6 +127,8 @@ ::A::B::ABEnum c_return_nested_ns_enum(uint16_t n); std::unique_ptr c_return_borrow(const std::string &s); const C *c_return_const_ptr(size_t n); C *c_return_mut_ptr(size_t n); +EnumSimple c_return_enum_simple(bool first); +EnumImproper c_return_enum_improper(bool first); void c_take_primitive(size_t n); void c_take_shared(Shared shared); diff --git a/tests/test.rs b/tests/test.rs index 6ef9a8293..4a89c9b00 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -379,3 +379,12 @@ fn test_raw_ptr() { assert_eq!(2025, unsafe { ffi::c_take_const_ptr(c3) }); assert_eq!(2025, unsafe { ffi::c_take_mut_ptr(c3 as *mut ffi::C) }); // deletes c3 } + +#[test] +fn test_data_enums() { + let v1 = ffi::c_return_enum_improper(true); + assert!(matches!(v1, ffi::EnumImproper::AVal(2))); + + // let v1 = ffi::c_return_enum_simple(true); + // assert!(matches!(v1, ffi::EnumSimple::AVal(false))); +} From c1e4f0501e1b0a8be9c5388d059a027e25928678 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Fri, 29 Dec 2023 11:22:37 +0100 Subject: [PATCH 21/37] add tests --- gen/src/write.rs | 4 ++-- include/cxx.h | 26 +++++++++++++------------- syntax/pod.rs | 13 ++++--------- tests/ffi/build.rs | 1 + tests/test.rs | 30 ++++++++++++++++++++++++++---- 5 files changed, 46 insertions(+), 28 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index fedd7d226..25f603d95 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -367,8 +367,8 @@ fn write_enum_unnamed(out: &mut OutFile, enm: &Enum) { write_variants(out, enm); writeln!(out, ";"); writeln!(out, " {}() = delete;", enm.name.cxx); - writeln!(out, " {}(const {}&) = default;", enm.name.cxx, enm.name.cxx); - writeln!(out, " {}({}&&) = default;", enm.name.cxx, enm.name.cxx); + writeln!(out, " {0}(const {0}&) = default;", enm.name.cxx); + writeln!(out, " {0}({0}&&) = delete;", enm.name.cxx); writeln!(out, " using base::base;"); writeln!(out, " using base::operator=;"); writeln!(out, "}};"); diff --git a/include/cxx.h b/include/cxx.h index c16d37093..4cdf3a463 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -410,23 +410,23 @@ template struct exactly_once : std::conditional_t::value == 1, std::true_type, std::false_type> {}; -template -constexpr std::size_t compile_time_index() noexcept { - static_assert(sizeof...(Values) != 0, "Provide at least one value"); - static_assert(exactly_once::value, "Ambiguous index"); - std::size_t ii = 0; - // TODO: This might be a too simplistic implementation. The order of `ii++` - // should be guaranteed by the standard but I haven't found where. - return ((ii++ * static_cast(Values)) + ...); -} + +template +struct index_from_type_impl; + +template +struct index_from_type_impl {}; + +template +struct index_from_type_impl + : std::conditional_t, + index_from_type_impl> {}; template struct index_from_type - : std::integral_constant< - std::size_t, - compile_time_index, Ts>...>()> {}; + : index_from_type_impl<0, std::is_same_v, Ts>...> {}; -template +template struct visitor_type; template diff --git a/syntax/pod.rs b/syntax/pod.rs index d29d8baf6..0d951e5af 100644 --- a/syntax/pod.rs +++ b/syntax/pod.rs @@ -19,15 +19,10 @@ impl<'a> Types<'a> { .iter() .all(|field| self.is_guaranteed_pod(&field.ty)) } else if let Some(enm) = self.enums.get(ident) { - if enm.variants.iter().all(|variant| variant.ty.is_some()) { - enm.variants - .iter() - .all(|variant| self.is_guaranteed_pod(variant.ty.as_ref().unwrap())) - } else { - // This assumes that every variant has no ty set which - // means we're in the "c-style" branch. - true - } + // The data enums are not pods, since the c++ side + // implements custom copy constructors and destructors. The + // c-like enums are pods, though. + !enm.variants.iter().any(|variant| variant.ty.is_some()) } else { false } diff --git a/tests/ffi/build.rs b/tests/ffi/build.rs index a1a64b7f0..31bc597c9 100644 --- a/tests/ffi/build.rs +++ b/tests/ffi/build.rs @@ -10,6 +10,7 @@ fn main() { let mut build = cxx_build::bridges(sources); build.file("tests.cc"); build.flag_if_supported(cxxbridge_flags::STD); + build.flag_if_supported("/Zc:__cplusplus"); build.warnings_into_errors(cfg!(deny_warnings)); if cfg!(not(target_env = "msvc")) { build.define("CXX_TEST_INSTANTIATIONS", None); diff --git a/tests/test.rs b/tests/test.rs index 4a89c9b00..865551b80 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -382,9 +382,31 @@ fn test_raw_ptr() { #[test] fn test_data_enums() { - let v1 = ffi::c_return_enum_improper(true); - assert!(matches!(v1, ffi::EnumImproper::AVal(2))); + use ffi::{c_return_enum_improper, c_return_enum_simple}; + use ffi::{EnumImproper, EnumSimple}; - // let v1 = ffi::c_return_enum_simple(true); - // assert!(matches!(v1, ffi::EnumSimple::AVal(false))); + assert!(matches!( + c_return_enum_simple(true), + EnumSimple::AVal(false) + )); + + assert!(matches!( + c_return_enum_simple(false), + EnumSimple::BVal(ffi::Shared { z: 123 }) + )); + + assert!(matches!( + c_return_enum_improper(true), + EnumImproper::AVal(2) + )); + + let msg = "Some string".to_string(); + match c_return_enum_improper(false) { + EnumImproper::BVal(val) => { + assert_eq!(val.msg, msg); + } + EnumImproper::AVal(_) => { + assert!(false); + } + } } From 81c0cb9521dc579e9b87d8ae970f238843326d96 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Fri, 29 Dec 2023 11:54:01 +0100 Subject: [PATCH 22/37] add check --- include/cxx.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/cxx.h b/include/cxx.h index 4cdf3a463..ab4534a09 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -410,7 +410,6 @@ template struct exactly_once : std::conditional_t::value == 1, std::true_type, std::false_type> {}; - template struct index_from_type_impl; @@ -424,7 +423,10 @@ struct index_from_type_impl template struct index_from_type - : index_from_type_impl<0, std::is_same_v, Ts>...> {}; + : index_from_type_impl<0, std::is_same_v, Ts>...> { + static_assert(exactly_once, Ts>...>::value, + "Index must be unique"); +}; template struct visitor_type; From 790e4e3eef4371554dbe305ea5842c6658c343d3 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Fri, 29 Dec 2023 12:45:21 +0100 Subject: [PATCH 23/37] support lifetimes --- gen/src/write.rs | 15 ++++++++++++++- tests/ffi/lib.rs | 6 ++++++ tests/ffi/tests.cc | 8 +++++++- tests/ffi/tests.h | 2 ++ tests/test.rs | 12 ++++++++++-- 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 25f603d95..1d7d83fd6 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -352,7 +352,20 @@ fn write_enum_unnamed(out: &mut OutFile, enm: &Enum) { write!(out, "::rust::variant<"); let mut iter = enm.variants.iter().peekable(); while let Some(value) = iter.next() { - write_type(out, value.ty.as_ref().unwrap()); + let ty = value.ty.as_ref().unwrap(); + match ty { + Type::Ref(r) => { + // References are not allowed in variants, so we wrap them + // into std::reference_wrapper. + write!(out, "std::reference_wrapper<"); + write_type_space(out, &r.inner); + if !r.mutable { + write!(out, "const "); + } + write!(out, ">"); + }, + _ => write_type(out, ty), + } if iter.peek().is_some() { write!(out, ", "); } diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index 17a1b6210..44e04ecdc 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -102,6 +102,11 @@ pub mod ffi { BVal(SharedString), } + enum EnumWithLifeTime<'a> { + AVal(&'a str), + BVal(&'a i32), + } + unsafe extern "C++" { include!("tests/ffi/tests.h"); @@ -143,6 +148,7 @@ pub mod ffi { fn c_return_mut_ptr(n: usize) -> *mut C; fn c_return_enum_simple(first: bool) -> EnumSimple; fn c_return_enum_improper(first: bool) -> EnumImproper; + fn c_return_enum_with_lifetime<'a>(val: &'a i32) -> EnumWithLifeTime<'a>; fn c_take_primitive(n: usize); fn c_take_shared(shared: Shared); diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc index 9616ec34d..1b34792dc 100644 --- a/tests/ffi/tests.cc +++ b/tests/ffi/tests.cc @@ -230,7 +230,7 @@ std::unique_ptr c_return_borrow(const std::string &s) { } EnumSimple c_return_enum_simple(bool first) { - if(first) + if (first) return false; return Shared{123}; } @@ -242,7 +242,13 @@ EnumImproper c_return_enum_improper(bool first) { return SharedString{rust::String{"Some string"}}; } +EnumWithLifeTime c_return_enum_with_lifetime(const int &val) { + return std::reference_wrapper(val); +} + void c_take_primitive(size_t n) { + int v = 123; + c_return_enum_with_lifetime(v); if (n == 2020) { cxx_test_suite_set_correct(); } diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h index 786b88625..e4b464aef 100644 --- a/tests/ffi/tests.h +++ b/tests/ffi/tests.h @@ -41,6 +41,7 @@ enum class Enum : uint16_t; struct EnumSimple; struct EnumImproper; +struct EnumWithLifeTime; class C { public: @@ -129,6 +130,7 @@ const C *c_return_const_ptr(size_t n); C *c_return_mut_ptr(size_t n); EnumSimple c_return_enum_simple(bool first); EnumImproper c_return_enum_improper(bool first); +EnumWithLifeTime c_return_enum_with_lifetime(const int& val); void c_take_primitive(size_t n); void c_take_shared(Shared shared); diff --git a/tests/test.rs b/tests/test.rs index 865551b80..930f90852 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -382,8 +382,8 @@ fn test_raw_ptr() { #[test] fn test_data_enums() { - use ffi::{c_return_enum_improper, c_return_enum_simple}; - use ffi::{EnumImproper, EnumSimple}; + use ffi::{c_return_enum_improper, c_return_enum_simple, c_return_enum_with_lifetime}; + use ffi::{EnumImproper, EnumSimple, EnumWithLifeTime}; assert!(matches!( c_return_enum_simple(true), @@ -409,4 +409,12 @@ fn test_data_enums() { assert!(false); } } + + let a = 0xdead; + match c_return_enum_with_lifetime(&a) { + EnumWithLifeTime::AVal(_) => assert!(false), + EnumWithLifeTime::BVal(&v) => { + assert_eq!(a, v); + } + } } From 547d95aad4324cddaeb9df16e34096829f2ca9d3 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Fri, 29 Dec 2023 15:24:03 +0100 Subject: [PATCH 24/37] revert changes --- syntax/types.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/syntax/types.rs b/syntax/types.rs index bfd2d7e74..f2eafe620 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -71,7 +71,11 @@ impl<'a> Types<'a> { Api::Include(_) => {} Api::Struct(strct) => { let ident = &strct.name.rust; - if !type_names.insert(ident) && !cxx.contains(ident) { + if !type_names.insert(ident) + && (!cxx.contains(ident) + || structs.contains_key(ident) + || enums.contains_key(ident)) + { // If already declared as a struct or enum, or if // colliding with something other than an extern C++ // type, then error. @@ -92,7 +96,11 @@ impl<'a> Types<'a> { EnumRepr::Foreign { rust_type: _ } => {} } let ident = &enm.name.rust; - if !type_names.insert(ident) && !cxx.contains(ident) { + if !type_names.insert(ident) + && (!cxx.contains(ident) + || structs.contains_key(ident) + || enums.contains_key(ident)) + { // If already declared as a struct or enum, or if // colliding with something other than an extern C++ // type, then error. @@ -108,7 +116,11 @@ impl<'a> Types<'a> { } Api::EnumUnnamed(enm) => { let ident = &enm.name.rust; - if !type_names.insert(ident) && !cxx.contains(ident) { + if !type_names.insert(ident) + && (!cxx.contains(ident) + || structs.contains_key(ident) + || enums.contains_key(ident)) + { // If already declared as a struct or enum, or if // colliding with something other than an extern C++ // type, then error. @@ -255,7 +267,7 @@ impl<'a> Types<'a> { // If the ty is missing we're dealing with the C-style enum. let ty = match &var.ty { None => return false, - Some(ty) => ty + Some(ty) => ty, }; if match types.determine_improper_ctype(ty) { ImproperCtype::Depends(inner) => { From 98f136f300d0e1e9f6f3d053947ca8abbc2363ae Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Fri, 29 Dec 2023 15:48:19 +0100 Subject: [PATCH 25/37] add std::reference_wrapper check --- src/cxx.cc | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/cxx.cc b/src/cxx.cc index cc326b699..4f4f6835b 100644 --- a/src/cxx.cc +++ b/src/cxx.cc @@ -1,6 +1,7 @@ #include "../include/cxx.h" #include #include +#include #include #include @@ -186,7 +187,7 @@ String String::lossy(const char16_t *s, std::size_t len) noexcept { return String(lossy_t{}, s, len); } -String &String::operator=(const String &other) &noexcept { +String &String::operator=(const String &other) & noexcept { if (this != &other) { cxxbridge1$string$drop(this); cxxbridge1$string$clone(this, other); @@ -194,7 +195,7 @@ String &String::operator=(const String &other) &noexcept { return *this; } -String &String::operator=(String &&other) &noexcept { +String &String::operator=(String &&other) & noexcept { cxxbridge1$string$drop(this); this->repr = other.repr; cxxbridge1$string$new(&other); @@ -641,6 +642,15 @@ static_assert( static_assert( !std::is_constructible_v, int>); +// We're using the std::reference_wrapper if the enum holds a reference. The +// standard however does not guarantee that the size of std::reference_wrapper +// is the same size as a pointer (required to be compatible with Rust's +// references). Therefore we check it at compile time. +// +// [1] https://en.cppreference.com/w/cpp/utility/functional/reference_wrapper +// [2] https://doc.rust-lang.org/std/mem/fn.size_of.html#examples +static_assert(sizeof(std::reference_wrapper) == + sizeof(std::ptrdiff_t)); } // namespace detail #endif @@ -683,7 +693,7 @@ Error &Error::operator=(const Error &other) & { return *this; } -Error &Error::operator=(Error &&other) &noexcept { +Error &Error::operator=(Error &&other) & noexcept { std::exception::operator=(std::move(other)); delete[] this->msg; this->msg = other.msg; From e091567bf6d8072498116fee8e1596b63aa32737 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Sat, 30 Dec 2023 12:34:01 +0100 Subject: [PATCH 26/37] add derives --- macro/src/derive.rs | 61 ++++++++++++++++++++++----------------------- macro/src/expand.rs | 9 +++---- tests/ffi/lib.rs | 1 + 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/macro/src/derive.rs b/macro/src/derive.rs index 6d33e81fc..ad4132d18 100644 --- a/macro/src/derive.rs +++ b/macro/src/derive.rs @@ -97,37 +97,36 @@ pub(crate) fn expand_enum(enm: &Enum, actual_derives: &mut Option) expanded } -// pub(crate) fn expand_enum_unnamed(enm: &Enum, actual_derives: &mut Option) -> TokenStream { -// let mut expanded = TokenStream::new(); -// let mut traits = Vec::new(); - -// for derive in &enm.derives { -// let span = derive.span; -// match derive.what { -// // The traits below will always be implemented. This is consistent -// // with the logic for c-like enums. -// Trait::Copy | Trait::Clone | Trait::Eq | Trait::PartialEq => {} -// Trait::Debug => expanded.extend(enum_debug(enm, span)), -// Trait::Default => unreachable!(), -// Trait::ExternType => unreachable!(), -// Trait::Hash => traits.push(quote_spanned!(span=> ::cxx::core::hash::Hash)), -// Trait::Ord => expanded.extend(enum_ord(enm, span)), -// Trait::PartialOrd => expanded.extend(enum_partial_ord(enm, span)), -// Trait::Serialize => traits.push(quote_spanned!(span=> ::serde::Serialize)), -// Trait::Deserialize => traits.push(quote_spanned!(span=> ::serde::Deserialize)), -// } -// } - -// let span = enm.name.rust.span(); -// expanded.extend(enum_copy(enm, span)); -// expanded.extend(enum_clone(enm, span)); -// traits.push(quote_spanned!(span=> ::cxx::core::cmp::Eq)); -// traits.push(quote_spanned!(span=> ::cxx::core::cmp::PartialEq)); - -// *actual_derives = Some(quote!(#[derive(#(#traits),*)])); - -// expanded -// } +pub(crate) fn expand_enum_unnamed( + enm: &Enum, + actual_derives: &mut Option, +) -> TokenStream { + let mut traits = Vec::new(); + + // I don't get it why we're using everywhere self written derives. For now + // I just use the build ins until someone complains. + for derive in &enm.derives { + let span = derive.span; + match derive.what { + Trait::Copy => traits.push(quote_spanned!(span=> Copy)), + Trait::Clone => traits.push(quote_spanned!(span=> Clone)), + Trait::Eq => traits.push(quote_spanned!(span=> Eq)), + Trait::PartialEq => traits.push(quote_spanned!(span=> PartialEq)), + Trait::Debug => traits.push(quote_spanned!(span=> Debug)), + Trait::Default => unreachable!(), + Trait::ExternType => unreachable!(), + Trait::Hash => traits.push(quote_spanned!(span=> ::cxx::core::hash::Hash)), + Trait::Ord => traits.push(quote_spanned!(span=> Ord)), + Trait::PartialOrd => traits.push(quote_spanned!(span=> PartialOrd)), + Trait::Serialize => traits.push(quote_spanned!(span=> ::serde::Serialize)), + Trait::Deserialize => traits.push(quote_spanned!(span=> ::serde::Deserialize)), + } + } + + *actual_derives = Some(quote!(#[derive(#(#traits),*)])); + + TokenStream::new() +} fn struct_copy(strct: &Struct, span: Span) -> TokenStream { let ident = &strct.name.rust; diff --git a/macro/src/expand.rs b/macro/src/expand.rs index eb5318653..f1032a386 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -337,9 +337,8 @@ fn expand_enum_unnamed(enm: &Enum) -> TokenStream { // https://doc.rust-lang.org/reference/items/enumerations.html#variant-visibility quote!(#doc #attrs #variant(#vis #ty)) }); - // TODO I don't understand the custom derives. - // let mut derives = None; - // let derived_traits = derive::expand_struct(strct, &mut derives); + let mut derives = None; + let derived_traits = derive::expand_enum_unnamed(enm, &mut derives); let span = ident.span(); let visibility = enm.visibility; @@ -353,7 +352,7 @@ fn expand_enum_unnamed(enm: &Enum) -> TokenStream { quote! { #doc - // #derives + #derives #attrs #[repr(C)] #enum_def @@ -365,7 +364,7 @@ fn expand_enum_unnamed(enm: &Enum) -> TokenStream { type Kind = ::cxx::kind::Trivial; } - // #derived_traits + #derived_traits } } diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index 44e04ecdc..3a3a8e1a5 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -92,6 +92,7 @@ pub mod ffi { s: &'a str, } + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] enum EnumSimple { AVal(bool), BVal(Shared), From 2266e36842681f306ef1d2ef9ec76c66d494df51 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Sat, 30 Dec 2023 13:49:26 +0100 Subject: [PATCH 27/37] fix warning --- include/cxx.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/cxx.h b/include/cxx.h index ab4534a09..a86286dc7 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -710,7 +710,7 @@ struct attempt { void destroy() { visit( - [this](const auto &value) { + [](const auto &value) { using type = std::decay_t; value.~type(); }, From 564281e3955a5c69c74714f444dd20ad63d1b82e Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Sun, 31 Dec 2023 13:21:57 +0100 Subject: [PATCH 28/37] fix get --- include/cxx.h | 90 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/include/cxx.h b/include/cxx.h index a86286dc7..36167bbe7 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -381,21 +381,28 @@ class Vec final { #if __cplusplus >= 201703L -template -struct type_from_index_impl { - using type = Type; -}; - +// Adjusted from std::variant_alternative. Standard selects always the most +// specialized template specialization. See +// https://timsong-cpp.github.io/cppwp/n4140/temp.class.spec.match and +// https://timsong-cpp.github.io/cppwp/n4140/temp.class.order. template -struct type_from_index; +struct variant_alternative; +// Specialization for gracefully handling invalid indices. template -struct type_from_index {}; +struct variant_alternative {}; template -struct type_from_index - : std::conditional_t, - type_from_index> {}; +struct variant_alternative + : variant_alternative {}; + +template +struct variant_alternative<0, First, Remainder...> { + using type = First; +}; + +template +using variant_alternative_t = typename variant_alternative::type; template constexpr size_t compile_time_count() noexcept { @@ -410,20 +417,23 @@ template struct exactly_once : std::conditional_t::value == 1, std::true_type, std::false_type> {}; -template -struct index_from_type_impl; +template +struct index_from_booleans; + +template +struct index_from_booleans {}; -template -struct index_from_type_impl {}; +template +struct index_from_booleans + : index_from_booleans {}; -template -struct index_from_type_impl - : std::conditional_t, - index_from_type_impl> {}; +template +struct index_from_booleans + : std::integral_constant {}; template struct index_from_type - : index_from_type_impl<0, std::is_same_v, Ts>...> { + : index_from_booleans<0, std::is_same_v, Ts>...> { static_assert(exactly_once, Ts>...>::value, "Index must be unique"); }; @@ -435,10 +445,10 @@ template struct attempt; template -constexpr decltype(auto) get(attempt &); +constexpr variant_alternative_t &get(attempt &); template -constexpr decltype(auto) get(const attempt &); +constexpr const variant_alternative_t &get(const attempt &); template constexpr decltype(auto) visit(Visitor &&visitor, attempt &); @@ -509,7 +519,7 @@ struct attempt { std::forward(args)...} {} template - using type_from_index_t = typename type_from_index::type; + using type_from_index_t = variant_alternative_t; /// @brief Participates in the resolution only if the index is within range /// and if the type can be constructor from Args. Corresponds to (7) of @@ -686,19 +696,6 @@ struct attempt { : std::runtime_error{"The index should be " + std::to_string(index)} {} }; -public: - template - friend constexpr decltype(auto) get(attempt &variant) { - variant.throw_if_invalid(); - return *reinterpret_cast *>(variant.t_buff); - } - - template - friend constexpr decltype(auto) get(const attempt &variant) { - variant.throw_if_invalid(); - return *reinterpret_cast *>(variant.t_buff); - } - private: template void throw_if_invalid() const { @@ -722,6 +719,15 @@ struct attempt { // https://stackoverflow.com/questions/71828288/why-is-stdaligned-storage-to-be-deprecated-in-c23-and-what-to-use-instead alignas(Ts...) std::byte t_buff[std::max({sizeof(Ts)...})]; + // The friend zone + template + friend constexpr variant_alternative_t & + get(attempt &variant); + + template + friend constexpr const variant_alternative_t & + get(const attempt &variant); + template friend struct visitor_type; }; @@ -780,6 +786,20 @@ constexpr decltype(auto) visit(Visitor &&visitor, return visitor_type::visit(std::forward(visitor), variant); } +template +constexpr variant_alternative_t &get(attempt &variant) { + variant.template throw_if_invalid(); + return *reinterpret_cast *>(variant.t_buff); +} + +template +constexpr const variant_alternative_t & +get(const attempt &variant) { + variant.template throw_if_invalid(); + return *reinterpret_cast *>( + variant.t_buff); +} + template >...>::value>> From 85f9f85f4a4571a07de8cca2d46cb3cad998bc42 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Sun, 31 Dec 2023 13:34:35 +0100 Subject: [PATCH 29/37] simplify get --- include/cxx.h | 15 ++++++--------- src/cxx.cc | 12 ++++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/include/cxx.h b/include/cxx.h index 36167bbe7..2cc33b13e 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -445,10 +445,10 @@ template struct attempt; template -constexpr variant_alternative_t &get(attempt &); +constexpr decltype(auto) get(attempt &); template -constexpr const variant_alternative_t &get(const attempt &); +constexpr decltype(auto) get(const attempt &); template constexpr decltype(auto) visit(Visitor &&visitor, attempt &); @@ -721,12 +721,10 @@ struct attempt { // The friend zone template - friend constexpr variant_alternative_t & - get(attempt &variant); + friend constexpr decltype(auto) get(attempt &variant); template - friend constexpr const variant_alternative_t & - get(const attempt &variant); + friend constexpr decltype(auto) get(const attempt &variant); template friend struct visitor_type; @@ -787,14 +785,13 @@ constexpr decltype(auto) visit(Visitor &&visitor, } template -constexpr variant_alternative_t &get(attempt &variant) { +constexpr decltype(auto) get(attempt &variant) { variant.template throw_if_invalid(); return *reinterpret_cast *>(variant.t_buff); } template -constexpr const variant_alternative_t & -get(const attempt &variant) { +constexpr decltype(auto) get(const attempt &variant) { variant.template throw_if_invalid(); return *reinterpret_cast *>( variant.t_buff); diff --git a/src/cxx.cc b/src/cxx.cc index 4f4f6835b..0dc606633 100644 --- a/src/cxx.cc +++ b/src/cxx.cc @@ -652,6 +652,18 @@ static_assert( static_assert(sizeof(std::reference_wrapper) == sizeof(std::ptrdiff_t)); + +// Check that getting something works and actually returns the same type. +template +using copy_variant_alternative_t = + variant_alternative_t; + +static_assert(std::is_same_v(std::declval())), + const copy_variant_alternative_t<0> &>); + +static_assert(std::is_same_v(std::declval())), + const copy_variant_alternative_t<1> &>); + } // namespace detail #endif From 73603b186e9d2de67b10055230ae3809e820f5fa Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Sun, 31 Dec 2023 14:22:53 +0100 Subject: [PATCH 30/37] add empty support --- gen/src/write.rs | 32 +++++++++++++++++++------------- include/cxx.h | 7 ++----- macro/src/expand.rs | 29 ++++++++++++++++++----------- src/cxx.cc | 2 ++ syntax/check.rs | 3 +++ syntax/improper.rs | 2 +- syntax/parse.rs | 31 +++++++++++++++---------------- syntax/trivial.rs | 1 + syntax/types.rs | 4 +++- tests/ffi/lib.rs | 3 ++- tests/ffi/tests.cc | 8 +++++--- tests/ffi/tests.h | 2 +- tests/test.rs | 9 ++++----- 13 files changed, 76 insertions(+), 57 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 1d7d83fd6..71a6ad1e1 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -352,20 +352,26 @@ fn write_enum_unnamed(out: &mut OutFile, enm: &Enum) { write!(out, "::rust::variant<"); let mut iter = enm.variants.iter().peekable(); while let Some(value) = iter.next() { - let ty = value.ty.as_ref().unwrap(); - match ty { - Type::Ref(r) => { - // References are not allowed in variants, so we wrap them - // into std::reference_wrapper. - write!(out, "std::reference_wrapper<"); - write_type_space(out, &r.inner); - if !r.mutable { - write!(out, "const "); + match &value.ty { + None => { + write!(out, "::rust::empty"); + } + Some(ty) => { + match ty { + Type::Ref(r) => { + // References are not allowed in variants, so we wrap them + // into std::reference_wrapper. + write!(out, "std::reference_wrapper<"); + write_type_space(out, &r.inner); + if !r.mutable { + write!(out, "const "); + } + write!(out, ">"); + } + _ => write_type(out, ty), } - write!(out, ">"); - }, - _ => write_type(out, ty), - } + } + }; if iter.peek().is_some() { write!(out, ", "); } diff --git a/include/cxx.h b/include/cxx.h index 2cc33b13e..fe467c457 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -850,11 +850,8 @@ struct variant : public attempt, private allow_copy { using base::operator=; }; -template -constexpr decltype(auto) visit(Visitor &&visitor, attempt &); - -template -constexpr decltype(auto) visit(Visitor &&visitor, const attempt &); +/// An empty type used for unit variants from Rust. +struct empty {}; #endif diff --git a/macro/src/expand.rs b/macro/src/expand.rs index f1032a386..a0f39fa94 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -7,8 +7,8 @@ use crate::syntax::qualified::QualifiedName; use crate::syntax::report::Errors; use crate::syntax::symbol::Symbol; use crate::syntax::{ - self, check, mangle, Api, Doc, Enum, ExternFn, ExternType, Impl, Lifetimes, Pair, - Signature, Struct, Trait, Type, TypeAlias, Types, CEnumOpts, + self, check, mangle, Api, CEnumOpts, Doc, Enum, ExternFn, ExternType, Impl, Lifetimes, Pair, + Signature, Struct, Trait, Type, TypeAlias, Types, }; use crate::type_id::Crate; use crate::{derive, generics}; @@ -328,14 +328,21 @@ fn expand_enum_unnamed(enm: &Enum) -> TokenStream { let variants = enm.variants.iter().map(|variant| { let doc = &variant.doc; let attrs = &variant.attrs; - let ty = &variant.ty.as_ref().unwrap(); - let vis = &variant.vis; - let variant = &variant.name.rust; - // We add the visibility if defined here and let it crash since the doc - // says that the syntax allows visibility specifiers but they should be - // rejected. See - // https://doc.rust-lang.org/reference/items/enumerations.html#variant-visibility - quote!(#doc #attrs #variant(#vis #ty)) + let ident = &variant.name.rust; + + match &variant.ty { + None => { + quote!(#doc #attrs #ident) + } + Some(ty) => { + let vis = &variant.vis; + // We add the visibility if defined here and let it crash since the doc + // says that the syntax allows visibility specifiers but they should be + // rejected. See + // https://doc.rust-lang.org/reference/items/enumerations.html#variant-visibility + quote!(#doc #attrs #ident(#vis #ty)) + } + } }); let mut derives = None; let derived_traits = derive::expand_enum_unnamed(enm, &mut derives); @@ -368,7 +375,7 @@ fn expand_enum_unnamed(enm: &Enum) -> TokenStream { } } -fn expand_enum(enm: &Enum, opts:& CEnumOpts) -> TokenStream { +fn expand_enum(enm: &Enum, opts: &CEnumOpts) -> TokenStream { let ident = &enm.name.rust; let doc = &enm.doc; let attrs = &enm.attrs; diff --git a/src/cxx.cc b/src/cxx.cc index 0dc606633..5291b828f 100644 --- a/src/cxx.cc +++ b/src/cxx.cc @@ -664,6 +664,8 @@ static_assert(std::is_same_v(std::declval())), static_assert(std::is_same_v(std::declval())), const copy_variant_alternative_t<1> &>); +static_assert(sizeof(empty) == sizeof(std::uint8_t)); + } // namespace detail #endif diff --git a/syntax/check.rs b/syntax/check.rs index 7a89af8e8..5e7677f39 100644 --- a/syntax/check.rs +++ b/syntax/check.rs @@ -395,6 +395,9 @@ fn check_api_enum_unnamed(cx: &mut Check, enm: &Enum) { } for variant in &enm.variants { + if variant.ty.is_none() { + continue; + } let ty = variant.ty.as_ref().unwrap(); if let Type::Fn(_) = ty { cx.error( diff --git a/syntax/improper.rs b/syntax/improper.rs index bd2a8f336..dffeaf892 100644 --- a/syntax/improper.rs +++ b/syntax/improper.rs @@ -19,7 +19,7 @@ impl<'a> Types<'a> { } else if let Some(strct) = self.structs.get(ident) { Depends(&strct.name.rust) // iterate to fixed-point } else if let Some(enm) = self.enums.get(ident) { - if enm.variants.iter().all(|variant| variant.ty.is_some()) { + if enm.variants.iter().any(|variant| variant.ty.is_some()) { Depends(&enm.name.rust) // iterate to fixed-point } else { Definite(self.rust.contains(ident) || self.aliases.contains_key(ident)) diff --git a/syntax/parse.rs b/syntax/parse.rs index 72b1a5de0..175ff1528 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -242,15 +242,13 @@ fn parse_enum_unnamed(cx: &mut Errors, mut item: ItemEnum, namespace: &Namespace } // Get the unnamed field of the variant. - let field = match variant.fields { - // TODO(ddo) This is consistent with the struct but it makes the - // feature way less useful. - Fields::Unit => { - return Err(Error::new_spanned(item, "Unit variants are not supported")) - } + let (ty, vis) = match variant.fields { Fields::Named(_) => { return Err(Error::new_spanned(item, "Named variants are not supported")) } + Fields::Unit => { + (None, Visibility::Inherited) + } Fields::Unnamed(ref unnamed_variant) => { // Having move than one unnamed field is also illegal since we can't // represent it in c++. @@ -260,17 +258,18 @@ fn parse_enum_unnamed(cx: &mut Errors, mut item: ItemEnum, namespace: &Namespace "More than one unnamed field is not supported", )); } - unnamed_variant.unnamed.first().unwrap() + let field = unnamed_variant.unnamed.first().unwrap(); + let ty = match parse_type(&field.ty) { + Ok(ty) => ty, + Err(err) => { + cx.push(err); + continue; + } + }; + (Some(ty), field.vis.clone()) } }; - let ty = match parse_type(&field.ty) { - Ok(ty) => ty, - Err(err) => { - cx.push(err); - continue; - } - }; let mut cfg = CfgExpr::Unconditional; let mut doc = Doc::new(); @@ -292,10 +291,10 @@ fn parse_enum_unnamed(cx: &mut Errors, mut item: ItemEnum, namespace: &Namespace doc, attrs: variant_attrs, name, - vis: field.vis.clone(), + vis, discriminant: Discriminant::zero(), expr: None, - ty: Some(ty), + ty, }); } diff --git a/syntax/trivial.rs b/syntax/trivial.rs index 953340055..bd6ad3881 100644 --- a/syntax/trivial.rs +++ b/syntax/trivial.rs @@ -38,6 +38,7 @@ pub(crate) fn required_trivial_reasons<'a>( for api in apis { match api { + // TODO Add here the enums Api::Struct(strct) => { for field in &strct.fields { if let Type::Ident(ident) = &field.ty { diff --git a/syntax/types.rs b/syntax/types.rs index f2eafe620..d8315427d 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -129,7 +129,9 @@ impl<'a> Types<'a> { enums.insert(ident, enm); for variant in &enm.variants { - visit(&mut all, variant.ty.as_ref().unwrap()); + if variant.ty.is_some() { + visit(&mut all, variant.ty.as_ref().unwrap()); + } } add_resolution(&enm.name, &enm.generics); } diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index 3a3a8e1a5..1cf5358d9 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -96,6 +96,7 @@ pub mod ffi { enum EnumSimple { AVal(bool), BVal(Shared), + CVal, } enum EnumImproper { @@ -147,7 +148,7 @@ pub mod ffi { fn c_return_nested_ns_enum(n: u16) -> ABEnum; fn c_return_const_ptr(n: usize) -> *const C; fn c_return_mut_ptr(n: usize) -> *mut C; - fn c_return_enum_simple(first: bool) -> EnumSimple; + fn c_return_enum_simple(value: i32) -> EnumSimple; fn c_return_enum_improper(first: bool) -> EnumImproper; fn c_return_enum_with_lifetime<'a>(val: &'a i32) -> EnumWithLifeTime<'a>; diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc index 1b34792dc..2bb319228 100644 --- a/tests/ffi/tests.cc +++ b/tests/ffi/tests.cc @@ -229,10 +229,12 @@ std::unique_ptr c_return_borrow(const std::string &s) { return std::unique_ptr(new Borrow(s)); } -EnumSimple c_return_enum_simple(bool first) { - if (first) +EnumSimple c_return_enum_simple(int value) { + if (value == 0) return false; - return Shared{123}; + else if (value == 1) + return Shared{123}; + return rust::empty{}; } EnumImproper c_return_enum_improper(bool first) { diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h index e4b464aef..462058b71 100644 --- a/tests/ffi/tests.h +++ b/tests/ffi/tests.h @@ -128,7 +128,7 @@ ::A::B::ABEnum c_return_nested_ns_enum(uint16_t n); std::unique_ptr c_return_borrow(const std::string &s); const C *c_return_const_ptr(size_t n); C *c_return_mut_ptr(size_t n); -EnumSimple c_return_enum_simple(bool first); +EnumSimple c_return_enum_simple(int value); EnumImproper c_return_enum_improper(bool first); EnumWithLifeTime c_return_enum_with_lifetime(const int& val); diff --git a/tests/test.rs b/tests/test.rs index 930f90852..a3bccecbd 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -385,16 +385,15 @@ fn test_data_enums() { use ffi::{c_return_enum_improper, c_return_enum_simple, c_return_enum_with_lifetime}; use ffi::{EnumImproper, EnumSimple, EnumWithLifeTime}; - assert!(matches!( - c_return_enum_simple(true), - EnumSimple::AVal(false) - )); + assert!(matches!(c_return_enum_simple(0), EnumSimple::AVal(false))); assert!(matches!( - c_return_enum_simple(false), + c_return_enum_simple(1), EnumSimple::BVal(ffi::Shared { z: 123 }) )); + assert!(matches!(c_return_enum_simple(2), EnumSimple::CVal)); + assert!(matches!( c_return_enum_improper(true), EnumImproper::AVal(2) From e450e453e116e1e40890c071f1754328997869d9 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Tue, 2 Jan 2024 17:21:28 +0100 Subject: [PATCH 31/37] doc --- include/cxx.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/cxx.h b/include/cxx.h index fe467c457..d342c5e9e 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -716,7 +716,8 @@ struct attempt { // Until c++23 enums are represented as ints. int m_Type; - // https://stackoverflow.com/questions/71828288/why-is-stdaligned-storage-to-be-deprecated-in-c23-and-what-to-use-instead + // std::aligned_storage is deprecated and may be replaced with the construct + // below. See https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1413r3.pdf alignas(Ts...) std::byte t_buff[std::max({sizeof(Ts)...})]; // The friend zone From 470004cff7b72cb881cd3a0bd40835a647aa69f4 Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Tue, 2 Jan 2024 18:14:57 +0100 Subject: [PATCH 32/37] Extend tests --- gen/src/write.rs | 3 ++- tests/ffi/lib.rs | 3 +++ tests/ffi/tests.cc | 20 ++++++++++++++++++++ tests/ffi/tests.h | 3 +++ tests/test.rs | 23 +++++++++++++++++++++++ 5 files changed, 51 insertions(+), 1 deletion(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 71a6ad1e1..7c733e82a 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -892,7 +892,8 @@ fn write_cxx_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) { write!(out, "(::rust::unsafe_bitcopy, *{})", arg.name.cxx); } else if out.types.needs_indirect_abi(&arg.ty) { out.include.utility = true; - write!(out, "::std::move(*{})", arg.name.cxx); + // TODO -how do I get with unmovable types? + write!(out, "*{}", arg.name.cxx); } else { write!(out, "{}", arg.name.cxx); } diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index 1cf5358d9..a99d11c45 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -199,6 +199,9 @@ pub mod ffi { fn c_take_rust_vec_nested_ns_shared(v: Vec); unsafe fn c_take_const_ptr(c: *const C) -> usize; unsafe fn c_take_mut_ptr(c: *mut C) -> usize; + fn c_take_enum_simple(enm: EnumSimple) -> i32; + fn c_take_enum_improper(enm: EnumImproper) -> i32; + fn c_take_enum_with_lifetime<'a>(enm: &'a EnumWithLifeTime<'a>) -> i32; fn c_try_return_void() -> Result<()>; fn c_try_return_primitive() -> Result; diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc index 2bb319228..825e508af 100644 --- a/tests/ffi/tests.cc +++ b/tests/ffi/tests.cc @@ -576,6 +576,26 @@ size_t c_take_mut_ptr(C *c) { return result; } +int _visit(bool v) { return static_cast(v); }; +int _visit(const Shared &v) { return v.z; }; +int _visit(const ::rust::empty &) { return -1; }; +int _visit(rust::Str value) { return value.size(); } + +int c_take_enum_simple(EnumSimple enm) { + return ::rust::visit([](const auto &val) { return _visit(val); }, enm); +} + +int c_take_enum_improper(EnumImproper enm) { return enm.index(); } + +int c_take_enum_with_lifetime(const EnumWithLifeTime &enm) { + try { + return ::rust::get>(enm) + + ::rust::get<1>(enm); + } catch (...) { + return ::rust::get<0>(enm).size(); + } +} + void c_try_return_void() {} size_t c_try_return_primitive() { return 2020; } diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h index 462058b71..0d75078a7 100644 --- a/tests/ffi/tests.h +++ b/tests/ffi/tests.h @@ -180,6 +180,9 @@ void c_take_ns_enum(::A::AEnum e); void c_take_nested_ns_enum(::A::B::ABEnum e); size_t c_take_const_ptr(const C *c); size_t c_take_mut_ptr(C *c); +int c_take_enum_simple(EnumSimple enm); +int c_take_enum_improper(EnumImproper enm); +int c_take_enum_with_lifetime(const EnumWithLifeTime& enm); void c_try_return_void(); size_t c_try_return_primitive(); diff --git a/tests/test.rs b/tests/test.rs index a3bccecbd..ff43b32a8 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -383,6 +383,7 @@ fn test_raw_ptr() { #[test] fn test_data_enums() { use ffi::{c_return_enum_improper, c_return_enum_simple, c_return_enum_with_lifetime}; + use ffi::{c_take_enum_improper, c_take_enum_simple, c_take_enum_with_lifetime}; use ffi::{EnumImproper, EnumSimple, EnumWithLifeTime}; assert!(matches!(c_return_enum_simple(0), EnumSimple::AVal(false))); @@ -416,4 +417,26 @@ fn test_data_enums() { assert_eq!(a, v); } } + + assert_eq!(c_take_enum_simple(EnumSimple::AVal(false)), 0); + assert_eq!(c_take_enum_simple(EnumSimple::AVal(true)), 1); + assert_eq!( + c_take_enum_simple(EnumSimple::BVal(ffi::Shared { z: 100 })), + 100 + ); + assert_eq!(c_take_enum_simple(EnumSimple::CVal), -1); + + assert_eq!(c_take_enum_improper(EnumImproper::AVal(1)), 0); + assert_eq!( + c_take_enum_improper(EnumImproper::BVal(ffi::SharedString { + msg: "foo".to_string() + })), + 1 + ); + + let a = EnumWithLifeTime::BVal(&10); + assert_eq!(c_take_enum_with_lifetime(&a), 20); + + let a = EnumWithLifeTime::AVal("foo"); + assert_eq!(c_take_enum_with_lifetime(&a), 3); } From fc1a76d756bed7bf8ca79dfcecce32bb074f9c1f Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Tue, 2 Jan 2024 23:33:43 +0100 Subject: [PATCH 33/37] add doc --- gen/src/write.rs | 2 +- include/cxx.h | 111 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 76 insertions(+), 37 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 7c733e82a..0dccbff0b 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -892,7 +892,7 @@ fn write_cxx_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) { write!(out, "(::rust::unsafe_bitcopy, *{})", arg.name.cxx); } else if out.types.needs_indirect_abi(&arg.ty) { out.include.utility = true; - // TODO -how do I get with unmovable types? + // Let the compiler resolve if this is a copy or a move constructor. write!(out, "*{}", arg.name.cxx); } else { write!(out, "{}", arg.name.cxx); diff --git a/include/cxx.h b/include/cxx.h index d342c5e9e..ad33d3599 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -442,36 +442,41 @@ template struct visitor_type; template -struct attempt; +struct variant_base; template -constexpr decltype(auto) get(attempt &); +constexpr decltype(auto) get(variant_base &); template -constexpr decltype(auto) get(const attempt &); +constexpr decltype(auto) get(const variant_base &); template -constexpr decltype(auto) visit(Visitor &&visitor, attempt &); +constexpr decltype(auto) visit(Visitor &&visitor, variant_base &); template -constexpr decltype(auto) visit(Visitor &&visitor, const attempt &); +constexpr decltype(auto) visit(Visitor &&visitor, const variant_base &); +/// @brief A std::variant like tagged union with the same memory layout as a +/// Rust Enum. +/// +/// The memory layout of the Rust enum is defined under +/// https://doc.rust-lang.org/reference/type-layout.html#reprc-enums-with-fields template -struct attempt { +struct variant_base { static_assert(sizeof...(Ts) > 0, - "attempt must hold at least one alternative"); + "variant_base must hold at least one alternative"); /// @brief Delete the default constructor since we cannot be in an /// uninitialized state (if the first alternative throws). Corresponds to the /// (1) constructor in std::variant. - attempt() = delete; + variant_base() = delete; constexpr static bool all_copy_constructible_v = std::conjunction_v...>; /// @brief Copy constructor. Participates only in the resolution if all types /// are copy constructable. Corresponds to (2) constructor of std::variant. - attempt(const attempt &other) { + variant_base(const variant_base &other) { static_assert( all_copy_constructible_v, "Copy constructor requires that all types are copy constructable"); @@ -488,7 +493,7 @@ struct attempt { /// @brief Delete the move constructor since if we move this container it's /// unclear in which state it is. Corresponds to (3) constructor of /// std::variant. - attempt(attempt &&other) = delete; + variant_base(variant_base &&other) = delete; template constexpr static bool is_unique_v = @@ -503,7 +508,7 @@ struct attempt { template , typename = std::enable_if_t && std::is_constructible_v>> - attempt(T &&other) noexcept(std::is_nothrow_constructible_v) { + variant_base(T &&other) noexcept(std::is_nothrow_constructible_v) { m_Type = index_from_type_v; new (static_cast(t_buff)) D(std::forward(other)); } @@ -513,10 +518,10 @@ struct attempt { template >, typename = std::enable_if_t>> - explicit attempt(std::in_place_type_t type, Args &&...args) noexcept( + explicit variant_base(std::in_place_type_t type, Args &&...args) noexcept( std::is_nothrow_constructible_v) - : attempt{std::in_place_index>, - std::forward(args)...} {} + : variant_base{std::in_place_index>, + std::forward(args)...} {} template using type_from_index_t = variant_alternative_t; @@ -526,8 +531,9 @@ struct attempt { /// std::variant. template , typename = std::enable_if_t>> - explicit attempt(std::in_place_index_t index, Args &&...args) noexcept( - std::is_nothrow_constructible_v) { + explicit variant_base( + std::in_place_index_t index, + Args &&...args) noexcept(std::is_nothrow_constructible_v) { m_Type = I; new (static_cast(t_buff)) T(std::forward(args)...); } @@ -540,7 +546,7 @@ struct attempt { /// the resolution if all types in Ts are copy constructable. template && all_copy_constructible_v>> - attempt(const std::variant &other) { + variant_base(const std::variant &other) { m_Type = other.index(); std::visit( [this](const auto &value) { @@ -557,7 +563,7 @@ struct attempt { /// the resolution if all types in Ts are move constructable. template && all_move_constructible_v>> - attempt(std::variant &&other) { + variant_base(std::variant &&other) { m_Type = other.index(); std::visit( [this](auto &&value) { @@ -567,11 +573,11 @@ struct attempt { other); } - ~attempt() { destroy(); } + ~variant_base() { destroy(); } /// @brief Copy assignment. Staticly fails if not every type in Ts is copy /// constructable. Corresponds to (1) assignment of std::variant. - attempt &operator=(const attempt &other) { + variant_base &operator=(const variant_base &other) { static_assert( all_copy_constructible_v, "Copy assignment requires that all types are copy constructable"); @@ -583,13 +589,13 @@ struct attempt { /// @brief Deleted move assignment. Same as for the move constructor. /// Would correspond to (2) assignment of std::variant. - attempt &operator=(attempt &&other) = delete; + variant_base &operator=(variant_base &&other) = delete; /// @brief Converting assignment. Corresponds to (3) assignment of /// std::variant. template && std::is_constructible_v>> - attempt &operator=(T &&other) { + variant_base &operator=(T &&other) { constexpr auto index = index_from_type_v; if (m_Type == index) { @@ -606,7 +612,7 @@ struct attempt { /// resolution if all types in Ts are copy constructable. template && all_copy_constructible_v>> - attempt &operator=(const std::variant &other) { + variant_base &operator=(const std::variant &other) { // TODO this is not really clean since we fail if std::variant has // duplicated types. std::visit( @@ -622,7 +628,7 @@ struct attempt { /// resolution if all types in Ts are move constructable. template && all_move_constructible_v>> - attempt &operator=(std::variant &&other) { + variant_base &operator=(std::variant &&other) { // TODO this is not really clean since we fail if std::variant has // duplicated types. std::visit( @@ -649,6 +655,37 @@ struct attempt { /// @brief Emplace function. Participates in the resolution only if T can be /// constructed from Args. Offers strong exception guarantee. Corresponds to /// the (2) emplace function of std::variant. + /// + /// The std::variant can have no valid state if the type throws during the + /// construction. This is represented by the `valueless_by_exception` flag. + /// The same approach is also used in absl::variant [2]. + /// In our case we can't accept valueless enums since we can't represent this + /// in Rust. We must therefore provide a strong exception guarantee for all + /// operations using `emplace`. Two famous implementations of never valueless + /// variants are Boost/variant [3] and Boost/variant2 [4]. Boost/variant2 uses + /// two memory buffers - which would be not compatible with Rust Enum's memory + /// layout. The Boost/variant backs up the old object and calls its d'tor + /// before constructing the new object; It then copies the old data back to + /// the buffer if the construction fails - which might contain garbage (since) + /// the d'tor was already called. + /// + /// + /// We take a similar approach to Boost/variant. Assuming that constructing or + /// moving the new type can throw, we backup the old data, try to construct + /// the new object in the final buffer, swap the buffers, such that the old + /// object is back in its original place, detroy it and move the new object + /// from the old buffer back to the final place. + /// + /// Sources + /// + /// [1] + /// https://en.cppreference.com/w/cpp/utility/variant/valueless_by_exception + /// [2] + /// https://github.com/abseil/abseil-cpp/blob/master/absl/types/variant.h + /// [3] + /// https://www.boost.org/doc/libs/1_84_0/libs/variant2/doc/html/variant2.html + /// [4] + /// https://www.boost.org/doc/libs/1_84_0/doc/html/variant/design.html#variant.design.never-empty template , typename = std::enable_if_t>> T &emplace(Args &&...args) { @@ -687,7 +724,7 @@ struct attempt { } constexpr std::size_t index() const noexcept { return m_Type; } - void swap(attempt &other) { + void swap(variant_base &other) { // TODO } @@ -717,15 +754,16 @@ struct attempt { // Until c++23 enums are represented as ints. int m_Type; // std::aligned_storage is deprecated and may be replaced with the construct - // below. See https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1413r3.pdf + // below. See + // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1413r3.pdf alignas(Ts...) std::byte t_buff[std::max({sizeof(Ts)...})]; // The friend zone template - friend constexpr decltype(auto) get(attempt &variant); + friend constexpr decltype(auto) get(variant_base &variant); template - friend constexpr decltype(auto) get(const attempt &variant); + friend constexpr decltype(auto) get(const variant_base &variant); template friend struct visitor_type; @@ -773,7 +811,8 @@ struct visitor_type { /// @brief Applies the visitor to the variant. Corresponds to the (3) /// std::visit defintion. template -constexpr decltype(auto) visit(Visitor &&visitor, attempt &variant) { +constexpr decltype(auto) visit(Visitor &&visitor, + variant_base &variant) { return visitor_type::visit(std::forward(visitor), variant); } @@ -781,18 +820,18 @@ constexpr decltype(auto) visit(Visitor &&visitor, attempt &variant) { /// std::visit defintion. template constexpr decltype(auto) visit(Visitor &&visitor, - const attempt &variant) { + const variant_base &variant) { return visitor_type::visit(std::forward(visitor), variant); } template -constexpr decltype(auto) get(attempt &variant) { +constexpr decltype(auto) get(variant_base &variant) { variant.template throw_if_invalid(); return *reinterpret_cast *>(variant.t_buff); } template -constexpr decltype(auto) get(const attempt &variant) { +constexpr decltype(auto) get(const variant_base &variant) { variant.template throw_if_invalid(); return *reinterpret_cast *>( variant.t_buff); @@ -801,7 +840,7 @@ constexpr decltype(auto) get(const attempt &variant) { template >...>::value>> -constexpr const T &get(const attempt &variant) { +constexpr const T &get(const variant_base &variant) { constexpr auto index = index_from_type::value; return get(variant); } @@ -809,7 +848,7 @@ constexpr const T &get(const attempt &variant) { template >...>::value>> -constexpr T &get(attempt &variant) { +constexpr T &get(variant_base &variant) { constexpr auto index = index_from_type::value; return get(variant); } @@ -836,8 +875,8 @@ using allow_copy = copy_control...>>; template -struct variant : public attempt, private allow_copy { - using base = attempt; +struct variant : public variant_base, private allow_copy { + using base = variant_base; variant() = delete; variant(const variant &) = default; From 4fa9151bb2ade8366afa2936f5932ea76b3b094f Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Tue, 2 Jan 2024 23:43:13 +0100 Subject: [PATCH 34/37] Add holds_alternative --- demo/src/blobstore.cc | 2 +- include/cxx.h | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/demo/src/blobstore.cc b/demo/src/blobstore.cc index 04920bb2b..90d1e8c0f 100644 --- a/demo/src/blobstore.cc +++ b/demo/src/blobstore.cc @@ -86,7 +86,7 @@ void take_enum(const BlobEnum &enm) { void take_mut_enum(BlobEnum &enm) { take_enum(enm); - if (enm.index() == 0) { + if (!::rust::holds_alternative(enm)) { enm = false; } else { enm = 111; diff --git a/include/cxx.h b/include/cxx.h index ad33d3599..d2bfafd4c 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -853,6 +853,18 @@ constexpr T &get(variant_base &variant) { return get(variant); } +template +constexpr bool holds_alternative(const variant_base &variant) { + return variant.index() == I; +} + +template >...>::value>> +constexpr bool holds_alternative(const variant_base &variant) { + return variant.index() == index_from_type::value; +} + template struct copy_control; From ad9368b4ed911f0c6410f1e393f014f65767f64a Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Tue, 2 Jan 2024 23:50:50 +0100 Subject: [PATCH 35/37] rename internal types --- include/cxx.h | 56 +++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/include/cxx.h b/include/cxx.h index d2bfafd4c..6b9f9b43c 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -481,11 +481,11 @@ struct variant_base { all_copy_constructible_v, "Copy constructor requires that all types are copy constructable"); - m_Type = other.m_Type; + m_Index = other.m_Index; visit( [this](const auto &value) { using type = std::decay_t; - new (static_cast(t_buff)) type(value); + new (static_cast(m_Buff)) type(value); }, other); }; @@ -509,8 +509,8 @@ struct variant_base { typename = std::enable_if_t && std::is_constructible_v>> variant_base(T &&other) noexcept(std::is_nothrow_constructible_v) { - m_Type = index_from_type_v; - new (static_cast(t_buff)) D(std::forward(other)); + m_Index = index_from_type_v; + new (static_cast(m_Buff)) D(std::forward(other)); } /// @brief Participates in the resolution only if we can construct T from Args @@ -534,8 +534,8 @@ struct variant_base { explicit variant_base( std::in_place_index_t index, Args &&...args) noexcept(std::is_nothrow_constructible_v) { - m_Type = I; - new (static_cast(t_buff)) T(std::forward(args)...); + m_Index = I; + new (static_cast(m_Buff)) T(std::forward(args)...); } template @@ -547,11 +547,11 @@ struct variant_base { template && all_copy_constructible_v>> variant_base(const std::variant &other) { - m_Type = other.index(); + m_Index = other.index(); std::visit( [this](const auto &value) { using type = std::decay_t; - new (static_cast(t_buff)) type(value); + new (static_cast(m_Buff)) type(value); }, other); } @@ -564,11 +564,11 @@ struct variant_base { template && all_move_constructible_v>> variant_base(std::variant &&other) { - m_Type = other.index(); + m_Index = other.index(); std::visit( [this](auto &&value) { using type = std::decay_t; - new (static_cast(t_buff)) type(std::move(value)); + new (static_cast(m_Buff)) type(std::move(value)); }, other); } @@ -598,7 +598,7 @@ struct variant_base { variant_base &operator=(T &&other) { constexpr auto index = index_from_type_v; - if (m_Type == index) { + if (m_Index == index) { if constexpr (std::is_nothrow_assignable_v) { get(*this) = std::forward(other); return *this; @@ -691,39 +691,39 @@ struct variant_base { T &emplace(Args &&...args) { if constexpr (std::is_nothrow_constructible_v) { destroy(); - new (static_cast(t_buff)) T(std::forward(args)...); + new (static_cast(m_Buff)) T(std::forward(args)...); } else if constexpr (std::is_nothrow_move_constructible_v) { // This operation may throw, but we know that the move does not. const T tmp{std::forward(args)...}; // The operations below are save. destroy(); - new (static_cast(t_buff)) T(std::move(tmp)); + new (static_cast(m_Buff)) T(std::move(tmp)); } else { // Backup the old data. alignas(Ts...) std::byte old_buff[std::max({sizeof(Ts)...})]; - std::memcpy(old_buff, t_buff, sizeof(t_buff)); + std::memcpy(old_buff, m_Buff, sizeof(m_Buff)); try { // Try to construct the new object - new (static_cast(t_buff)) T(std::forward(args)...); + new (static_cast(m_Buff)) T(std::forward(args)...); } catch (...) { // Restore the old buffer - std::memcpy(t_buff, old_buff, sizeof(t_buff)); + std::memcpy(m_Buff, old_buff, sizeof(m_Buff)); throw; } // Fetch the old buffer and detroy it. - std::swap_ranges(t_buff, t_buff + sizeof(t_buff), old_buff); + std::swap_ranges(m_Buff, m_Buff + sizeof(m_Buff), old_buff); destroy(); - std::memcpy(t_buff, old_buff, sizeof(t_buff)); + std::memcpy(m_Buff, old_buff, sizeof(m_Buff)); } - m_Type = I; + m_Index = I; return get(*this); } - constexpr std::size_t index() const noexcept { return m_Type; } + constexpr std::size_t index() const noexcept { return m_Index; } void swap(variant_base &other) { // TODO } @@ -738,8 +738,8 @@ struct variant_base { void throw_if_invalid() const { static_assert(I < (sizeof...(Ts)), "Invalid index"); - if (m_Type != I) - throw my_bad_variant_access(m_Type); + if (m_Index != I) + throw my_bad_variant_access(m_Index); } void destroy() { @@ -752,11 +752,11 @@ struct variant_base { } // Until c++23 enums are represented as ints. - int m_Type; + int m_Index; // std::aligned_storage is deprecated and may be replaced with the construct // below. See // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1413r3.pdf - alignas(Ts...) std::byte t_buff[std::max({sizeof(Ts)...})]; + alignas(Ts...) std::byte m_Buff[std::max({sizeof(Ts)...})]; // The friend zone template @@ -773,8 +773,8 @@ template struct visitor_type { template constexpr static decltype(auto) visit(Visitor &&visitor, Variant &&variant) { - return visit(std::forward(visitor), variant.m_Type, - variant.t_buff); + return visit(std::forward(visitor), variant.m_Index, + variant.m_Buff); } /// @brief The visit method which will pick the right type depending on the @@ -827,14 +827,14 @@ constexpr decltype(auto) visit(Visitor &&visitor, template constexpr decltype(auto) get(variant_base &variant) { variant.template throw_if_invalid(); - return *reinterpret_cast *>(variant.t_buff); + return *reinterpret_cast *>(variant.m_Buff); } template constexpr decltype(auto) get(const variant_base &variant) { variant.template throw_if_invalid(); return *reinterpret_cast *>( - variant.t_buff); + variant.m_Buff); } template Date: Wed, 3 Jan 2024 00:02:33 +0100 Subject: [PATCH 36/37] check for underlying type --- include/cxx.h | 5 ++++- src/cxx.cc | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/cxx.h b/include/cxx.h index 6b9f9b43c..cd6cf8ca8 100644 --- a/include/cxx.h +++ b/include/cxx.h @@ -751,8 +751,11 @@ struct variant_base { *this); } - // Until c++23 enums are represented as ints. + // The underlying type is not fixed, but should be int - which we will verify + // statically. See + // https://timsong-cpp.github.io/cppwp/n4659/dcl.enum#7 int m_Index; + // std::aligned_storage is deprecated and may be replaced with the construct // below. See // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1413r3.pdf diff --git a/src/cxx.cc b/src/cxx.cc index 5291b828f..3dc6a20bf 100644 --- a/src/cxx.cc +++ b/src/cxx.cc @@ -666,6 +666,13 @@ static_assert(std::is_same_v(std::declval())), static_assert(sizeof(empty) == sizeof(std::uint8_t)); +// Verify that enums are represented as ints. We kind of assume that the enums +// with more fields would be represented by the underlying type not smaller +// than this enum (which is our smallest...). If this fails, we would need +// to pass the enum-type to the variant. +enum a_enum { AA }; + +static_assert(sizeof(std::underlying_type_t) == sizeof(int)); } // namespace detail #endif From ae8e8b40575a13e82c59926a80e768e9a628a7ff Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Thu, 21 Mar 2024 12:37:56 +0100 Subject: [PATCH 37/37] Update main.rs --- demo/src/main.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/demo/src/main.rs b/demo/src/main.rs index 6c93b0989..d6d304a40 100644 --- a/demo/src/main.rs +++ b/demo/src/main.rs @@ -6,15 +6,6 @@ mod ffi { tags: Vec, } - struct Blob<'a> { - size: &'a usize, - } - - enum BlobB<'a> { - Foo(Blob<'a>), - Bar(&'a usize), - } - /// A classic. enum BlobEnum { /// This is my doc