| Message ID | 20201214235327.1007124-5-jsnow@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: static typing conversion, pt1.5 | expand |
John Snow <jsnow@redhat.com> writes: > We already assert this in end_if, but that's opaque to mypy. Do it in > _wrap_ifcond instead. Same effect at runtime, but mypy can now infer > the type in _wrap_ifcond's body. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/gen.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > index b40f18eee3cd..a6dc991b1d03 100644 > --- a/scripts/qapi/gen.py > +++ b/scripts/qapi/gen.py > @@ -130,11 +130,11 @@ def start_if(self, ifcond: List[str]) -> None: > self._start_if = (ifcond, self._body, self._preamble) > > def end_if(self) -> None: > - assert self._start_if > self._wrap_ifcond() > self._start_if = None > > def _wrap_ifcond(self) -> None: > + assert self._start_if > self._body = _wrap_ifcond(self._start_if[0], > self._start_if[1], self._body) > self._preamble = _wrap_ifcond(self._start_if[0], Drawback: the public method's precondition is now more opaque. Do we care?
On 12/16/20 3:26 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> We already assert this in end_if, but that's opaque to mypy. Do it in >> _wrap_ifcond instead. Same effect at runtime, but mypy can now infer >> the type in _wrap_ifcond's body. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> scripts/qapi/gen.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py >> index b40f18eee3cd..a6dc991b1d03 100644 >> --- a/scripts/qapi/gen.py >> +++ b/scripts/qapi/gen.py >> @@ -130,11 +130,11 @@ def start_if(self, ifcond: List[str]) -> None: >> self._start_if = (ifcond, self._body, self._preamble) >> >> def end_if(self) -> None: >> - assert self._start_if >> self._wrap_ifcond() >> self._start_if = None >> >> def _wrap_ifcond(self) -> None: >> + assert self._start_if >> self._body = _wrap_ifcond(self._start_if[0], >> self._start_if[1], self._body) >> self._preamble = _wrap_ifcond(self._start_if[0], > > Drawback: the public method's precondition is now more opaque. Do we > care? > Ish. If you call end_if before start_if, what did you want to have happen? Or more to the point: do you want the assertion in both places? --js
John Snow <jsnow@redhat.com> writes: > On 12/16/20 3:26 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> We already assert this in end_if, but that's opaque to mypy. Do it in >>> _wrap_ifcond instead. Same effect at runtime, but mypy can now infer >>> the type in _wrap_ifcond's body. >>> >>> Signed-off-by: John Snow <jsnow@redhat.com> >>> --- >>> scripts/qapi/gen.py | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py >>> index b40f18eee3cd..a6dc991b1d03 100644 >>> --- a/scripts/qapi/gen.py >>> +++ b/scripts/qapi/gen.py >>> @@ -130,11 +130,11 @@ def start_if(self, ifcond: List[str]) -> None: >>> self._start_if = (ifcond, self._body, self._preamble) >>> >>> def end_if(self) -> None: >>> - assert self._start_if >>> self._wrap_ifcond() >>> self._start_if = None >>> >>> def _wrap_ifcond(self) -> None: >>> + assert self._start_if >>> self._body = _wrap_ifcond(self._start_if[0], >>> self._start_if[1], self._body) >>> self._preamble = _wrap_ifcond(self._start_if[0], >> >> Drawback: the public method's precondition is now more opaque. Do we >> care? >> > > Ish. If you call end_if before start_if, what did you want to have happen? Point. > Or more to the point: do you want the assertion in both places? What about inlining QAPIGenCCode._wrap_ifcond() into .end_if()?
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index b40f18eee3cd..a6dc991b1d03 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -130,11 +130,11 @@ def start_if(self, ifcond: List[str]) -> None: self._start_if = (ifcond, self._body, self._preamble) def end_if(self) -> None: - assert self._start_if self._wrap_ifcond() self._start_if = None def _wrap_ifcond(self) -> None: + assert self._start_if self._body = _wrap_ifcond(self._start_if[0], self._start_if[1], self._body) self._preamble = _wrap_ifcond(self._start_if[0],
We already assert this in end_if, but that's opaque to mypy. Do it in _wrap_ifcond instead. Same effect at runtime, but mypy can now infer the type in _wrap_ifcond's body. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)