It does happen to be a list (as of now), but we can describe it in more
general terms with no loss in accuracy to allow tuples and other
constructs.

In the future, we can write "ifcond: Sequence[str] = ()" as a default
parameter, which we could not do with a Mutable type like a List.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/commands.py |  3 ++-
 scripts/qapi/events.py   |  4 ++--
 scripts/qapi/gen.py      | 12 ++++++------
 scripts/qapi/types.py    | 12 ++++++------
 scripts/qapi/visit.py    | 10 +++++-----
 5 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 54af519f44d..0a75a9371ba 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -17,6 +17,7 @@
     Dict,
     List,
     Optional,
+    Sequence,
     Set,
 )
 
@@ -297,7 +298,7 @@ def visit_end(self) -> None:
     def visit_command(self,
                       name: str,
                       info: Optional[QAPISourceInfo],
-                      ifcond: List[str],
+                      ifcond: Sequence[str],
                       features: List[QAPISchemaFeature],
                       arg_type: Optional[QAPISchemaObjectType],
                       ret_type: Optional[QAPISchemaType],
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 8c57deb2b89..90d2f6156d8 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -12,7 +12,7 @@
 See the COPYING file in the top-level directory.
 """
 
-from typing import List, Optional
+from typing import List, Optional, Sequence
 
 from .common import c_enum_const, c_name, mcgen
 from .gen import QAPISchemaModularCVisitor, build_params, ifcontext
@@ -214,7 +214,7 @@ def visit_end(self) -> None:
     def visit_event(self,
                     name: str,
                     info: Optional[QAPISourceInfo],
-                    ifcond: List[str],
+                    ifcond: Sequence[str],
                     features: List[QAPISchemaFeature],
                     arg_type: Optional[QAPISchemaObjectType],
                     boxed: bool) -> None:
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 63549cc8d47..1fa503bdbdf 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -17,8 +17,8 @@
 from typing import (
     Dict,
     Iterator,
-    List,
     Optional,
+    Sequence,
     Tuple,
 )
 
@@ -85,7 +85,7 @@ def write(self, output_dir: str) -> None:
                 fp.write(text)
 
 
-def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str:
+def _wrap_ifcond(ifcond: Sequence[str], before: str, after: str) -> str:
     if before == after:
         return after   # suppress empty #if ... #endif
 
@@ -127,9 +127,9 @@ def build_params(arg_type: Optional[QAPISchemaObjectType],
 class QAPIGenCCode(QAPIGen):
     def __init__(self, fname: str):
         super().__init__(fname)
-        self._start_if: Optional[Tuple[List[str], str, str]] = None
+        self._start_if: Optional[Tuple[Sequence[str], str, str]] = None
 
-    def start_if(self, ifcond: List[str]) -> None:
+    def start_if(self, ifcond: Sequence[str]) -> None:
         assert self._start_if is None
         self._start_if = (ifcond, self._body, self._preamble)
 
@@ -187,11 +187,11 @@ def _bottom(self) -> str:
 
 
 @contextmanager
-def ifcontext(ifcond: List[str], *args: QAPIGenCCode) -> Iterator[None]:
+def ifcontext(ifcond: Sequence[str], *args: QAPIGenCCode) -> Iterator[None]:
     """
     A with-statement context manager that wraps with `start_if()` / `end_if()`.
 
-    :param ifcond: A list of conditionals, passed to `start_if()`.
+    :param ifcond: A sequence of conditionals, passed to `start_if()`.
     :param args: any number of `QAPIGenCCode`.
 
     Example::
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 2bdd6268476..20d572a23aa 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -13,7 +13,7 @@
 # See the COPYING file in the top-level directory.
 """
 
-from typing import List, Optional
+from typing import List, Optional, Sequence
 
 from .common import (
     c_enum_const,
@@ -139,7 +139,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
     return ret
 
 
-def gen_object(name: str, ifcond: List[str],
+def gen_object(name: str, ifcond: Sequence[str],
                base: Optional[QAPISchemaObjectType],
                members: List[QAPISchemaObjectTypeMember],
                variants: Optional[QAPISchemaVariants]) -> str:
@@ -307,7 +307,7 @@ def _gen_type_cleanup(self, name: str) -> None:
     def visit_enum_type(self,
                         name: str,
                         info: Optional[QAPISourceInfo],
-                        ifcond: List[str],
+                        ifcond: Sequence[str],
                         features: List[QAPISchemaFeature],
                         members: List[QAPISchemaEnumMember],
                         prefix: Optional[str]) -> None:
@@ -318,7 +318,7 @@ def visit_enum_type(self,
     def visit_array_type(self,
                          name: str,
                          info: Optional[QAPISourceInfo],
-                         ifcond: List[str],
+                         ifcond: Sequence[str],
                          element_type: QAPISchemaType) -> None:
         with ifcontext(ifcond, self._genh, self._genc):
             self._genh.preamble_add(gen_fwd_object_or_array(name))
@@ -328,7 +328,7 @@ def visit_array_type(self,
     def visit_object_type(self,
                           name: str,
                           info: Optional[QAPISourceInfo],
-                          ifcond: List[str],
+                          ifcond: Sequence[str],
                           features: List[QAPISchemaFeature],
                           base: Optional[QAPISchemaObjectType],
                           members: List[QAPISchemaObjectTypeMember],
@@ -351,7 +351,7 @@ def visit_object_type(self,
     def visit_alternate_type(self,
                              name: str,
                              info: Optional[QAPISourceInfo],
-                             ifcond: List[str],
+                             ifcond: Sequence[str],
                              features: List[QAPISchemaFeature],
                              variants: QAPISchemaVariants) -> None:
         with ifcontext(ifcond, self._genh):
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 22e62df9017..9aa0b1e11e9 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -13,7 +13,7 @@
 See the COPYING file in the top-level directory.
 """
 
-from typing import List, Optional
+from typing import List, Optional, Sequence
 
 from .common import (
     c_enum_const,
@@ -337,7 +337,7 @@ def _begin_user_module(self, name: str) -> None:
     def visit_enum_type(self,
                         name: str,
                         info: Optional[QAPISourceInfo],
-                        ifcond: List[str],
+                        ifcond: Sequence[str],
                         features: List[QAPISchemaFeature],
                         members: List[QAPISchemaEnumMember],
                         prefix: Optional[str]) -> None:
@@ -348,7 +348,7 @@ def visit_enum_type(self,
     def visit_array_type(self,
                          name: str,
                          info: Optional[QAPISourceInfo],
-                         ifcond: List[str],
+                         ifcond: Sequence[str],
                          element_type: QAPISchemaType) -> None:
         with ifcontext(ifcond, self._genh, self._genc):
             self._genh.add(gen_visit_decl(name))
@@ -357,7 +357,7 @@ def visit_array_type(self,
     def visit_object_type(self,
                           name: str,
                           info: Optional[QAPISourceInfo],
-                          ifcond: List[str],
+                          ifcond: Sequence[str],
                           features: List[QAPISchemaFeature],
                           base: Optional[QAPISchemaObjectType],
                           members: List[QAPISchemaObjectTypeMember],
@@ -379,7 +379,7 @@ def visit_object_type(self,
     def visit_alternate_type(self,
                              name: str,
                              info: Optional[QAPISourceInfo],
-                             ifcond: List[str],
+                             ifcond: Sequence[str],
                              features: List[QAPISchemaFeature],
                              variants: QAPISchemaVariants) -> None:
         with ifcontext(ifcond, self._genh, self._genc):
-- 
2.29.2

John Snow <jsnow@redhat.com> writes:

> It does happen to be a list (as of now), but we can describe it in more
> general terms with no loss in accuracy to allow tuples and other
> constructs.
>
> In the future, we can write "ifcond: Sequence[str] = ()" as a default
> parameter, which we could not do with a Mutable type like a List.

Well, we could write "= []", but we shouldn't.  Worth a commit message
tweak?

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

Reviewed-by: Markus Armbruster <armbru@redhat.com>

On 2/16/21 3:43 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> It does happen to be a list (as of now), but we can describe it in more
>> general terms with no loss in accuracy to allow tuples and other
>> constructs.
>>
>> In the future, we can write "ifcond: Sequence[str] = ()" as a default
>> parameter, which we could not do with a Mutable type like a List.
> 
> Well, we could write "= []", but we shouldn't.  Worth a commit message
> tweak?
> 

It would be funny to leave it in to see if anyone tries to disprove me, 
and in the act of disproving me, learns for themselves why you "can't" 
do that. Rite of passage for Python programming.

Jokes aside:

"which we could not do ^safely^ with a Mutable type like a List."

If that warrants further exposition by Professor Snow:

"(Unsafe due to how Python initializes defaults, see 
https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments)"

I leave it to your discretion.

>> Signed-off-by: John Snow <jsnow@redhat.com>
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

These are worth more than BTC!

--js

John Snow <jsnow@redhat.com> writes:

> On 2/16/21 3:43 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> It does happen to be a list (as of now), but we can describe it in more
>>> general terms with no loss in accuracy to allow tuples and other
>>> constructs.
>>>
>>> In the future, we can write "ifcond: Sequence[str] = ()" as a default
>>> parameter, which we could not do with a Mutable type like a List.
>> Well, we could write "= []", but we shouldn't.  Worth a commit
>> message
>> tweak?
>> 
>
> It would be funny to leave it in to see if anyone tries to disprove
> me, and in the act of disproving me, learns for themselves why you
> "can't" do that. Rite of passage for Python programming.
>
> Jokes aside:
>
> "which we could not do ^safely^ with a Mutable type like a List."

Works for me.

> If that warrants further exposition by Professor Snow:
>
> "(Unsafe due to how Python initializes defaults, see
> https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments)"

I can add that if you prefer.

> I leave it to your discretion.
>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> 
>
> These are worth more than BTC!

;)