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

Refactor constructor annotations #1366

Merged
merged 4 commits into from
Jun 22, 2023
Merged

Conversation

LucasSte
Copy link
Contributor

@LucasSte LucasSte commented Jun 14, 2023

Presently, we can declare an account on a Solana constructor using the @payer(my_account) annotation, while @seed(my_seed) refers to a constructor parameter. Such a construction is confusing for the same syntax have two different meanings.

This PR limits the scope of annotation above a constructor to accept only literals as parameters (e.g. @seed("pine_tree")). Annotations that refer to function parameters must appear before them: constructor (@seed bytes arg1). This is another in #1251.

@LucasSte LucasSte force-pushed the syntax-change branch 2 times, most recently from e19435a to b939c24 Compare June 15, 2023 13:02
@LucasSte LucasSte marked this pull request as ready for review June 15, 2023 14:18
@LucasSte LucasSte added this to the Prepare for Solana Hackathon milestone Jun 15, 2023
Copy link
Contributor

@seanyoung seanyoung left a comment

Choose a reason for hiding this comment

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

Looks good. I've checked and I think you have all the cases covered

docs/language/contracts.rst Outdated Show resolved Hide resolved
@seanyoung
Copy link
Contributor

There are no testcases with whitespace between @ and the identifier. I am not sure how we can disallow whitespace there (and if we want to)

Copy link
Contributor

@xermicus xermicus left a comment

Choose a reason for hiding this comment

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

LGTM

@LucasSte
Copy link
Contributor Author

There are no testcases with whitespace between @ and the identifier. I am not sure how we can disallow whitespace there (and if we want to)

The entire annotation is now a token. There is no way of declaring an annotation with a whitespace between the @ and the identifier. I've added a test for this case as well.

Signed-off-by: Lucas Steuernagel <[email protected]>
Signed-off-by: Lucas Steuernagel <[email protected]>
Signed-off-by: Lucas Steuernagel <[email protected]>
docs/targets/solana.rst Outdated Show resolved Hide resolved
docs/targets/solana.rst Outdated Show resolved Hide resolved
docs/targets/solana.rst Outdated Show resolved Hide resolved
solang-parser/src/pt.rs Outdated Show resolved Hide resolved
@LucasSte LucasSte merged commit 256b76e into hyperledger:main Jun 22, 2023
17 checks passed
@LucasSte LucasSte deleted the syntax-change branch June 22, 2023 15:08
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