[refactor] #3814: Update `iroha_ffi_derive` to use syn 2.0 #3830

DCNick3 · 2023-08-21T15:07:29Z

Description

Separate parsing & (some) validation of attributes and codegen using darling, by storing this information into a strongly typed struct (FfiTypeInput and alike).

To facilitate this, add parsers for derive(FfiType), getset crate and built-in repr and derive attributes and tests for them.

One unfortunate concern is that now ffi_derive has stopped supporting unions completely, due to a limitation of darling. I don't think they are that useful though, especially in context of a high level API iroha provides

Linked issue

Closes #3814

Benefits

Cleaner code, better macro errors

Checklist

Rebase (need less commits and proper messages)
~~[ ] Move utility classes to a common place (iroha-derive-primitives, probably)~~ not a good idea until iroha-derive-primitives is syn2-only
Make codegen for ffi!, #[ffi_import] and #[ffi_export] emit! errors rather than bail!ing
Clean up stray commented out code
Fix lints

coveralls · 2023-08-21T15:18:17Z

Pull Request Test Coverage Report for Build 5949198492

1434 of 1871 (76.64%) changed or added relevant lines in 11 files are covered.
5568 unchanged lines in 94 files lost coverage.
Overall coverage decreased (-1.4%) to 57.998%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
ffi/derive/src/attr_parse/derive.rs	98	100	98.0%
ffi/derive/src/getset_gen.rs	137	148	92.57%
ffi/derive/src/attr_parse/repr.rs	194	210	92.38%
ffi/derive/src/wrapper.rs	43	64	67.19%
ffi/derive/src/lib.rs	107	134	79.85%
ffi/derive/src/emitter.rs	25	59	42.37%
ffi/derive/src/impl_visitor.rs	81	147	55.1%
ffi/derive/src/convert.rs	295	400	73.75%
ffi/derive/src/attr_parse/getset.rs	441	596	73.99%

Files with Coverage Reduction	New Missed Lines	%
cli/src/main.rs	1	0%
cli/src/torii/pagination.rs	1	98.9%
config/base/src/runtime_upgrades.rs	1	51.72%
config/src/torii.rs	1	96.67%
core/src/smartcontracts/isi/block.rs	1	87.5%
crypto/src/merkle.rs	1	96.23%
data_model/derive/src/partially_tagged.rs	1	99.67%
data_model/src/account.rs	1	52.21%
data_model/src/domain.rs	1	47.37%
derive_primitives/src/lib.rs	1	0%

Totals
Change from base Build 5423219773:	-1.4%
Covered Lines:	21420
Relevant Lines:	36932

💛 - Coveralls

…ro-error with manyhow

…arse attributes and use syn 2.0 WIP: do not fail fast in ffi!, ffi_import and ffi_export WIP: fix clippy lints WIP: remove commented out code WIP: make the presence of generic arguments a fatal error (it leads to dubious errors if codegen is ran) WIP: actually fix clippy lints

…ro-error with manyhow Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

…arse attributes and use syn 2.0 Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

0x009922

Great PR and a huge code & UI improvement, it seems for me. I don't have critical objections, although I am not proficient in FFI & procmacro topics.

However, there are some questions and suggestions.

ffi/derive/src/wrapper.rs

ffi/derive/src/attr_parse/repr.rs

ffi/derive/src/getset_gen.rs

ffi/derive/src/lib.rs

ffi/derive/src/attr_parse/getset.rs

ffi/tests/ffi_export.rs

ffi/derive/src/attr_parse/repr.rs

ffi/derive/src/attr_parse/derive.rs

ffi/derive/src/attr_parse/getset.rs

ffi/derive/src/attr_parse/repr.rs

ffi/derive/src/convert.rs

ffi/derive/src/impl_visitor.rs

ilchu

Just a small doc typo fix, ping afterwards for approval.

ffi/derive/src/convert.rs

…ro-error with manyhow Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

…arse attributes and use syn 2.0 Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

…tokens Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

…into Emitter::finish_token_stream Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

…parsing Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

… ffi function args Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

… Attrs naming Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

…parsing attributes to lib.rs Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

…ives in proc macros Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Aug 21, 2023

DCNick3 force-pushed the syn2-iroha-ffi branch from cd093fa to 25af7e5 Compare August 21, 2023 15:08

DCNick3 added a commit to DCNick3/iroha that referenced this pull request Aug 23, 2023

[refactor] hyperledger-iroha#3830: iroha_ffi_derive: replace proc-mac…

f67a6e4

…ro-error with manyhow

DCNick3 force-pushed the syn2-iroha-ffi branch from 253155d to 4839cc6 Compare August 23, 2023 08:54

DCNick3 added a commit to DCNick3/iroha that referenced this pull request Aug 23, 2023

[refactor] hyperledger-iroha#3830: iroha_ffi_derive: replace proc-mac…

a0a9460

…ro-error with manyhow Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

DCNick3 added a commit to DCNick3/iroha that referenced this pull request Aug 23, 2023

[refactor] hyperledger-iroha#3830: iroha_ffi_derive: use darling to p…

73bbc38

…arse attributes and use syn 2.0 Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

DCNick3 force-pushed the syn2-iroha-ffi branch 2 times, most recently from 73bbc38 to 2567474 Compare August 23, 2023 08:56

DCNick3 marked this pull request as ready for review August 25, 2023 09:13

DCNick3 requested review from s8sato, appetrosyan, mversic, Arjentix, SamHSmith, QuentinI, outoftardis, ilchu, pesterev, Erigara and 0x009922 as code owners August 25, 2023 09:13

mversic assigned ilchu and 0x009922 Aug 28, 2023

0x009922 reviewed Aug 29, 2023

View reviewed changes

ilchu reviewed Sep 1, 2023

View reviewed changes

ilchu reviewed Sep 4, 2023

View reviewed changes

ffi/derive/src/convert.rs Outdated Show resolved Hide resolved

DCNick3 force-pushed the syn2-iroha-ffi branch from 6f9bfdd to 47d383a Compare September 4, 2023 11:28

DCNick3 added 2 commits September 4, 2023 15:18

[refactor] hyperledger-iroha#3814: iroha_ffi_derive: replace proc-mac…

bbf4493

…ro-error with manyhow Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

[refactor] hyperledger-iroha#3814: iroha_ffi_derive: use darling to p…

faecccd

…arse attributes and use syn 2.0 Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>

DCNick3 added 9 commits September 4, 2023 15:18

[refactor] hyperledger-iroha#3814: Fix typos and improve error messages

2439acb

Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#3814: Use parse_display to parse getset …

1f33f8a

…tokens Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#3814: rename Emitter::into_token_stream …

85e1c97

…into Emitter::finish_token_stream Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#3814: reduce repetition in getset attrs …

12dffd7

…parsing Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#2573: allow usage of pattern matching in…

7aa1e3a

… ffi function args Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#2573: use parse_display & update Attr ->…

7f0a772

… Attrs naming Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#2573: extract a common test utility for …

e5cf944

…parsing attributes to lib.rs Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#2573: clean up comments

8c247ca

Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

[refactor] hyperledger-iroha#2573: document limitation of parsing der…

a255c09

…ives in proc macros Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>

DCNick3 force-pushed the syn2-iroha-ffi branch from 47d383a to a255c09 Compare September 4, 2023 12:18

ilchu approved these changes Sep 5, 2023

View reviewed changes

0x009922 approved these changes Sep 5, 2023

View reviewed changes

Erigara assigned DCNick3 Sep 5, 2023

DCNick3 merged commit baed2fb into hyperledger-iroha:iroha2-dev Sep 5, 2023
11 checks passed

DCNick3 mentioned this pull request Sep 21, 2023

Bump syn to 2.0 #3682

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[refactor] #3814: Update `iroha_ffi_derive` to use syn 2.0 #3830

[refactor] #3814: Update `iroha_ffi_derive` to use syn 2.0 #3830

DCNick3 commented Aug 21, 2023 •

edited

Loading

coveralls commented Aug 21, 2023 •

edited

Loading

0x009922 left a comment

ilchu left a comment

[refactor] #3814: Update iroha_ffi_derive to use syn 2.0 #3830

[refactor] #3814: Update iroha_ffi_derive to use syn 2.0 #3830

Conversation

DCNick3 commented Aug 21, 2023 • edited Loading

Description

Linked issue

Benefits

Checklist

coveralls commented Aug 21, 2023 • edited Loading

Pull Request Test Coverage Report for Build 5949198492

💛 - Coveralls

0x009922 left a comment

Choose a reason for hiding this comment

ilchu left a comment

Choose a reason for hiding this comment

[refactor] #3814: Update `iroha_ffi_derive` to use syn 2.0 #3830

[refactor] #3814: Update `iroha_ffi_derive` to use syn 2.0 #3830

DCNick3 commented Aug 21, 2023 •

edited

Loading

coveralls commented Aug 21, 2023 •

edited

Loading