Re: [PATCH 04/16] qapi/expr.py: Add assertion for union type 'check_dict'

[John Snow]

On 9/24/20 9:08 PM, Eduardo Habkost wrote:
> On Thu, Sep 24, 2020 at 08:47:31PM -0400, John Snow wrote:
> > On 9/23/20 3:53 PM, Eduardo Habkost wrote:
> > > On Tue, Sep 22, 2020 at 05:13:01PM -0400, John Snow wrote:
> > > > mypy isn't fond of allowing you to check for bool membership in a
> > > > collection of str elements. Guard this lookup for precisely when we were
> > > > given a name.
> > > >
> > > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > > ---
> > > >    scripts/qapi/expr.py | 4 +++-
> > > >    1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> > > > index f6b55a87c1..67892502e9 100644
> > > > --- a/scripts/qapi/expr.py
> > > > +++ b/scripts/qapi/expr.py
> > > > @@ -166,7 +166,9 @@ def check_type(value, info, source,
> > > >            raise QAPISemError(info,
> > > >                               "%s should be an object or type name" % source)
> > > > -    permit_upper = allow_dict in info.pragma.name_case_whitelist
> > > > +    permit_upper = False
> > > > +    if isinstance(allow_dict, str):
> > > > +        permit_upper = allow_dict in info.pragma.name_case_whitelist
> > >
> > > Well, this keeps existing behavior, so:
> > >
> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > >
> > > But: what exactly is the meaning of allow_dict=False,
> > > allow_dict=True, and allow_dict being a string?
> >
> > allow_dict = True -- allows the type to be an object describing the type.
> >
> > allow_dict: str -- allows the type to be an object (like True), but also
> > passes a name in for the purposes of validating the name with the pragma
> > whitelist(!)
>
> What.

(lol)

>

What's going on here is that when you pass in a name, bool(allow_dict) 
is True -- so we will allow the object being checked here to be a dict.

Also, when you pass in a name, that name is looked up in 
info.pragma.name_case_whitelist to check if the names of the keys in the 
dict being checked (again, allow_dict is implicitly true here) are 
allowed to use uppercase names.

I have some more experimental patches I didn't want to mix in with the 
type checking patches that try to extract pragma checks from this code 
and perform them elsewhere; but that's going to come much, much later.

--js