-
Notifications
You must be signed in to change notification settings - Fork 68
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
feat: support natspec annotations #133
Conversation
@@ -887,9 +700,18 @@ def run_sequential(run_args: RunArgs) -> List[TestResult]: | |||
return test_results | |||
|
|||
|
|||
def extend_args(args: Namespace, more_opts: str) -> Namespace: | |||
if more_opts: | |||
new_args = deepcopy(args) |
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.
I was looking for something to avoid the need for a deepcopy here, I ended up with this:
import argparse
import collections
parser = argparse.ArgumentParser()
parser.add_argument('--foo')
parser.add_argument('--bar')
parser.add_argument('--loop')
def dictify(args):
return {k: v for k, v in vars(args).items() if v is not None}
defaults = parser.parse_args(['--foo', 'beep', '--bar', 'boop'])
overrides = parser.parse_args(['--foo', 'ding', '--loop', '42'])
combined = collections.ChainMap(dictify(overrides), dictify(defaults))
print(combined['foo']) # 'ding' comes from overrides, takes precedence over defaults
print(combined['bar']) # 'boop' comes from defaults
print(combined['loop']) # '42' comes from overrides, not specified in defaults
ChainMap is cool but in the end it's not necessarily better because there is still some copying involved because of a poor interaction with Namespace. Might make sense to write our own NamespaceChain
-like wrapper that avoids any copying at all at a later point
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.
do you suggest using chainmap now, or doing something better later?
i wasn't super happy with using deepcopy here, but thought that the namespace object should be small enough and deepcopy wouldn't affect performance much. are there any other issues that i missed?
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.
no it's just a general dislike of deepcopy creep. Disregard for now
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.
this is great, can't wait to start using it
the external test failures are independent from this pr. it will be fixed by #134. will merge this pr after merging that. |
fix #131
Support
@custom:halmos
natspec annotation to specify additional commandline options.The natspec for a contract is inherited by every function in the contract. The natspec for a function can overwrite the inherited natspec.
For example, given:
The options for the
checkA()
function are:--opt1 --opt2 --loop 2
Note that it inherits
--opt1
but overwrites--loop 1
.--
The halmos natspec can be provided in multiline:
or, using multiple tags: