From e433d4af4cf78c88dc0cb3e8139e835470b72fd3 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 22 Mar 2020 23:43:07 -0400 Subject: [PATCH] Extend Rust FFI Do idiomatic C++ copy and move constructors for a few things, so wrapping structs' defaults can work. --- nix-rust/src/c.rs | 7 +++++++ src/libstore/derivations.hh | 3 +++ src/libstore/nar-info.cc | 2 +- src/libstore/path.hh | 14 ++++++++++++++ src/libstore/store-api.cc | 15 --------------- src/libstore/store-api.hh | 5 +++-- src/libutil/hash.hh | 6 ++++++ src/libutil/rust-ffi.hh | 32 +++++++++++++++++++++++++++++--- 8 files changed, 63 insertions(+), 21 deletions(-) diff --git a/nix-rust/src/c.rs b/nix-rust/src/c.rs index c1358545f..dc2f8ddf5 100644 --- a/nix-rust/src/c.rs +++ b/nix-rust/src/c.rs @@ -64,6 +64,13 @@ pub extern "C" fn ffi_StorePath_clone(self_: &StorePath) -> StorePath { self_.clone() } +#[no_mangle] +pub extern "C" fn ffi_StorePath_clone_to(self_: &StorePath, other: *mut StorePath) { + unsafe { + core::ptr::write(other, self_.clone()); + } +} + #[no_mangle] pub extern "C" fn ffi_StorePath_name(self_: &StorePath) -> &str { self_.name.name() diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 7222d25e5..f010318ce 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -22,6 +22,9 @@ struct DerivationOutput , hashAlgo(std::move(hashAlgo)) , hash(std::move(hash)) { } + DerivationOutput(const DerivationOutput &) = default; + DerivationOutput(DerivationOutput &&) = default; + DerivationOutput & operator = (const DerivationOutput &) = default; void parseHashInfo(bool & recursive, Hash & hash) const; }; diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index 1375094b5..87b1b656b 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -4,7 +4,7 @@ namespace nix { NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & whence) - : ValidPathInfo(StorePath::dummy.clone()) // FIXME: hack + : ValidPathInfo(StorePath::dummy) // FIXME: hack { auto corrupt = [&]() { throw Error(format("NAR info file '%1%' is corrupt") % whence); diff --git a/src/libstore/path.hh b/src/libstore/path.hh index c90bb1fff..186976855 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -13,6 +13,7 @@ extern "C" { void ffi_StorePath_drop(void *); bool ffi_StorePath_less_than(const StorePath & a, const StorePath & b); bool ffi_StorePath_eq(const StorePath & a, const StorePath & b); + void ffi_StorePath_clone_to(const StorePath & _other, StorePath & _this); unsigned char * ffi_StorePath_hash_data(const StorePath & p); } @@ -43,6 +44,19 @@ struct StorePath : rust::Value<3 * sizeof(void *) + 24, ffi_StorePath_drop> return !(*this == other); } + StorePath(StorePath && that) = default; + + StorePath(const StorePath & that) + { + ffi_StorePath_clone_to(that, *this); + } + + void operator = (const StorePath & that) + { + (rust::Value<3 * sizeof(void *) + 24, ffi_StorePath_drop>::operator = (that)); + ffi_StorePath_clone_to(that, *this); + } + StorePath clone() const; /* Check whether a file name ends with the extension for diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b9e894a9a..261afed49 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -687,21 +687,6 @@ void copyClosure(ref srcStore, ref dstStore, } -ValidPathInfo::ValidPathInfo(const ValidPathInfo & other) - : path(other.path.clone()) - , deriver(other.deriver ? other.deriver->clone(): std::optional{}) - , narHash(other.narHash) - , references(cloneStorePathSet(other.references)) - , registrationTime(other.registrationTime) - , narSize(other.narSize) - , id(other.id) - , ultimate(other.ultimate) - , sigs(other.sigs) - , ca(other.ca) -{ -} - - std::optional decodeValidPathInfo(const Store & store, std::istream & str, bool hashGiven) { std::string path; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 0fa59be6a..0237d0b04 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -189,8 +189,9 @@ struct ValidPathInfo Strings shortRefs() const; - ValidPathInfo(StorePath && path) : path(std::move(path)) { } - explicit ValidPathInfo(const ValidPathInfo & other); + ValidPathInfo(StorePath && path) : path(std::move(path)) { }; + ValidPathInfo(const StorePath & path) : path(path) { }; + ValidPathInfo(const ValidPathInfo & other) = default; virtual ~ValidPathInfo() { } }; diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index ea9fca3e7..294555a09 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -44,6 +44,12 @@ struct Hash string. */ Hash(const std::string & s, HashType type = htUnknown); + Hash(const Hash &) = default; + + Hash(Hash &&) = default; + + Hash & operator = (const Hash &) = default; + void init(); /* Check whether a hash is set. */ diff --git a/src/libutil/rust-ffi.hh b/src/libutil/rust-ffi.hh index 228e2eead..584dcf110 100644 --- a/src/libutil/rust-ffi.hh +++ b/src/libutil/rust-ffi.hh @@ -30,7 +30,10 @@ protected: // Must not be called directly. Value() - { } + { + // Precaution, in case this is used improperly + evacuate(); + } Value(Value && other) : raw(other.raw) @@ -38,6 +41,19 @@ protected: other.evacuate(); } + // Not all Rust types are Clone / Copy, but our base Value class needs to + // have a copy constructor so that ones which do implement Copy/Clone + // can be copied/cloned. + Value(const Value & other) + : raw(other.raw) + { + } + void operator =(const Value & other) + { + if (!isEvacuated()) + drop(this); + } + void operator =(Value && other) { if (!isEvacuated()) @@ -76,6 +92,16 @@ struct Vec : Value<3 * sizeof(void *), drop> { return ((const T * *) &this->raw)[0]; } + +protected: + + // Must not be called directly. + Vec(); + + Vec(Vec && other) = default; + + // Delete until we know how to do this properly. + Vec(const Vec & other) = delete; }; /* A Rust slice. */ @@ -144,7 +170,7 @@ struct Result std::exception_ptr * exc; }; - Result() : tag(Uninit) { }; // FIXME: remove + Result() = delete; Result(const Result &) = delete; @@ -171,7 +197,7 @@ struct Result } /* Rethrow the wrapped exception or return the wrapped value. */ - T unwrap() + T unwrap() && { if (tag == Ok) { tag = Uninit;