On Tue, Aug 3, 2021 at 5:35 PM Markus Armbruster <arm...@redhat.com> wrote:
> marcandre.lur...@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >
> > Change the 'if' condition strings to be C-agnostic and be simple
> > identifiers.
>
> This is less general, and that's okay, we're doing it for a purpose.
> But the commit message should mention / explain all this.
>
ok
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
> > Tested-by: John Snow <js...@redhat.com>
> > ---
>
> [...]
>
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index f8718e201b..0c718e43c9 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -218,7 +218,7 @@ def cgen_ifcond(ifcond: Union[str, Dict[str, Any]])
> -> str:
> > if not ifcond:
> > return ''
> > if isinstance(ifcond, str):
> > - return ifcond
> > + return 'defined(' + ifcond + ')'
> >
> > oper, operands = next(iter(ifcond.items()))
> > if oper == 'not':
> > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> > index d2bd52c49f..d355cbc8c1 100644
> > --- a/scripts/qapi/expr.py
> > +++ b/scripts/qapi/expr.py
> > @@ -281,10 +281,10 @@ def check_if(expr: _JSONObject, info:
> QAPISourceInfo, source: str) -> None:
> >
> > def _check_if(cond: Union[str, object]) -> None:
> > if isinstance(cond, str):
> > - if not cond.strip():
> > + if not cond.isidentifier():
>
> This accepts *Python* identifiers:
>
> $ python
> Python 3.9.6 (default, Jul 16 2021, 00:00:00)
> [...]
> >>> 'André'.isidentifier()
> True
>
> These may or may not work for the languages we generate. Wouldn't
> restricting identifiers to something like /[A-Z][A-Z0-9_]*/ make more
> sense?
>
>
yes, works for me
> > raise QAPISemError(
> > info,
> > - "'if' condition '%s' of %s makes no sense"
> > + "'if' condition '%s' of %s is not a valid
> identifier"
> > % (cond, source))
> > return
> >
> > diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err
> b/tests/qapi-schema/alternate-branch-if-invalid.err
> > index d384929c51..03bad877a3 100644
> > --- a/tests/qapi-schema/alternate-branch-if-invalid.err
> > +++ b/tests/qapi-schema/alternate-branch-if-invalid.err
> > @@ -1,2 +1,2 @@
> > alternate-branch-if-invalid.json: In alternate 'Alt':
> > -alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member
> 'branch' makes no sense
> > +alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member
> 'branch' is not a valid identifier
>
> [...]
>
>
