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

Conditional compilation #1707

Merged
merged 28 commits into from
Mar 16, 2023
Merged

Conditional compilation #1707

merged 28 commits into from
Mar 16, 2023

Conversation

SkymanOne
Copy link
Contributor

@SkymanOne SkymanOne commented Mar 7, 2023

Closes #1231

Summary

This PR introduces a native support for conditional compilation in ink! smart contracts

High level overview

It is now possible to attribute ink! elements with #[cfg(...)] macro attribute. You can use platform specific conditions or custom features specified in Cargo.toml file. (See integration-tests/condtional-compilation for example).

Important notice: Attributing event fields is not allowed.

Low level overview

IR structures have stayed the same, no significant changes there: just added a helper function to retrieve a list of all cfg attributes from the element.

Codegen has undergone some changes. Structures ContractAmountDispatchables, ContractDispatchableMessages and ContractDispatchableConstructors have been removed due to tight dependance on syntax analysis to count the number of elements which do not work with conditional compilation.

Main changes are based around DispatchableConstructorInfo and DispatchableMessageInfo code generation. Earlier, the logic was based around retrieving dispatchable IDs from the constant array. As mentioned in the #1231, this logic doesn't work when some ink items are omitted at the macro expansion. Therefore, it was decided to explicitly pass dispatchable id in the codegen instead of retrieving it from the array.

Other changes involve just putting bunch of cfg attributes around the codegen to omit associated structures and type definitions at the macro expansion stage.

The feature flag only works with the release of cargo-contract which includes: use-ink/cargo-contract#1004

Checklist

@SkymanOne SkymanOne marked this pull request as ready for review March 10, 2023 13:59
@SkymanOne SkymanOne changed the title [WIP] Conditional compilation Conditional compilation Mar 10, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2023

Codecov Report

Merging #1707 (2f54585) into master (c1111b5) will decrease coverage by 0.08%.
The diff coverage is 85.91%.

❗ Current head 2f54585 differs from pull request most recent head 07ba757. Consider uploading reports for the commit 07ba757 to get more accurate results

@@            Coverage Diff             @@
##           master    #1707      +/-   ##
==========================================
- Coverage   70.60%   70.53%   -0.08%     
==========================================
  Files         205      205              
  Lines        6434     6483      +49     
==========================================
+ Hits         4543     4573      +30     
- Misses       1891     1910      +19     
Impacted Files Coverage Δ
crates/ink/ir/src/ir/item_impl/mod.rs 86.29% <0.00%> (-4.39%) ⬇️
crates/ink/src/reflect/dispatch.rs 0.00% <ø> (ø)
...ink/tests/ui/contract/pass/constructor-selector.rs 46.15% <ø> (ø)
...tes/ink/tests/ui/contract/pass/message-selector.rs 33.33% <ø> (ø)
crates/ink/ir/src/ir/item_impl/constructor.rs 89.70% <66.66%> (-2.23%) ⬇️
crates/ink/ir/src/ir/item_impl/message.rs 91.08% <66.66%> (-1.55%) ⬇️
crates/ink/ir/src/ir/trait_def/item/trait_item.rs 89.70% <66.66%> (-2.23%) ⬇️
crates/ink/ir/src/ir/item/event.rs 85.89% <80.00%> (-0.87%) ⬇️
crates/ink/codegen/src/generator/dispatch.rs 92.95% <93.33%> (-1.25%) ⬇️
...odegen/src/generator/as_dependency/call_builder.rs 100.00% <100.00%> (ø)
... and 11 more

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

crates/ink/codegen/src/generator/dispatch.rs Outdated Show resolved Hide resolved
crates/ink/codegen/src/generator/dispatch.rs Outdated Show resolved Hide resolved
crates/ink/codegen/src/generator/dispatch.rs Outdated Show resolved Hide resolved
crates/ink/codegen/src/generator/dispatch.rs Outdated Show resolved Hide resolved
crates/ink/codegen/src/generator/item_impls.rs Outdated Show resolved Hide resolved
crates/ink/ir/src/ir/item/event.rs Outdated Show resolved Hide resolved
crates/ink/codegen/src/generator/dispatch.rs Outdated Show resolved Hide resolved
@SkymanOne SkymanOne requested a review from a team as a code owner March 14, 2023 12:19
@SkymanOne SkymanOne requested review from xgreenx and ascjones and removed request for xgreenx March 14, 2023 12:36
.gitlab-ci.yml Outdated Show resolved Hide resolved
@SkymanOne SkymanOne requested a review from xgreenx March 15, 2023 15:11
.gitlab-ci.yml Show resolved Hide resolved
@SkymanOne SkymanOne requested a review from xgreenx March 15, 2023 16:56
Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

crates/ink/ir/src/ir/mod.rs Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@@ -24,18 +24,14 @@ fn main() {
use contract::Contract;
use std::any::TypeId;

const ID: u32 = <Contract as ::ink::reflect::ContractDispatchableConstructors<
{ <Contract as ::ink::reflect::ContractAmountDispatchables>::CONSTRUCTORS },
>>::IDS[0];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to document the breaking change that you can no longer access constructor/message info in this way by the index. But I doubt too many people are depending on that...

Copy link
Contributor Author

@SkymanOne SkymanOne Mar 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please open a separate issue for this? I guess this needs to be documented in release notes

@SkymanOne SkymanOne merged commit bb9cc8c into master Mar 16, 2023
@SkymanOne SkymanOne deleted the gn/conditional-compilation branch March 16, 2023 18:55
@SkymanOne SkymanOne mentioned this pull request Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Messages and contstructor cannot be conditionally compiled
4 participants