perf: early exit for fail() #322
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
daejunpark
commented
Jul 16, 2024
src/halmos/exceptions.py
Outdated
| @@ -20,6 +20,22 @@ class NotConcreteError(HalmosException): | |||
| pass | |||
|
|
|||
|
|
|||
| class HalmosTargetReached(Exception): | |||
There was a problem hiding this comment.
please feel free to suggest a better name
There was a problem hiding this comment.
it sounds a little too semantically charged for me, I would go for something more descriptive like RunEndingException(Exception), and I would make both HalmosException and FailCheatcode inherit from RunEndingException:
class RunEndingException(Exception):
"""
Base class for any exception that should not just stop the current EVM context, instead aborting the current test run.
"""
pass
class HalmosException(RunEndingException):
"""
Base class for unexpected internal errors happening during a test run.
Inherits from RunEndingException because it should not just stop the current EVM context, but stop the whole run.
"""
pass
class FailCheatcode(RunEndingException):
"""
Raised when invoking DSTest's fail() pseudo-cheatcode.
Inherits from RunEndingException because it should not just stop the current EVM context, but stop the whole run.
"""
passThere was a problem hiding this comment.
some alternatives along the same idea:
- AbortCallException (not great,
Callis ambiguous there) - AbortTestException
- TestStopException
- RunHaltException (
Haltis neat) - HaltAndCatchFireException if you don't mind a fun reference 😄
There was a problem hiding this comment.
thanks for suggestion! i ended up with PathEndingException, because "run" is ambiguous with the entire test run.
| fail(); | ||
| } | ||
|
|
||
| function check_early_fail_cheatcode(uint x) public { |
karmacoma-eth
approved these changes
Jul 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fix fail() to exit early.
previously, fail() was treated as a normal evm exception, allowing execution to continue after returning to the caller if any. (see the new test as example.) such continued execution is unnecessary. now fail() exits immediately, avoiding potential performance degradation.