Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] #2434: FFI bindgen library #2314

Merged
merged 15 commits into from
Jul 22, 2022

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Jun 3, 2022

Signed-off-by: Marin Veršić [email protected]

293320603_1100305063913355_1700096561062333955_n

Description of the Change

I've found that often times I need to traverse the type and produce conversions from source to FFI compliant type and vice versa.
There have been a lot of code duplication and convoluted logic which this PR aims to resolve

Traits which enable this happen: ReprC, AsReprC, TryFromReprC IntoFfi. A Rust structure is converted into an intermediary FFI struct via IntoFfi which implements the AsReprC and can be converted into a type that can cross FFI boundary. This goes the other way as well - FFI ReprC type is converted into a struct via TryFromReprC

  • 1st rule of this lib - there is no ownership transfer except for opaque pointer types
  • Added catch_unwind to prevent UB when panicking over FFI

Issue

Closes #2327

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jun 3, 2022
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
@mversic mversic force-pushed the ffi_common_arg_convert branch 2 times, most recently from 1a880b2 to 4b02d18 Compare June 3, 2022 15:03
@codecov
Copy link

codecov bot commented Jun 3, 2022

Codecov Report

Merging #2314 (586d59a) into iroha2-dev (a16d9c3) will decrease coverage by 0.83%.
The diff coverage is 60.38%.

@@              Coverage Diff               @@
##           iroha2-dev    #2314      +/-   ##
==============================================
- Coverage       67.61%   66.77%   -0.84%     
==============================================
  Files             140      145       +5     
  Lines           26173    27150     +977     
==============================================
+ Hits            17696    18130     +434     
- Misses           8477     9020     +543     
Impacted Files Coverage Δ
actor/src/broker.rs 88.05% <ø> (ø)
client/src/client.rs 45.33% <0.00%> (-0.99%) ⬇️
core/src/smartcontracts/isi/asset.rs 54.82% <ø> (ø)
core/src/smartcontracts/isi/mod.rs 56.77% <ø> (+3.90%) ⬆️
core/src/smartcontracts/isi/triggers.rs 0.00% <0.00%> (ø)
core/src/wsv.rs 83.17% <ø> (+0.92%) ⬆️
data_model/src/events/execute_trigger.rs 7.40% <ø> (ø)
data_model/src/metadata.rs 90.08% <0.00%> (ø)
data_model/src/predicate.rs 99.79% <ø> (-0.01%) ⬇️
data_model/src/role.rs 6.57% <0.00%> (-0.37%) ⬇️
... and 62 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

ffi/derive/src/bindgen.rs Outdated Show resolved Hide resolved
@mversic mversic force-pushed the ffi_common_arg_convert branch 3 times, most recently from 96ed308 to 65840fc Compare June 3, 2022 17:30
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
ffi/derive/src/bindgen.rs Outdated Show resolved Hide resolved
ffi/derive/src/bindgen.rs Outdated Show resolved Hide resolved
ffi/derive/tests/ui_fail/derive_skip_field.rs Outdated Show resolved Hide resolved
ffi/derive/tests/ui_fail/mutable_input_arg.rs Show resolved Hide resolved
@mversic mversic force-pushed the ffi_common_arg_convert branch 2 times, most recently from 2c64900 to 5c9ef15 Compare June 6, 2022 20:10
@appetrosyan appetrosyan changed the title [refactor] #0000: extract FFI conversion logic to separate module [refactor] #2327: extract FFI conversion logic to separate module Jun 7, 2022
@mversic mversic force-pushed the ffi_common_arg_convert branch 2 times, most recently from 6be3eb2 to 9406231 Compare June 7, 2022 07:55
ffi/derive/src/arg.rs Outdated Show resolved Hide resolved
@mversic mversic force-pushed the ffi_common_arg_convert branch 3 times, most recently from f06bbbe to 8270fc1 Compare July 22, 2022 12:55
Signed-off-by: Marin Veršić <[email protected]>
@mversic mversic changed the title [refactor] #2434: extract FFI conversion logic to separate module [feature] #2434: FFI bindgen library Jul 22, 2022
@mversic mversic merged commit 985f41f into hyperledger:iroha2-dev Jul 22, 2022
@mversic mversic deleted the ffi_common_arg_convert branch July 22, 2022 16:20
mversic added a commit to mversic/iroha that referenced this pull request Jul 22, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 6, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 6, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 6, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 7, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 8, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 9, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 9, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants