Conversation
| m_nonPayablePublic = _function.isPublic() && !_function.isPayable(); | ||
| m_constructor = _function.isConstructor(); | ||
| if (_function.stateMutability() == StateMutability::Pure) | ||
| m_errorReporter.warning(_function.location(), "Function is marked pure. Be careful, pureness is not enforced yet."); |
There was a problem hiding this comment.
I think this is the safe way to introduce it unless the call graph analyser is finished.
axic
force-pushed
the
statemutability-pure
branch
4 times, most recently
from
c986134 to
5ae7d0e
Compare
|
Need to add tests for pure functions and function types. |
| Features: | ||
| * ABI JSON: Include new field ``statemutability`` with values ``view``, ``nonpayable`` and ``payable``. | ||
| * Introduce ``pure`` functions. The pureness is not enforced yet, use with care. | ||
| * ABI JSON: Include new field ``statemutability`` with values ``pure``, ``view``, ``nonpayable`` and ``payable``. |
There was a problem hiding this comment.
any chance of making statemutability: state_mutability? Pure aesthetics I know. But might make it easier to read. Atleast a mixedCase or - of some kind.
There was a problem hiding this comment.
There's no prior example in the ABI so not sure which way to go. (However this was merged in a different PR so please open an issue/PR to solve it.)
axic
force-pushed
the
statemutability-pure
branch
from
5ae7d0e to
6959fe5
Compare
libsolidity/ast/ASTJsonConverter.cpp
Outdated
| make_pair( | ||
| m_legacy ? "constant" : "isDeclaredConst", | ||
| _node.stateMutability() == StateMutability::View || | ||
| _node.stateMutability() == StateMutability::Pure |
There was a problem hiding this comment.
Actually I don' think this should contain pure as it is only for the AST. By using this AST one couldn't reliable rebuild the source.
There was a problem hiding this comment.
Not sure I understand. constant / isDeclaredConst is used by static analyzers and I would like to break them rather later than sooner. We have an additional field that provides the state mutability, so all the information is there.
There was a problem hiding this comment.
While I still don't agree it belongs to isDeclaredConstant, because that refers to the constant specifier, I can understand the reasoning for static analyzers not yet aware of the statemutability field. Ultimately, removing the obsolete field will solve this.
libsolidity/ast/ASTPrinter.cpp
Outdated
| (_node.isPublic() ? " - public" : "") + | ||
| ( | ||
| _node.stateMutability() == StateMutability::View || | ||
| _node.stateMutability() == StateMutability::Pure ? |
There was a problem hiding this comment.
Same applies here as for the AST JSON.
There was a problem hiding this comment.
But the AST printer should at least contain the information about state mutability.
axic
force-pushed
the
statemutability-pure
branch
3 times, most recently
from
4ba6558 to
5cf2997
Compare
axic
changed the title
| Pure Functions | ||
| ************** | ||
|
|
||
| Functions can be declared ``pure`` in which case they promise not to read from or modify the state. |
There was a problem hiding this comment.
Perhaps we should explain in more detail what reading from or writing to the state means.
There was a problem hiding this comment.
We can also postpone this explanation so that we do not delay the release.
There was a problem hiding this comment.
Can we move it to #2792? Do you want to list all the disallowed things?
There was a problem hiding this comment.
Yes, we can move it to #2792. We should list as much as people need to understand.
axic
force-pushed
the
statemutability-pure
branch
from
5cf2997 to
6c0f21e
Compare
|
I think there are some places where Also ASTPrinter sholud be updated for completeness. |
|
I've thought I've changed the ABI (seems like it was gone through some wrong conflict resolution in rebasing), but explained above why I don't think it applies to ASTJsonConverter and ASTPrinter in #2745 (comment). |
axic
force-pushed
the
statemutability-pure
branch
from
6c0f21e to
75d35ab
Compare
axic
force-pushed
the
statemutability-pure
branch
from
75d35ab to
f646247
Compare
|
@chriseth should be ready |
3 participants
Part of #992. Depends on #2754 and #2762.