John Snow <[email protected]> writes:
> On 1/20/21 9:20 AM, Markus Armbruster wrote:
>> John Snow <[email protected]> writes:
[...]
>>> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
>>> index 55acd7e080d..b5505685e6e 100644
>>> --- a/scripts/qapi/gen.py
>>> +++ b/scripts/qapi/gen.py
[...]
>>> @@ -313,7 +306,8 @@ def visit_module(self, module: QAPISchemaModule) ->
>>> None:
>>> self._genc = None
>>> self._genh = None
>>> else:
>>> - self._add_user_module(module.name, self._user_blurb)
>>> + assert module.user_module, "Unexpected system module"
>> The string provides no value.
>>
>
> That's just, like, your opinion, man. It has value to me.
>
>
> Compare:
>
> ```
> #!/usr/bin/env python3
>
> def main():
> assert False
>
> if __name__ == '__main__':
> main()
> ```
>
> ```
> # ./foo.py
>
> Traceback (most recent call last):
> File "./foo.py", line 7, in <module>
> main()
> File "./foo.py", line 4, in main
> assert False
> AssertionError
> ```
>
> With:
>
>
> ```
> #!/usr/bin/env python3
>
> def main():
> assert False, "Unexpected system module"
>
> if __name__ == '__main__':
> main()
> ```
>
> ```
> # ./foo.py
>
> Traceback (most recent call last):
> File "./foo.py", line 7, in <module>
> main()
> File "./foo.py", line 4, in main
> assert False, "Unexpected system module"
> AssertionError: Unexpected system module
> ```
>
> I like the extra string for giving some semantic context as to
> specifically what broke (We don't expect to see system modules here)
> instead of just a stack trace.
Your test uses assert with an argument that tells us nothing. But the
assert we're arguing about has a nice, expressive argument.
The string improves the report from
File "/work/armbru/qemu/scripts/qapi/gen.py", line 325, in visit_module
assert module.user_module
AssertionError
to
File "/work/armbru/qemu/scripts/qapi/gen.py", line 325, in visit_module
assert module.user_module, "Unexpected system module"
AssertionError: Unexpected system module
Even if you value the difference, I very much doubt it justifies the
clutter. Also, slippery slope towards pigs wearing way too much
lipstick.
Tested with
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index f3f4d2b011..bbfb30dc5a 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -321,6 +321,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
# be generated.
self._current_module = None
else:
+ module.name = "./HACK"
assert module.user_module, "Unexpected system module"
self._add_module(module.name, self._user_blurb)
self._begin_user_module(module.name)
>
>>> + self._add_module(module.name, self._user_blurb)
>>> self._begin_user_module(module.name)
>>> def visit_include(self, name: str, info: QAPISourceInfo) ->
>>> None:
