Conversation
046f830 to
5dd3058
Compare
4b8159f to
b4ba6d1
Compare
201bfc1 to
4e0438e
Compare
b4ba6d1 to
ae5fa34
Compare
4174c6a to
c621909
Compare
mijovic
left a comment
There was a problem hiding this comment.
Few small comments and suggestions for first round.
Also this needs to be rebased again, as there are some commits from other PRs
51857d7 to
728bc1b
Compare
728bc1b to
69d6b62
Compare
cameel
left a comment
There was a problem hiding this comment.
Only minor suggestions. I haven't found anything serious.
ae5fa34 to
1aad33d
Compare
69d6b62 to
d4421a4
Compare
8184bea to
4438a59
Compare
d4421a4 to
7d7b6b8
Compare
4438a59 to
d0cec87
Compare
59523fa to
5a033f0
Compare
| } | ||
|
|
||
| if (noErrors) | ||
| { |
There was a problem hiding this comment.
I think brackets are not needed, but not mandatory to change
|
Can you also add this test: should succeed (already does) because |
|
It actually fails :( |
5a033f0 to
ea0e693
Compare
|
Added the test as |
| // edges to all functions could possibly be called. | ||
| add(m_currentNode, CallGraph::SpecialNode::InternalDispatch); | ||
| else if (functionType->kind() == FunctionType::Kind::Error) | ||
| m_graph.usedErrors.insert(&dynamic_cast<ErrorDefinition const&>(functionType->declaration())); |
There was a problem hiding this comment.
maybe we should add an assert here?
There was a problem hiding this comment.
You mean as else? Not really. There are other case possible here, the graph just skips them.
There was a problem hiding this comment.
There is a check and I would say it is highly unlikely for this assert to trigger...
produces an ICE: But the following works! |
| contract C { | ||
| event E(); | ||
| function f() public pure { | ||
| revert E(); |
There was a problem hiding this comment.
I was wondering why pure functions are allowed to revert. I think that should be forbidden.
There was a problem hiding this comment.
This has been discussed a lot in the past. Pure means the result only depends on the arguments, and that is the case here.
|
@hrkrshnn oh wow! Good that you found this! I could swear the code for that was there before... |
ea0e693 to
002f79f
Compare
|
@hrkrshnn added the check to TypeChecker and added tests. |
d0cec87 to
b04b189
Compare
002f79f to
d80059f
Compare
The base branch was changed.
It was introduced alongside the `revert` contextual keyword, see argotorg/solidity#11037.
The statement was introduced alongside the `revert` contextual keyword, see argotorg/solidity#11037.
No description provided.