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.

Sign up for GitHub

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

Jump to bottom

Replace the impure keyword with #[storage(...)] annotations. #1518

Merged
merged 4 commits into from
May 19, 2022
Merged

Replace the impure keyword with #[storage(...)] annotations. #1518

merged 4 commits into from
May 19, 2022

Conversation

otrho
Copy link
Contributor

@otrho otrho commented May 11, 2022 •
edited
Loading

Closes #1447.
Closes #470. Except for the chain bit. Do we want a new issue for chain attributes?

@otrho otrho added enhancement New feature or request compiler General compiler. Should eventually become more specific as the issue is triaged labels May 11, 2022
@otrho otrho self-assigned this May 11, 2022
@otrho otrho force-pushed the otrho/replace_purity_keywords branch from f85c688 to d16e680 Compare May 11, 2022 03:26
adlerjohn
adlerjohn reviewed May 11, 2022
View reviewed changes
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

Can you also add documentation to The Sway Book?

sway-core/src/parse_tree/declaration/function.rs Outdated Show resolved Hide resolved
sway-core/src/error.rs Outdated Show resolved Hide resolved
@otrho
Copy link
Contributor Author

otrho commented May 12, 2022

Have updated the docs though I'm not sure the terms I've used are OK. Like using 'privilege'. And maybe the page needs more structure, it only has a single section.

adlerjohn
adlerjohn reviewed May 12, 2022
View reviewed changes
docs/src/blockchain-development/purity.md Outdated Show resolved Hide resolved
docs/src/blockchain-development/purity.md Outdated Show resolved Hide resolved
docs/src/blockchain-development/purity.md Show resolved Hide resolved
docs/src/blockchain-development/purity.md Outdated Show resolved Hide resolved
@otrho otrho force-pushed the otrho/replace_purity_keywords branch from f58436b to 8fa2f3e Compare May 13, 2022 01:16
@otrho otrho mentioned this pull request May 13, 2022
Closed
@otrho otrho linked an issue May 13, 2022 that may be closed by this pull request
AST item annotation system (pure/read/write/chain for storage access) #470
Closed
@canndrew
Copy link
Contributor

canndrew commented May 16, 2022 •
edited
Loading

The parsing side of this looks okay to me. We should really have a Spanned trait that we can use rather than having a macro for adding a span method to Annotated<$ty_name> but that's something that can be done in a follow-up PR.

Edit: I went and wrote a PR for this: #1559. So maybe merge that first or just use those commits in this PR.

@canndrew canndrew mentioned this pull request May 16, 2022
Merged
@otrho otrho marked this pull request as draft May 16, 2022 12:50
@otrho otrho force-pushed the otrho/replace_purity_keywords branch from 92696c9 to 55346c9 Compare May 18, 2022 12:35
@otrho otrho marked this pull request as ready for review May 18, 2022 12:51
@mohammadfawaz mohammadfawaz mentioned this pull request May 18, 2022
Merged
8 tasks
mohammadfawaz
mohammadfawaz reviewed May 18, 2022
View reviewed changes
Copy link
Contributor

@mohammadfawaz mohammadfawaz left a comment

Choose a reason for hiding this comment

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

LGTM. I'll let @sezna make the final approval.

sezna
sezna previously approved these changes May 19, 2022
View reviewed changes
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

lgtm, love the thorough testing.

@adlerjohn adlerjohn requested a review from sezna May 19, 2022 04:11
mitchmindtree
mitchmindtree approved these changes May 19, 2022
View reviewed changes
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Ugh, that's the last time I try to resolve a merge conflict in the GitHub web editor. :(

😂

@otrho otrho merged commit b1c9514 into master May 19, 2022
@otrho otrho deleted the otrho/replace_purity_keywords branch May 19, 2022 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mohammadfawaz mohammadfawaz mohammadfawaz left review comments

@adlerjohn adlerjohn adlerjohn left review comments

@mitchmindtree mitchmindtree mitchmindtree approved these changes

@canndrew canndrew Awaiting requested review from canndrew

@emilyaherbert emilyaherbert Awaiting requested review from emilyaherbert

@sezna sezna Awaiting requested review from sezna

Assignees

@otrho otrho

Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
Fuel Network
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove impure keyword AST item annotation system (pure/read/write/chain for storage access)
6 participants
@otrho @canndrew @adlerjohn @mitchmindtree @sezna @mohammadfawaz