-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Warn about unreachable code. #5765
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
Conversation
ea296b7
to
db8d785
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I guess this logic does not detect that e.g. a function always reverts, and thus calls to that function will have similar semantics to |
Another question to discuss: Do we want to make this an error? |
Function calls are basically ignored in the CFG so far, but yes, we can change that in the future (out of scope for now I would say, though). |
I think we could make this an error, if we want to. But to me it seems a warning would suffice... we could remove the dead code ourselves during optimization. |
Nice! |
33d9734
to
7d4f6ef
Compare
7d4f6ef
to
d859f44
Compare
Should be squashed before merging. |
This comment has been minimized.
This comment has been minimized.
de1d5b1
to
1f54cc0
Compare
This comment has been minimized.
This comment has been minimized.
1f54cc0
to
b6d04a4
Compare
This comment has been minimized.
This comment has been minimized.
b6d04a4
to
8552512
Compare
This comment has been minimized.
This comment has been minimized.
8552512
to
6f3d810
Compare
fee0a2b
to
3faf44f
Compare
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.
Fine apart from the small changes. Also, should this be squashed to a single commit?
3faf44f
to
d3d394a
Compare
Tried to address the small changes and squashed to one commit.
d3d394a
to
0dfd4a7
Compare
Closes #2340.
This still does not consider constant conditions (e.g.
if (false) ...
). I would only add support for that, once we have a full solidity-level compile-time constant expression evaluator, though (which at least I would say we should have).