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

Polished refactor for constructor return type #1500

Merged
merged 10 commits into from
Nov 16, 2022

Conversation

SkymanOne
Copy link
Contributor

This is my iteration over #1491

  • Fixes unnecessary reference issue
  • Adds docs
  • Adds test assertion for Error variant
  • Indicated the right return type in metadata for Self and Result<Self>

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.

Haven't decided about the display name but it should be harmless

#constructor_info ::Error
>
>(
"core::result::Result"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure of the value of adding this to the display name...really we would want the generic parameters e.g. Result<(), MyError>. I know that is slightly tricky but not impossible. The other way would be to just have no display name, anyway the UI should detect whether we are dealing with a result using the type definition.

Anyway, leave it and I can always modify that in my PR

@ascjones ascjones merged commit 87ba86c into aj/constructor-return-type Nov 16, 2022
@ascjones ascjones deleted the gn/constructor-return-type branch November 16, 2022 17:54
HCastano pushed a commit that referenced this pull request Nov 17, 2022
* Remove unused trait `InitializerReturnType`

* WIP experimenting with static constructor return types

* WIP

* WIP generating metadata at compile time

* WIP using constructor result type

* Use generated method for metadata

* Generate constructor info using static types

* WIP restoring dispatch

* WIP adding metadata test to ui test

* Fix match as_result

* Attempt to infer result type

* Constructor dispatch compiles, remove metadata syntax stuff

* Fmt

* Remove unnecessary braces

* Fix up execution for results

* Fix up tests

* Remove printlns

* Explicitly set reverted flag

* ReturnTypeSpec accurately reflects ABI

* Polished refactor for constructor return type (#1500)

* revert instantiating when error

* display name + correct return typ when Self

* fix dereferencing issue

* UI test error variant + cleanup

* cargo fmt

* cargo fmt

* fix another reference issue

* fix spec contract test

Co-authored-by: Andrew Jones <ascjones@gmail.com>

* explicit prelude + fixing test error messages (#1503)

* Add constructor to ReturnFlags to not require Default trait impl

* Only invoke return_value for fallible constructors

* Add e2e fallible constructor value tests

* Fmt

* Add some reflection tests for constructors returning an error

* Check reflection for return types in UI tests

* Check reflection in example

* Fmt

* Add test for fallible constructor succeeding

* Add test for fallible constructor failing

* Remove unused message

* Fully qualify Ok(())

Co-authored-by: German <german@parity.io>
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.

2 participants