From: John Snow <jsnow@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
Michael Roth <mdroth@linux.vnet.ibm.com>,
Cleber Rosa <crosa@redhat.com>
Subject: Re: [PATCH 04/16] qapi/expr.py: Add assertion for union type 'check_dict'
Date: Thu, 24 Sep 2020 21:30:39 -0400 [thread overview]
Message-ID: <7e40f8a3-e87f-2786-aef7-79f9a8015a58@redhat.com> (raw)
In-Reply-To: <20200925010857.GM3717385@habkost.net>
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
next prev parent reply other threads:[~2020-09-25 1:32 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-22 21:12 [PATCH 00/16] qapi: static typing conversion, pt2 John Snow
2020-09-22 21:12 ` [PATCH 01/16] qapi/expr.py: Remove 'info' argument from nested check_if_str John Snow
2020-09-23 19:26 ` Eduardo Habkost
2020-09-24 23:06 ` Cleber Rosa
2020-09-22 21:12 ` [PATCH 02/16] qapi/expr.py: Check for dict instead of OrderedDict John Snow
2020-09-23 19:26 ` Eduardo Habkost
2020-09-24 23:07 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 03/16] qapi/expr.py: constrain incoming expression types John Snow
2020-09-23 19:42 ` Eduardo Habkost
2020-09-25 0:05 ` Cleber Rosa
2020-09-25 0:43 ` John Snow
2020-09-25 0:40 ` John Snow
2020-09-25 0:06 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 04/16] qapi/expr.py: Add assertion for union type 'check_dict' John Snow
2020-09-23 19:53 ` Eduardo Habkost
2020-09-25 0:47 ` John Snow
2020-09-25 1:08 ` Eduardo Habkost
2020-09-25 1:30 ` John Snow [this message]
2020-09-25 0:19 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 05/16] qapi/expr.py: move string check upwards in check_type John Snow
2020-09-23 19:56 ` Eduardo Habkost
2020-09-25 0:22 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 06/16] qapi/expr.py: Check type of 'data' member John Snow
2020-09-23 19:58 ` Eduardo Habkost
2020-09-25 0:31 ` Cleber Rosa
2020-09-25 0:50 ` John Snow
2020-09-25 16:48 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 07/16] qapi/expr.py: Add casts in a few select cases John Snow
2020-09-23 20:01 ` Eduardo Habkost
2020-09-25 0:36 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 08/16] qapi/expr.py: add type hint annotations John Snow
2020-09-23 20:06 ` Eduardo Habkost
2020-09-25 0:44 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 09/16] qapi/expr.py: rewrite check_if John Snow
2020-09-23 20:09 ` Eduardo Habkost
2020-09-25 0:50 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 10/16] qapi/expr.py: Remove single-letter variable John Snow
2020-09-23 20:11 ` Eduardo Habkost
2020-09-25 0:52 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 11/16] qapi/expr.py: enable pylint checks John Snow
2020-09-23 20:12 ` Eduardo Habkost
2020-09-25 0:53 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 12/16] qapi/expr.py: Add docstrings John Snow
2020-09-23 20:16 ` Eduardo Habkost
2020-09-25 0:52 ` John Snow
2020-09-25 0:59 ` Cleber Rosa
2020-09-25 1:10 ` John Snow
2020-09-22 21:13 ` [PATCH 13/16] qapi/expr.py: Modify check_keys to accept any Iterable John Snow
2020-09-23 20:17 ` Eduardo Habkost
2020-09-25 1:02 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 14/16] qapi/expr.py: Use tuples instead of lists for static data John Snow
2020-09-23 20:18 ` Eduardo Habkost
2020-09-25 1:03 ` Cleber Rosa
2020-09-25 1:12 ` John Snow
2020-09-22 21:13 ` [PATCH 15/16] qapi/expr.py: move related checks inside check_xxx functions John Snow
2020-09-23 20:25 ` Eduardo Habkost
2020-09-25 0:58 ` John Snow
2020-09-25 1:09 ` Cleber Rosa
2020-09-22 21:13 ` [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table John Snow
2020-09-23 20:36 ` Eduardo Habkost
2020-09-25 0:59 ` John Snow
2020-09-25 1:18 ` Cleber Rosa
2020-09-25 1:32 ` John Snow
2020-09-25 6:03 ` Helio Loureiro
2020-09-25 14:16 ` John Snow
2020-09-26 11:31 ` Helio Loureiro
2020-09-30 4:46 ` John Snow
2020-09-25 16:38 ` Cleber Rosa
2020-09-25 17:04 ` John Snow
2020-09-25 22:54 ` [PATCH 00/16] qapi: static typing conversion, pt2 John Snow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7e40f8a3-e87f-2786-aef7-79f9a8015a58@redhat.com \
--to=jsnow@redhat.com \
--cc=armbru@redhat.com \
--cc=crosa@redhat.com \
--cc=ehabkost@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).