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

Add Result<Self, Error> return type for constructors #1446

Merged
merged 30 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
7a4193b
constructors return Err abort transaction + started example
Sep 22, 2022
e9af561
return type parser
Sep 25, 2022
89ec79c
Merge branch 'master' into gn/constructor-err
Sep 25, 2022
2cd9dd9
add output type to dispatchable constructor
Sep 25, 2022
fdad4dc
codegen adds Result to return type
Sep 26, 2022
47fba58
add type cheking
Sep 26, 2022
109536d
fix CALLABLE type
Sep 26, 2022
ab3897f
add comments for refactoring
Oct 12, 2022
4d6766f
ugly solution for failable contracts
Oct 12, 2022
c0ef46c
Merge branch 'master' into gn/constructor-err
Oct 13, 2022
10126f2
avoid unnecessary error check for constructors
Oct 17, 2022
2fa1ac0
avoids encoding storage, adds demo example
Oct 17, 2022
e2649c5
allows differents syntax format of result return type
Oct 21, 2022
8d79fef
cargo fmt
Oct 21, 2022
89e1789
add changes to CHANGELOG.md
Oct 21, 2022
36de262
add tests to the example
Oct 23, 2022
50cf900
add UI tests
Oct 23, 2022
c372352
return comment to dns example
Oct 23, 2022
52da525
fix formatting in UI examples
Oct 23, 2022
6cea984
fix tests and docs
Oct 24, 2022
d7dcb59
apply formatting suggestions
Oct 24, 2022
9d00fd2
change example and UI tests for more clarity
Oct 24, 2022
f35ef98
add imports for UI test
Oct 24, 2022
0532993
refactor execution.rs
Oct 26, 2022
7b4d56c
remove syntactical check for constructors return type
Oct 26, 2022
0f8abd1
UI test for type aliases
Oct 26, 2022
b5e99fe
LLVM optimisation and refactoring
Oct 27, 2022
ae37fe6
updated docs
Oct 27, 2022
10bbbc6
refactoring and moving feature demo to mother contract
Oct 27, 2022
b94d31f
remove cargo-contract install in docker
Oct 27, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Allows to use `Result<Self, Error>` as a return type in constructors - [#1446](https://github.com/paritytech/ink/pull/1446)

## Version 4.0.0-alpha.3

Expand All @@ -22,7 +23,7 @@ crate. All existing sub-crates are reexported and should be used via the new `in
- Replace all usages of individual crates with reexports, e.g. `ink_env` ➜ `ink::env`.

#### Storage Rework
[#1331](https://github.com/paritytech/ink/pull/1331) changes the way `ink!` works with contract storage. Storage keys
[#1331](https://github.com/paritytech/ink/pull/1331) changes the way `ink!` works with contract storage. Storage keys
are generated at compile-time, and user facing abstractions which determine how contract data is laid out in storage
have changed.

Expand Down
8 changes: 5 additions & 3 deletions crates/ink/codegen/src/generator/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,19 @@ impl Dispatch<'_> {
let payable = constructor.is_payable();
let selector_id = constructor.composed_selector().into_be_u32().hex_padded_suffixed();
let selector_bytes = constructor.composed_selector().hex_lits();
let output_tuple_type = constructor.output().map(quote::ToTokens::to_token_stream)
.unwrap_or_else(|| quote! { () });
let input_bindings = generator::input_bindings(constructor.inputs());
let input_tuple_type = generator::input_types_tuple(constructor.inputs());
let input_tuple_bindings = generator::input_bindings_tuple(constructor.inputs());
quote_spanned!(constructor_span=>
impl ::ink::reflect::DispatchableConstructorInfo<#selector_id> for #storage_ident {
type Input = #input_tuple_type;
type Output = #output_tuple_type;
type Storage = #storage_ident;

const CALLABLE: fn(Self::Input) -> Self::Storage = |#input_tuple_bindings| {
#storage_ident::#constructor_ident( #( #input_bindings ),* )
const CALLABLE: fn(Self::Input) -> Self::Output = |#input_tuple_bindings| {
#storage_ident::#constructor_ident(#( #input_bindings ),* )
};
const PAYABLE: ::core::primitive::bool = #payable;
const SELECTOR: [::core::primitive::u8; 4usize] = [ #( #selector_bytes ),* ];
Expand Down Expand Up @@ -559,7 +562,6 @@ impl Dispatch<'_> {
}>>::IDS[#index]
}>>::PAYABLE
);

quote_spanned!(constructor_span=>
Self::#constructor_ident(input) => {
if #any_constructor_accept_payment && #deny_payment {
Expand Down
3 changes: 2 additions & 1 deletion crates/ink/codegen/src/generator/item_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,11 @@ impl ItemImpls<'_> {
let ident = constructor.ident();
let inputs = constructor.inputs();
let statements = constructor.statements();
let output = constructor.output();
quote_spanned!(span =>
#( #attrs )*
#[cfg(not(feature = "__ink_dylint_Constructor"))]
#vis fn #ident( #( #inputs ),* ) -> Self {
#vis fn #ident( #( #inputs ),* ) -> #output {
#( #statements )*
}
)
Expand Down
55 changes: 50 additions & 5 deletions crates/ink/ir/src/ir/item_impl/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,34 @@ impl Constructor {
}) if path.is_ident("Self"))
}

/// Ensures that the return type of the ink! constructor is `Self`.
/// Returns `true` if the given type is `Result<Self>`.
fn type_is_result_self_val(ty: &syn::Type) -> bool {
let res = matches!(ty, syn::Type::Path(syn::TypePath {
qself: None,
path
}) if Self::is_result_path(path));
res
}

/// Helper functions that evaluates that the return type is `Result`
/// and the `Ok` variant is `Self` type
fn is_result_path(path: &syn::Path) -> bool {
if path.leading_colon.is_none()
&& path.segments.last().is_some()
&& path.segments.last().unwrap().ident == "Result"
{
if let syn::PathArguments::AngleBracketed(angle_args) =
&path.segments.last().unwrap().arguments
{
if let Some(syn::GenericArgument::Type(ty)) = angle_args.args.first() {
return Self::type_is_self_val(ty)
}
}
}
false
}
SkymanOne marked this conversation as resolved.
Show resolved Hide resolved

/// Ensures that the return type of the ink! constructor is `Self` or `Result<Self>`.
SkymanOne marked this conversation as resolved.
Show resolved Hide resolved
///
/// Returns an appropriate error otherwise.
///
Expand All @@ -111,10 +138,12 @@ impl Constructor {
))
}
syn::ReturnType::Type(_, return_type) => {
if !Self::type_is_self_val(return_type.as_ref()) {
if !Self::type_is_self_val(return_type.as_ref())
&& !Self::type_is_result_self_val(return_type.as_ref())
{
return Err(format_err_spanned!(
return_type,
"ink! constructors must return Self",
"ink! constructors must return Self or Result<Self>",
))
}
}
Expand Down Expand Up @@ -241,6 +270,14 @@ impl Constructor {
pub fn attrs(&self) -> &[syn::Attribute] {
&self.item.attrs
}

/// Returns the return type of the ink! constructor if any.
pub fn output(&self) -> Option<&syn::Type> {
match &self.item.sig.output {
syn::ReturnType::Default => None,
syn::ReturnType::Type(_, return_type) => Some(return_type),
}
}
}

#[cfg(test)]
Expand Down Expand Up @@ -390,6 +427,11 @@ mod tests {
#[ink(constructor)]
fn my_constructor(input1: i32, input2: i64, input3: u32, input4: u64) -> Self {}
},
// Result return type
syn::parse_quote! {
#[ink(constructor)]
pub fn my_constructor() -> Result<Self, ()> {}
},
];
for item_method in item_methods {
assert!(<ir::Constructor as TryFrom<_>>::try_from(item_method).is_ok());
Expand Down Expand Up @@ -438,11 +480,14 @@ mod tests {
},
syn::parse_quote! {
#[ink(constructor)]
pub fn my_constructor() -> Result<Self, ()> {}
pub fn my_constructor() -> Result<i32, ()> {}
},
];
for item_method in item_methods {
assert_try_from_fails(item_method, "ink! constructors must return Self")
assert_try_from_fails(
item_method,
"ink! constructors must return Self or Result<Self>",
)
}
}

Expand Down
7 changes: 5 additions & 2 deletions crates/ink/src/codegen/dispatch/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl<C> ConstructorReturnType<C> for private::Seal<C> {
impl<C, E> ConstructorReturnType<C> for private::Seal<Result<C, E>> {
const IS_RESULT: bool = true;
type Error = E;
type ReturnValue = Result<C, E>;
type ReturnValue = Self::Error;
SkymanOne marked this conversation as resolved.
Show resolved Hide resolved

#[inline]
fn as_result(&self) -> Result<&C, &Self::Error> {
Expand All @@ -176,7 +176,10 @@ impl<C, E> ConstructorReturnType<C> for private::Seal<Result<C, E>> {

#[inline]
fn return_value(&self) -> &Self::ReturnValue {
&self.0
self.0
.as_ref()
.err()
.expect("value returned only when error occurs. qed")
}
}

Expand Down
5 changes: 4 additions & 1 deletion crates/ink/src/reflect/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,11 @@ pub trait DispatchableConstructorInfo<const ID: u32> {
/// The ink! storage struct type.
type Storage;

/// Reflects the output type of the dispatchable ink! message.
type Output;

/// The closure that can be used to dispatch into the dispatchable ink! constructor.
const CALLABLE: fn(Self::Input) -> Self::Storage;
const CALLABLE: fn(Self::Input) -> Self::Output;

/// Yields `true` if the dispatchable ink! constructor is payable.
const PAYABLE: bool;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#[ink::contract]
mod contract {
#[ink(storage)]
pub struct Contract {}

#[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))]
pub enum Error {
Foo,
}

impl Contract {
#[ink(constructor)]
pub fn constructor() -> core::result::Result<u8, Error> {
Ok(5_u8)
}

#[ink(message)]
pub fn message(&self) {}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: ink! constructors must return Self or Result<Self>
--> tests/ui/contract/fail/constructor-return-result-invalid.rs:14:33
|
14 | pub fn constructor() -> core::result::Result<u8, Error> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#[ink::contract]
mod contract {
#[ink(storage)]
pub struct Contract {}

#[derive(Debug, PartialEq, Eq)]
pub enum Error {
Foo,
}

impl Contract {
#[ink(constructor)]
pub fn constructor() -> core::result::Result<Self, Error> {
Ok(Self {})
}

#[ink(message)]
pub fn message(&self) {}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0277]: the trait bound `contract::Error: WrapperTypeEncode` is not satisfied
--> tests/ui/contract/fail/constructor-return-result-non-codec-error.rs:13:9
|
13 | pub fn constructor() -> core::result::Result<Self, Error> {
| ^^^ the trait `WrapperTypeEncode` is not implemented for `contract::Error`
|
= help: the following other types implement trait `WrapperTypeEncode`:
&T
&mut T
Arc<T>
Box<T>
Cow<'a, T>
Rc<T>
String
Vec<T>
parity_scale_codec::Ref<'a, T, U>
= note: required because of the requirements on the impl of `Encode` for `contract::Error`
note: required by a bound in `execute_constructor`
--> src/codegen/dispatch/execution.rs
|
| <private::Seal<R> as ConstructorReturnType<Contract>>::ReturnValue: Encode,
| ^^^^^^ required by this bound in `execute_constructor`
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#[ink::contract]
mod contract {
#[ink(storage)]
pub struct Contract {}

#[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))]
pub enum Error {
Foo,
}

pub type Result<T> = core::result::Result<T, Error>;

impl Contract {
#[ink(constructor)]
pub fn constructor() -> Result<Self> {
Err(Error::Foo)
}

#[ink(message)]
pub fn message(&self) {}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#[ink::contract]
mod contract {
#[ink(storage)]
pub struct Contract {}

#[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))]
pub enum Error {
Foo,
}

impl Contract {
#[ink(constructor)]
pub fn constructor() -> Result<Self, Error> {
Err(Error::Foo)
}

#[ink(message)]
pub fn message(&self) {}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#[ink::contract]
mod contract {
#[ink(storage)]
pub struct Contract {}

#[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))]
pub enum Error {
Foo,
}

impl Contract {
#[ink(constructor)]
pub fn constructor() -> core::result::Result<Self, Error> {
Err(Error::Foo)
}

#[ink(message)]
pub fn message(&self) {}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#[ink::contract]
mod contract {
#[ink(storage)]
pub struct Contract {}

#[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))]
pub enum Error {
Foo,
}

impl Contract {
#[ink(constructor)]
pub fn constructor() -> Result<Self, Error> {
Ok(Self {})
}

#[ink(message)]
pub fn message(&self) {}
}
}

fn main() {}
Loading