diff mbox series

[06/37] qapi: delint using flake8

Message ID 20200915224027.2529813-7-jsnow@redhat.com
State Superseded
Headers show
Series qapi: static typing conversion, pt1 | expand

Commit Message

John Snow Sept. 15, 2020, 10:39 p.m. UTC
Petty style guide fixes and line length enforcement.  Not a big win, not
a big loss, but flake8 passes 100% on the qapi module, which gives us an
easy baseline to enforce hereafter.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/.flake8     |  2 ++
 scripts/qapi/commands.py |  3 ++-
 scripts/qapi/schema.py   |  8 +++++---
 scripts/qapi/visit.py    | 15 ++++++++++-----
 4 files changed, 19 insertions(+), 9 deletions(-)
 create mode 100644 scripts/qapi/.flake8

Comments

Markus Armbruster Sept. 16, 2020, 12:12 p.m. UTC | #1
John Snow <jsnow@redhat.com> writes:

> Petty style guide fixes and line length enforcement.  Not a big win, not

> a big loss, but flake8 passes 100% on the qapi module, which gives us an

> easy baseline to enforce hereafter.

>

> Signed-off-by: John Snow <jsnow@redhat.com>

> ---

>  scripts/qapi/.flake8     |  2 ++

>  scripts/qapi/commands.py |  3 ++-

>  scripts/qapi/schema.py   |  8 +++++---

>  scripts/qapi/visit.py    | 15 ++++++++++-----

>  4 files changed, 19 insertions(+), 9 deletions(-)

>  create mode 100644 scripts/qapi/.flake8

>

> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8

> new file mode 100644

> index 0000000000..45d8146f3f

> --- /dev/null

> +++ b/scripts/qapi/.flake8

> @@ -0,0 +1,2 @@

> +[flake8]

> +extend-ignore = E722  # Pylint handles this, but smarter.


I guess you mean pylint's W0702 a.k.a. bare-except.  What's wrong with
flake8's E722 compared to pylint's W0702?

> \ No newline at end of file


So put one there :)

> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py

> index e1df0e341f..2e4b4de0fa 100644

> --- a/scripts/qapi/commands.py

> +++ b/scripts/qapi/commands.py

> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):

>  def gen_marshal_output(ret_type):

>      return mcgen('''

>  

> -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)

> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,

> +                                          Error **errp)


The continued parameter list may become misalignd in generated C.  E.g.

    static void qmp_marshal_output_BlockInfoList(BlockInfoList *ret_in, QObject **ret_out,
                                              Error **errp)
    {
    ...
    }

Do we care?

More of the same below.

>  {

>      Visitor *v;

>  

[...]
John Snow Sept. 16, 2020, 2:29 p.m. UTC | #2
On 9/16/20 8:12 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:

> 

>> Petty style guide fixes and line length enforcement.  Not a big win, not

>> a big loss, but flake8 passes 100% on the qapi module, which gives us an

>> easy baseline to enforce hereafter.

>>

>> Signed-off-by: John Snow <jsnow@redhat.com>

>> ---

>>   scripts/qapi/.flake8     |  2 ++

>>   scripts/qapi/commands.py |  3 ++-

>>   scripts/qapi/schema.py   |  8 +++++---

>>   scripts/qapi/visit.py    | 15 ++++++++++-----

>>   4 files changed, 19 insertions(+), 9 deletions(-)

>>   create mode 100644 scripts/qapi/.flake8

>>

>> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8

>> new file mode 100644

>> index 0000000000..45d8146f3f

>> --- /dev/null

>> +++ b/scripts/qapi/.flake8

>> @@ -0,0 +1,2 @@

>> +[flake8]

>> +extend-ignore = E722  # Pylint handles this, but smarter.

> 

> I guess you mean pylint's W0702 a.k.a. bare-except.  What's wrong with

> flake8's E722 compared to pylint's W0702?

> 


Flake8 will warn on *any* bare except, but Pylint's is context-aware and 
will suppress the warning if you re-raise the exception.

I don't actually think this comes up in the qapi code base, but it does 
come up in the ./python/qemu code base.

(One of my goals is unifying the lint checking regime for both packages.)

>> \ No newline at end of file

> 

> So put one there :)

> 


Whupps, okay.

>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py

>> index e1df0e341f..2e4b4de0fa 100644

>> --- a/scripts/qapi/commands.py

>> +++ b/scripts/qapi/commands.py

>> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):

>>   def gen_marshal_output(ret_type):

>>       return mcgen('''

>>   

>> -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)

>> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,

>> +                                          Error **errp)

> 

> The continued parameter list may become misalignd in generated C.  E.g.

> 

>      static void qmp_marshal_output_BlockInfoList(BlockInfoList *ret_in, QObject **ret_out,

>                                                Error **errp)

>      {

>      ...

>      }

> 

> Do we care?

> 


Yeah, I don't know. Do we?

It actually seemed more annoying to try and get flake8 to make an 
exception for these handful of examples.

Path of least resistance led me here, but I can try and appease both 
systems if you'd prefer.

> More of the same below.

> 

>>   {

>>       Visitor *v;

>>   

> [...]

>
Markus Armbruster Sept. 17, 2020, 7:54 a.m. UTC | #3
John Snow <jsnow@redhat.com> writes:

> On 9/16/20 8:12 AM, Markus Armbruster wrote:

>> John Snow <jsnow@redhat.com> writes:

>> 

>>> Petty style guide fixes and line length enforcement.  Not a big win, not

>>> a big loss, but flake8 passes 100% on the qapi module, which gives us an

>>> easy baseline to enforce hereafter.

>>>

>>> Signed-off-by: John Snow <jsnow@redhat.com>

>>> ---

>>>   scripts/qapi/.flake8     |  2 ++

>>>   scripts/qapi/commands.py |  3 ++-

>>>   scripts/qapi/schema.py   |  8 +++++---

>>>   scripts/qapi/visit.py    | 15 ++++++++++-----

>>>   4 files changed, 19 insertions(+), 9 deletions(-)

>>>   create mode 100644 scripts/qapi/.flake8

>>>

>>> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8

>>> new file mode 100644

>>> index 0000000000..45d8146f3f

>>> --- /dev/null

>>> +++ b/scripts/qapi/.flake8

>>> @@ -0,0 +1,2 @@

>>> +[flake8]

>>> +extend-ignore = E722  # Pylint handles this, but smarter.

>> I guess you mean pylint's W0702 a.k.a. bare-except.  What's wrong

>> with

>> flake8's E722 compared to pylint's W0702?

>> 

>

> Flake8 will warn on *any* bare except, but Pylint's is context-aware

> and will suppress the warning if you re-raise the exception.


Should this information be worked into the comment?

> I don't actually think this comes up in the qapi code base, but it

> does come up in the ./python/qemu code base.

>

> (One of my goals is unifying the lint checking regime for both packages.)

>

>>> \ No newline at end of file

>> So put one there :)

>> 

>

> Whupps, okay.

>

>>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py

>>> index e1df0e341f..2e4b4de0fa 100644

>>> --- a/scripts/qapi/commands.py

>>> +++ b/scripts/qapi/commands.py

>>> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):

>>>   def gen_marshal_output(ret_type):

>>>       return mcgen('''

>>>   -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,

>>> QObject **ret_out, Error **errp)

>>> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,

>>> +                                          Error **errp)

>> The continued parameter list may become misalignd in generated C.

>> E.g.

>>      static void qmp_marshal_output_BlockInfoList(BlockInfoList

>> *ret_in, QObject **ret_out,

>>                                                Error **errp)

>>      {

>>      ...

>>      }

>> Do we care?

>> 

>

> Yeah, I don't know. Do we?


I care, but I also care for automated style checks.

> It actually seemed more annoying to try and get flake8 to make an

> exception for these handful of examples.

>

> Path of least resistance led me here, but I can try and appease both

> systems if you'd prefer.


Up to now, I ran the style checkers manually, and this was just one of
several complaints to ignore, so I left the code alone.

If it gets in the way of running them automatically, and messing up the
generated code slightly is the easiest way to get it out of the way,
then I can accept the slight mess.

>> More of the same below.

>> 

>>>   {

>>>       Visitor *v;

>>>   

>> [...]

>>
John Snow Sept. 17, 2020, 4:57 p.m. UTC | #4
On 9/17/20 3:54 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 9/16/20 8:12 AM, Markus Armbruster wrote:
>>> John Snow <jsnow@redhat.com> writes:
>>>
>>>> Petty style guide fixes and line length enforcement.  Not a big win, not
>>>> a big loss, but flake8 passes 100% on the qapi module, which gives us an
>>>> easy baseline to enforce hereafter.
>>>>
>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>> ---
>>>>    scripts/qapi/.flake8     |  2 ++
>>>>    scripts/qapi/commands.py |  3 ++-
>>>>    scripts/qapi/schema.py   |  8 +++++---
>>>>    scripts/qapi/visit.py    | 15 ++++++++++-----
>>>>    4 files changed, 19 insertions(+), 9 deletions(-)
>>>>    create mode 100644 scripts/qapi/.flake8
>>>>
>>>> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
>>>> new file mode 100644
>>>> index 0000000000..45d8146f3f
>>>> --- /dev/null
>>>> +++ b/scripts/qapi/.flake8
>>>> @@ -0,0 +1,2 @@
>>>> +[flake8]
>>>> +extend-ignore = E722  # Pylint handles this, but smarter.
>>> I guess you mean pylint's W0702 a.k.a. bare-except.  What's wrong
>>> with
>>> flake8's E722 compared to pylint's W0702?
>>>
>>
>> Flake8 will warn on *any* bare except, but Pylint's is context-aware
>> and will suppress the warning if you re-raise the exception.
> 
> Should this information be worked into the comment?
> 

I'll improve it a little, but I'll add what I wrote above to the commit 
message.

>> I don't actually think this comes up in the qapi code base, but it
>> does come up in the ./python/qemu code base.
>>
>> (One of my goals is unifying the lint checking regime for both packages.)
>>
>>>> \ No newline at end of file
>>> So put one there :)
>>>
>>
>> Whupps, okay.
>>
>>>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
>>>> index e1df0e341f..2e4b4de0fa 100644
>>>> --- a/scripts/qapi/commands.py
>>>> +++ b/scripts/qapi/commands.py
>>>> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):
>>>>    def gen_marshal_output(ret_type):
>>>>        return mcgen('''
>>>>    -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,
>>>> QObject **ret_out, Error **errp)
>>>> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
>>>> +                                          Error **errp)
>>> The continued parameter list may become misalignd in generated C.
>>> E.g.
>>>       static void qmp_marshal_output_BlockInfoList(BlockInfoList
>>> *ret_in, QObject **ret_out,
>>>                                                 Error **errp)
>>>       {
>>>       ...
>>>       }
>>> Do we care?
>>>
>>
>> Yeah, I don't know. Do we?
> 
> I care, but I also care for automated style checks.
> 
>> It actually seemed more annoying to try and get flake8 to make an
>> exception for these handful of examples.
>>
>> Path of least resistance led me here, but I can try and appease both
>> systems if you'd prefer.
> 
> Up to now, I ran the style checkers manually, and this was just one of
> several complaints to ignore, so I left the code alone.
> 
> If it gets in the way of running them automatically, and messing up the
> generated code slightly is the easiest way to get it out of the way,
> then I can accept the slight mess.
> 

I changed this a little to put all the args on the next line, which is 
slightly unusual but works okay.

I think that's a fine middle ground, because the alternative (to me) is 
to start using abstracted code generation tokens in a tree structure, 
etc etc etc.

Embedded templates are always gonna look kinda nasty, I think, because 
you're trying to fight style guidelines in two languages simultaneously 
and it's never gonna quite work out exactly how you want without some 
pretty complex abstraction mechanisms that are well beyond the power we 
need right now.

>>> More of the same below.
>>>
>>>>    {
>>>>        Visitor *v;
>>>>    
>>> [...]
>>>
Markus Armbruster Sept. 18, 2020, 10:33 a.m. UTC | #5
John Snow <jsnow@redhat.com> writes:

> On 9/17/20 3:54 AM, Markus Armbruster wrote:

>> John Snow <jsnow@redhat.com> writes:

>> 

>>> On 9/16/20 8:12 AM, Markus Armbruster wrote:

>>>> John Snow <jsnow@redhat.com> writes:

>>>>

>>>>> Petty style guide fixes and line length enforcement.  Not a big win, not

>>>>> a big loss, but flake8 passes 100% on the qapi module, which gives us an

>>>>> easy baseline to enforce hereafter.

>>>>>

>>>>> Signed-off-by: John Snow <jsnow@redhat.com>

[...]
>>>>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py

>>>>> index e1df0e341f..2e4b4de0fa 100644

>>>>> --- a/scripts/qapi/commands.py

>>>>> +++ b/scripts/qapi/commands.py

>>>>> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):

>>>>>    def gen_marshal_output(ret_type):

>>>>>        return mcgen('''

>>>>>    -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,

>>>>> QObject **ret_out, Error **errp)

>>>>> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,

>>>>> +                                          Error **errp)

>>>> The continued parameter list may become misalignd in generated C.

>>>> E.g.

>>>>       static void qmp_marshal_output_BlockInfoList(BlockInfoList *ret_in, QObject **ret_out,

>>>>                                                 Error **errp)

>>>>       {

>>>>       ...

>>>>       }

>>>> Do we care?

>>>>

>>>

>>> Yeah, I don't know. Do we?

>> I care, but I also care for automated style checks.

>> 

>>> It actually seemed more annoying to try and get flake8 to make an

>>> exception for these handful of examples.

>>>

>>> Path of least resistance led me here, but I can try and appease both

>>> systems if you'd prefer.

>> Up to now, I ran the style checkers manually, and this was just one

>> of

>> several complaints to ignore, so I left the code alone.

>> If it gets in the way of running them automatically, and messing up

>> the

>> generated code slightly is the easiest way to get it out of the way,

>> then I can accept the slight mess.

>> 

>

> I changed this a little to put all the args on the next line, which is

> slightly unusual but works okay.


I think it's slightly more unusual than the non-matching indentation
was.

Yet another way to skin this cat:

    static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,
                                    QObject **ret_out, Error **errp)

Now the second line is not aligned with the left parenthesis both in the
Python source and in the generated C.

> I think that's a fine middle ground, because the alternative (to me)

> is to start using abstracted code generation tokens in a tree

> structure, etc etc etc.

>

> Embedded templates are always gonna look kinda nasty, I think, because

> you're trying to fight style guidelines in two languages

> simultaneously and it's never gonna quite work out exactly how you

> want without some pretty complex abstraction mechanisms that are well

> beyond the power we need right now.


The thought "screw this, pile the output through /usr/bin/indent" has
crossed my mind more than once.
John Snow Sept. 18, 2020, 6:13 p.m. UTC | #6
On 9/18/20 6:33 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:

> 

>> On 9/17/20 3:54 AM, Markus Armbruster wrote:

>>> John Snow <jsnow@redhat.com> writes:

>>>

>>>> On 9/16/20 8:12 AM, Markus Armbruster wrote:

>>>>> John Snow <jsnow@redhat.com> writes:

>>>>>

>>>>>> Petty style guide fixes and line length enforcement.  Not a big win, not

>>>>>> a big loss, but flake8 passes 100% on the qapi module, which gives us an

>>>>>> easy baseline to enforce hereafter.

>>>>>>

>>>>>> Signed-off-by: John Snow <jsnow@redhat.com>

> [...]

>>>>>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py

>>>>>> index e1df0e341f..2e4b4de0fa 100644

>>>>>> --- a/scripts/qapi/commands.py

>>>>>> +++ b/scripts/qapi/commands.py

>>>>>> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):

>>>>>>     def gen_marshal_output(ret_type):

>>>>>>         return mcgen('''

>>>>>>     -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,

>>>>>> QObject **ret_out, Error **errp)

>>>>>> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,

>>>>>> +                                          Error **errp)

>>>>> The continued parameter list may become misalignd in generated C.

>>>>> E.g.

>>>>>        static void qmp_marshal_output_BlockInfoList(BlockInfoList *ret_in, QObject **ret_out,

>>>>>                                                  Error **errp)

>>>>>        {

>>>>>        ...

>>>>>        }

>>>>> Do we care?

>>>>>

>>>>

>>>> Yeah, I don't know. Do we?

>>> I care, but I also care for automated style checks.

>>>

>>>> It actually seemed more annoying to try and get flake8 to make an

>>>> exception for these handful of examples.

>>>>

>>>> Path of least resistance led me here, but I can try and appease both

>>>> systems if you'd prefer.

>>> Up to now, I ran the style checkers manually, and this was just one

>>> of

>>> several complaints to ignore, so I left the code alone.

>>> If it gets in the way of running them automatically, and messing up

>>> the

>>> generated code slightly is the easiest way to get it out of the way,

>>> then I can accept the slight mess.

>>>

>>

>> I changed this a little to put all the args on the next line, which is

>> slightly unusual but works okay.

> 

> I think it's slightly more unusual than the non-matching indentation

> was.

> 

> Yet another way to skin this cat:

> 

>      static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,

>                                      QObject **ret_out, Error **errp)

> 

> Now the second line is not aligned with the left parenthesis both in the

> Python source and in the generated C.

> 


Your wish is my command!
(Except in the other replies where I am arguing with you.)

>> I think that's a fine middle ground, because the alternative (to me)

>> is to start using abstracted code generation tokens in a tree

>> structure, etc etc etc.

>>

>> Embedded templates are always gonna look kinda nasty, I think, because

>> you're trying to fight style guidelines in two languages

>> simultaneously and it's never gonna quite work out exactly how you

>> want without some pretty complex abstraction mechanisms that are well

>> beyond the power we need right now.

> 

> The thought "screw this, pile the output through /usr/bin/indent" has

> crossed my mind more than once.

> 

> 


Bigger fish to fry, but with other languages like rust looming, making 
the core generation facilities nicer might be ...nice. Not for this series.

Two approaches in general would make sense to me:

1. Building an AST for C instead of strings, and rendering the AST.
2. Writing a templating engine that doesn't break the Python indentation 
flow by hoisting them into module constants and improving the rendering 
logic.

--js
Markus Armbruster Sept. 21, 2020, 7:31 a.m. UTC | #7
John Snow <jsnow@redhat.com> writes:

> On 9/18/20 6:33 AM, Markus Armbruster wrote:

>> John Snow <jsnow@redhat.com> writes:

[...]
>>> Embedded templates are always gonna look kinda nasty, I think, because

>>> you're trying to fight style guidelines in two languages

>>> simultaneously and it's never gonna quite work out exactly how you

>>> want without some pretty complex abstraction mechanisms that are well

>>> beyond the power we need right now.

>> The thought "screw this, pile the output through /usr/bin/indent"

>> has

>> crossed my mind more than once.

>> 

>

> Bigger fish to fry, but with other languages like rust looming, making

> the core generation facilities nicer might be ...nice. Not for this

> series.


Definitely not.

The way we generate C now is (close to) the stupidest way that could
possibly work.  Stupid is *good*, until it creates actual problems.

Any improvement will have to balance readability vs. hackability.

What we have now is often hard to read, but pretty easy to hack.  May
sound like a contradiction, but it's been my experience.

> Two approaches in general would make sense to me:

>

> 1. Building an AST for C instead of strings, and rendering the AST.


I fear the need to define an AST for C makes this uneconomical.

I've missed the ease of generating Lisp in Lisp many times.

> 2. Writing a templating engine that doesn't break the Python

> indentation flow by hoisting them into module constants and improving

> the rendering logic.


Too vague for me to judge, happy to look at PoC patches.
John Snow Sept. 21, 2020, 2:50 p.m. UTC | #8
On 9/21/20 3:31 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 9/18/20 6:33 AM, Markus Armbruster wrote:
>>> John Snow <jsnow@redhat.com> writes:
> [...]

>> 2. Writing a templating engine that doesn't break the Python
>> indentation flow by hoisting them into module constants and improving
>> the rendering logic.
> 
> Too vague for me to judge, happy to look at PoC patches.
> 

Okay, some other day. Not important enough presently. It's just hard to 
stop brainstorming ideas as I learn about and touch different areas of code.

--js
diff mbox series

Patch

diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
new file mode 100644
index 0000000000..45d8146f3f
--- /dev/null
+++ b/scripts/qapi/.flake8
@@ -0,0 +1,2 @@ 
+[flake8]
+extend-ignore = E722  # Pylint handles this, but smarter.
\ No newline at end of file
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index e1df0e341f..2e4b4de0fa 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -69,7 +69,8 @@  def gen_call(name, arg_type, boxed, ret_type):
 def gen_marshal_output(ret_type):
     return mcgen('''
 
-static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)
+static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
+                                          Error **errp)
 {
     Visitor *v;
 
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index a835ee6fde..b77e0e56b2 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -536,7 +536,7 @@  def set_defined_in(self, name):
             v.set_defined_in(name)
 
     def check(self, schema, seen):
-        if not self.tag_member: # flat union
+        if not self.tag_member:  # flat union
             self.tag_member = seen.get(c_name(self._tag_name))
             base = "'base'"
             # Pointing to the base type when not implicit would be
@@ -821,7 +821,7 @@  def __init__(self, fname):
         self._entity_dict = {}
         self._module_dict = OrderedDict()
         self._schema_dir = os.path.dirname(fname)
-        self._make_module(None) # built-ins
+        self._make_module(None)  # built-ins
         self._make_module(fname)
         self._predefining = True
         self._def_predefineds()
@@ -965,7 +965,9 @@  def _make_implicit_object_type(self, name, info, ifcond, role, members):
             # But it's not tight: the disjunction need not imply it.  We
             # may end up compiling useless wrapper types.
             # TODO kill simple unions or implement the disjunction
-            assert (ifcond or []) == typ._ifcond # pylint: disable=protected-access
+
+            # pylint: disable=protected-access
+            assert (ifcond or []) == typ._ifcond
         else:
             self._def_entity(QAPISchemaObjectType(
                 name, info, None, ifcond, None, None, members, None))
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index ea277e7704..31bf46e7f7 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -31,7 +31,8 @@  def gen_visit_decl(name, scalar=False):
     if not scalar:
         c_type += '*'
     return mcgen('''
-bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_type)sobj, Error **errp);
+bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_type)sobj,
+                           Error **errp);
 ''',
                  c_name=c_name(name), c_type=c_type)
 
@@ -125,7 +126,8 @@  def gen_visit_object_members(name, base, members, variants):
 def gen_visit_list(name, element_type):
     return mcgen('''
 
-bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp)
+bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj,
+                           Error **errp)
 {
     bool ok = false;
     %(c_name)s *tail;
@@ -158,7 +160,8 @@  def gen_visit_list(name, element_type):
 def gen_visit_enum(name):
     return mcgen('''
 
-bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s *obj, Error **errp)
+bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s *obj,
+                           Error **errp)
 {
     int value = *obj;
     bool ok = visit_type_enum(v, name, &value, &%(c_name)s_lookup, errp);
@@ -172,7 +175,8 @@  def gen_visit_enum(name):
 def gen_visit_alternate(name, variants):
     ret = mcgen('''
 
-bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp)
+bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj,
+                           Error **errp)
 {
     bool ok = false;
 
@@ -247,7 +251,8 @@  def gen_visit_alternate(name, variants):
 def gen_visit_object(name, base, members, variants):
     return mcgen('''
 
-bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp)
+bool visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj,
+                           Error **errp)
 {
     bool ok = false;