Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add flip for changing import statement semantics #277
Add flip for changing import statement semantics #277
Changes from 1 commit
5ba4d0c
9bac4dc
bfd15e8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So code that would need to be changed here is if you directly use a imported contracts fields and mutate them right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but in addition, if you use a field of a imported inside a function (say for example return as is), then the type annotation would change.
e.g:
then this would change to
(This is also the change needed for those three contracts that breaks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I think this is very dangerous. Cadence-1.0 is already breaking so this is just one more thing that we need to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is obviously beyond the scope of this current FLIP (and would be a new feature that can be added after Cadence 1.0), but I think that changing imports to create an unentitled reference to the contract value does suggest that it may be useful to also be able to import an entitled reference to the contract as well. There would need to be some kind of way to guarantee that only certain people can do this, and syntax for it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think easiest solution is to disallow entitlements on contract functions. ( which can be relaxed later if needed )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I double checked this, and entitled contract members are already forbidden: https://github.com/onflow/cadence/blob/a1199d656c222a1253fdb362c20bd05f7c310287/runtime/tests/checker/entitlements_test.go#L1673-L1686