Skip to content

Conversation

@ascjones
Copy link
Collaborator

@ascjones ascjones commented Dec 8, 2022

Follow up to #1450, addresses the metadata part of #1530.

Messages now return Result<_, LangError>. The metadata was updated to reflect this for inherent methods but not for trait methods.

Note that as mentioned in the #1530, this only fixes the external metadata, and does not address the codegen for cross contract calling.

Additionally this PR copies a similar test for fallible constructor metadata from one of the compile tests, and allows it to use the helpers for extracting result types here.

Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Quick turnaround ⚡

}

#[test]
fn trait_message_metadata_return_value_is_result() {
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of completeness, we may also want to add tests for inherent messages, inherent fallible messages, etc.

Right now as long as the tests inlang-err-integration-tests work our metadata stuff is probably fine, but it would be nice to be more explicit with our checks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree, we need more tests for the metadata like this. Work for a follow-on PR methinks.

@ascjones ascjones merged commit 82561fd into master Dec 8, 2022
@ascjones ascjones deleted the aj/fix-trait-call-codegen branch December 8, 2022 17:44
@ascjones ascjones mentioned this pull request Jan 12, 2023
HCastano pushed a commit that referenced this pull request Jan 23, 2023
* Add failing message return type metadata test

* Set wrapped return type for trait message

* Combine existing contractor metadata test

* Common extract result function

* Refactor fallible constructor test to use helpers

* Remove dummy test

* Fix clippy warning
@HCastano HCastano mentioned this pull request Jan 24, 2023
@ascjones ascjones mentioned this pull request Feb 15, 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.

3 participants