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

Add (and require) keyword for the default state mutability of functions #9248

Closed
chriseth opened this issue Jun 23, 2020 · 16 comments
Closed
Labels
breaking change ⚠️ closed due inactivity The issue/PR was automatically closed due to inactivity. language design :rage4: Any changes to the language, e.g. new features stale The issue/PR was marked as stale because it has been open for too long.

Comments

@chriseth
Copy link
Contributor

chriseth commented Jun 23, 2020

Proposals:

  • mutating
  • mutable
  • mutator
  • mut
  • impure
  • write
  • touch
  • default
  • nonpayable
  • unrestricted
  • !

Reason for requiring the keyword:

  • it is easier to see at a glance what the state mutability of the function is and it is possible to search for it

cc @MicahZoltu

@chriseth chriseth added the language design :rage4: Any changes to the language, e.g. new features label Jun 23, 2020
@hrkrshnn
Copy link
Member

hrkrshnn commented Jun 23, 2020

"mutable" might be more appropriate than "mutating". Looks more appropriate in a sentence. The function foo is mutating v/s The function foo is mutable.

@chriseth
Copy link
Contributor Author

@hrkrshnn I think I disagree - the function is mutating the contract's state, it is not that the function itself is modified.

@hrkrshnn
Copy link
Member

@chriseth I think 'mutating' doesn't fit with the existing tags: pure, view, payable. It would fit better if it was 'viewing' instead of 'view'.

Maybe 'mutable' isn't a good replacement, since it has a different meaning in other languages.

@MicahZoltu
Copy link
Contributor

It would be nice if the form of the word was of similar form with pure/view/payable. mutable does seem to align better with payable. I'm not actually sure how to use any in a sentence though without leaning on my existing understanding of their definitions!

@chriseth
Copy link
Contributor Author

We could also use mut...

@MicahZoltu
Copy link
Contributor

Ugh, while I can appreciate that it would resolve the requirements I mentioned (something that can be searched for and is immediately visible), I have a pretty strong disdain for abbreviations in code. 😖

@hrkrshnn
Copy link
Member

Another suggestion: impure.

@MicahZoltu
Copy link
Contributor

write? touch?

@cameel
Copy link
Member

cameel commented Jun 23, 2020

Another option: in Ruby names can contain characters like ! and ? and the convention for methods that modify the state of the object is to add ! to the name: C.append! x .
? is used for methods that return boolean values (e.g empty?). The convention originally comes from Scheme.

We could also require the object to be an explicit parameter: C.append(this, x) or C.append(self, x). This is how bound library methods already work. If we ever make arguments immutable by default (#715), distinguishing between methods that modify the object and ones that don't would be a natural extension: function append(C mutable this, uint x).

If we want a keyword though - what about mutator?
I think that mutable kind of works too - it's used this way e.g. for c++ lambdas. Though mutating is clearer.

@chriseth
Copy link
Contributor Author

Idea that came up in the design meeting: make the default pure and add a new keyword for the previously default behaviour

@MicahZoltu
Copy link
Contributor

I would still prefer explicit labels for everything, though if you are going to default then I think defaulting to pure is significantly better than defaulting to mutable (or whatever we end up calling it).

@leonardoalt
Copy link
Member

I agree with pure as default

@axic
Copy link
Member

axic commented Oct 21, 2020

We should perhaps also consider #3221 under the same discussion.

@axic
Copy link
Member

axic commented Oct 21, 2020

I think pure, view, nonpayable/mutable/mutating, payable is the order we should have and not consider any of them implicitly.

nonpayable is a bit too long, mutating implies an active role in making changes, while mutable is more passive. Additionally mutable suggest the function may be mutable, i.e. replaceable so probably not a good idea.

Useless blurb: Initially I was going to argue for mutating because a mutating function is known to be actively using opcodes which cause state changes, while in the case of payable it depends on inputs (whether there is a value or not). However, mutating functions can also be non-mutating depending on external input.


We could also consider moving to a different system, for example the following: Consider everything pure by default and introduce a new field/modifier called state and let it have multiple flags: read, write, payable.

Examples:

  • function f() state(read) {}
  • function f() state(write) {}
  • function f() state(read,write) {}
  • function f() state(read,write,payable) {}
  • function f() state(payable,selfdestruct) {}

Arguable as we have established some of these states imply others, such as it would be unrealistic to have a payable function which does not reads or writes -- albeit possible which does not "writes" if we assume balance changes not caused by the contract do not mean "writing".

@k06a
Copy link

k06a commented Nov 30, 2020

Wish to add more flags:

  • call
  • viewcall
  • delegatecall
  • codecall
  • create

@NunoFilipeSantos NunoFilipeSantos added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 25, 2022
@github-actions
Copy link

github-actions bot commented Feb 4, 2023

Hi everyone! This issue has been closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Feb 4, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ closed due inactivity The issue/PR was automatically closed due to inactivity. language design :rage4: Any changes to the language, e.g. new features stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Development

No branches or pull requests

8 participants