[Python-ideas] dictionary constructor should not allow duplicate keys

[M.-A. Lemburg]

On 04.05.2016 16:00, Steven D'Aprano wrote:
> On Wed, May 04, 2016 at 02:37:56PM +0200, M.-A. Lemburg wrote:
>> On 04.05.2016 14:15, Steven D'Aprano wrote:
>>> On Wed, May 04, 2016 at 01:56:56PM +0200, M.-A. Lemburg wrote:
>>>
>>>> Issuing a warning for this would probably help, but raising
>>>> an error introduced a backwards incompatibility which is
>>>> not warranted IMO, given how seldom such situations occur in
>>>> practice.
> 
> I agree with you that an error is unjustified but a warning might be.
> 
> 
>>>> For the implementation, there are two possibilities I can think
>>>> of:
>>>>
>>>> 1. have STORE_MAP run a test for an already existing entry
>>>>    and issue a warning
> 
> You have suggested that's too expensive, so I'm happy to rule that out.
> 
> 
>>>> 2. add a final CHECK_MAP byte code to check that the number
>>>>    of keys in the mapping correspond to the number of keys
>>>>    added via the dict literal
> [...]
>>>> The second one is cheap regarding performance, but may not
>>>> be accurate, since STORE_MAP may well be working on dynamically
>>>> generated keys. It does work well for constants, and if we're
>>>> just issuing a warning, may be good enough.
>>>
>>> I'm not sure I understand what you mean by this. Are you suggesting that 
>>> if I write:
>>>
>>> {1: None, 1: None}
>>>
>>> Python knows that it got two keys as argument, and so 
>>> CHECK_MAP can determine that there is a duplicate? (i.e. that 2 keys 
>>> were arguments, but the finished dict only has length 1). But if I 
>>> write:
>>>
>>> {x: None, x: None}
>>>
>>> Python *cannot* tell that there were two keys given (since they are 
>>> expressions, not constants) and CHECK_MAP can't do anything?
>>
>> Well, the CHECK_MAP byte code would only check whether the number
>> of assignments you are doing is equal to the number of keys
>> found in the final dict. That's a very coarse test and not
>> perfect. It can also not tell you which key causes the
>> mismatch.
>>
>> On the plus side, the extra check is a simply size check
>> and doesn't have to be applied per added mapping.
> 
> Sorry MAL, perhaps I'm a bit slow tonight, but I'm still lost. {x: None, 
> x: None} -- would CHECK_MAP detect that as a duplicate or not?
> 
> If the answer is "Yes", then this seems to me to be a cheap way of 
> detecting the existence of at least one duplicate (but not which key was 
> duplicate). That strikes me as good enough, in which case I would 
> support raising a warning.

Yes:

>>> dis.dis('{x:None, x:None}')
  1           0 BUILD_MAP                2
              3 LOAD_CONST               0 (None)
              6 LOAD_NAME                0 (x)
              9 STORE_MAP
             10 LOAD_CONST               0 (None)
             13 LOAD_NAME                0 (x)
             16 STORE_MAP
             17 RETURN_VALUE
>>> d = {x:None, x:None}
>>> len(d)
1

Since 1 != 2 -> issue warning

We'd just have to tell the CHECK_MAP byte code to check
for len(map) == 2. The byte code compiler will be able to
determine this and add the correct byte code entry.

-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Experts (#1, May 04 2016)
>>> Python Projects, Coaching and Consulting ...  http://www.egenix.com/
>>> Python Database Interfaces ...           http://products.egenix.com/
>>> Plone/Zope Database Interfaces ...           http://zope.egenix.com/
________________________________________________________________________

::: We implement business ideas - efficiently in both time and costs :::

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
                      http://www.malemburg.com/