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

Rework fallible constructors #1491

Merged
merged 36 commits into from
Nov 17, 2022
Merged

Rework fallible constructors #1491

merged 36 commits into from
Nov 17, 2022

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Nov 10, 2022

Follow up to #1460 and #1446. Closes #1483.

This PR:

Test with: cargo test --manifest-path ./examples/lang-err-integration-tests/constructors-return-value/Cargo.toml --features e2e-tests

@SkymanOne
Copy link
Contributor

I am also getting clippy warning for constructors attribute:

this expression creates a reference which is immediately dereferenced by the compiler

Currently debugging this

ascjones and others added 2 commits November 16, 2022 17:41
* 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>
@ascjones ascjones changed the title PoC: rework fallible constructors Rework fallible constructors Nov 16, 2022
@ascjones ascjones marked this pull request as ready for review November 17, 2022 17:27
@@ -0,0 +1,35 @@
[package]
name = "constructors_return_value"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added this "example" into your integration tests folder @HCastano, because already all the CI scripts and waterfall tests deal with this special case folder. Perhaps we could introduce an integration-tests folder and have feature specific integration tests in there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we could introduce an integration-tests folder and have feature specific integration tests in there.

Yep, I was thinking the same thing

Copy link
Contributor

@SkymanOne SkymanOne left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Good stuff!

Comment on lines +27 to +33
fn generate_metadata() -> InkProject {
extern "Rust" {
fn __ink_generate_metadata() -> InkProject;
}

unsafe { __ink_generate_metadata() }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

haxxor

unsafe { __ink_generate_metadata() }
}

fn main() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Really nice test 👍

@HCastano HCastano merged commit 300b6c4 into master Nov 17, 2022
@HCastano HCastano deleted the aj/constructor-return-type branch November 17, 2022 21:04
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.

Fix and refactor fallible constructors
3 participants