| msg275800 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-09-11 14:45 |
| find_empty_slot will also do *value_addr = &mp->ma_values[-1] if it encounters a split dict. |
|
|
| msg276026 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-09-12 12:01 |
| Hum, I understand that it's a bug, so do you know how to reproduce the bug from a pure Python script? If yes, it would be nice to include an unit test. |
|
|
| msg276027 - (view) |
Author: Inada Naoki (methane) * |
Date: 2016-09-12 12:15 |
| It cannot hit from Python. The function never called for split table, since resize function combine split table. So we can just comment the function supports only combined table. |
|
|
| msg276028 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-09-12 12:23 |
| New changeset 0e986b81cc1c by Victor Stinner in branch 'default': Issue #28077: find_empty_slot() only supports combined dict https://hg.python.org/cpython/rev/0e986b81cc1c |
|
|
| msg276029 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-09-12 12:25 |
| > find_empty_slot will also do *value_addr = &mp->ma_values[-1] if it encounters a split dict. I checked just before Naoki comment the issue, but I have the same conclusion: find_empty_slot() is never called on a split table. By the way, we might modify find_empty_slot() to call insertion_resize(), because insertion_resize() is always called before find_empty_slot(). I close the issue. Please open a new issue if you want to refactor the code. |
|
|
| msg276038 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-09-12 13:12 |
| Yes. It cannot be hit and I want to remove it at first. But I decide not to do that since there is no clue in the future find_empty_slot will not be used against split dict. And hapyo, if you decide to make find_empty_slot support only combined dict, why not remove mp_values check? |
|
|
| msg276039 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-09-12 13:13 |
| mp_values should be mp->ma_values, sorry. |
|
|
| msg276040 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-09-12 13:13 |
| > And hapyo, if you decide to make find_empty_slot support only combined dict, why not remove mp_values check? Oh, I didn't notice that find_empty_slot() partially support split tables. Would you like to write a patch? I reopen the issue to fix this nit ;-) |
|
|
| msg276042 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-09-12 13:28 |
| Just need to delete the lines :) : diff -r 0773e5cb8608 Objects/dictobject.c --- a/Objects/dictobject.c Mon Sep 12 14:43:14 2016 +0200 +++ b/Objects/dictobject.c Mon Sep 12 21:26:41 2016 +0800 @@ -1011,10 +1011,7 @@ ep = &ep0[mp->ma_keys->dk_nentries]; *hashpos = i & mask; assert(ep->me_value == NULL); - if (mp->ma_values) - *value_addr = &mp->ma_values[ix]; - else - *value_addr = &ep->me_value; + *value_addr = &ep->me_value; return ix; } |
|
|
| msg276205 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2016-09-13 07:47 |
| Victor committed the patch in in 579141d6e353. |
|
|
| msg276209 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-09-13 08:07 |
| > Victor committed the patch in in 579141d6e353. Well, my change is a little bit different than find_empty_slot.patch. I removed the return value and I kept "ep = &ep0[mp->ma_keys->dk_nentries];". |
|
|