find_empty_slot will also do *value_addr = &mp->ma_values[-1] if it encounters a split dict.

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.

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.

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

> 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.

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?

mp_values should be mp->ma_values, sorry.

> 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 ;-)

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;
 }

Victor committed the patch in msg276042 in 579141d6e353.

> Victor committed the patch in msg276042 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];".