Here is another updated extend() patch with all-or-nothing semantics.

This time, the tests include a check for the key order in d2 in

    extend(d, d2)

to make sure that the key that triggers the error is not the first key.
This makes sure that all-or-nothing behavior is tested.

Also, the key order was changed slightly so that the error key appears
as the last but one now.

Changed after another reading of ZyX' suggestions.


diff -r 18d84ed365a5 src/eval.c
--- a/src/eval.c        Wed Apr 22 22:18:22 2015 +0200
+++ b/src/eval.c        Sat May 02 08:50:39 2015 +0200
@@ -10502,6 +10502,10 @@ dict_extend(d1, d2, action)
     int                todo;
     char_u     *arg_errmsg = (char_u *)N_("extend() argument");
 
+    /*
+     * Check whether the operation can complete without errors (other than
+     * OOM).
+     */
     todo = (int)d2->dv_hashtab.ht_used;
     for (hi2 = d2->dv_hashtab.ht_array; todo > 0; ++hi2)
     {
@@ -10518,26 +10522,45 @@ dict_extend(d1, d2, action)
                        && HI2DI(hi2)->di_tv.v_type == VAR_FUNC
                        && var_check_func_name(hi2->hi_key,
                                                         di1 == NULL))
-                   break;
+                   return;
                if (!valid_varname(hi2->hi_key))
-                   break;
-           }
+                   return;
+           }
+           if (di1 == NULL)
+           {
+               if (tv_check_lock(d1->dv_lock, arg_errmsg, TRUE))
+                   return;
+           }
+           else if (*action == 'e')
+           {
+               EMSG2(_("E737: Key already exists: %s"), hi2->hi_key);
+               return;
+           }
+           else if (*action == 'f' && HI2DI(hi2) != di1
+                   && (tv_check_lock(di1->di_tv.v_lock, arg_errmsg, TRUE)
+                       || var_check_ro(di1->di_flags, arg_errmsg, TRUE)))
+                   return;
+       }
+    }
+
+    /*
+     * Replace or add dict items.
+     */
+    todo = (int)d2->dv_hashtab.ht_used;
+    for (hi2 = d2->dv_hashtab.ht_array; todo > 0; ++hi2)
+    {
+       if (!HASHITEM_EMPTY(hi2))
+       {
+           --todo;
+           di1 = dict_find(d1, hi2->hi_key, -1);
            if (di1 == NULL)
            {
                di1 = dictitem_copy(HI2DI(hi2));
                if (di1 != NULL && dict_add(d1, di1) == FAIL)
                    dictitem_free(di1);
            }
-           else if (*action == 'e')
-           {
-               EMSG2(_("E737: Key already exists: %s"), hi2->hi_key);
-               break;
-           }
            else if (*action == 'f' && HI2DI(hi2) != di1)
            {
-               if (tv_check_lock(di1->di_tv.v_lock, arg_errmsg, TRUE)
-                     || var_check_ro(di1->di_flags, arg_errmsg, TRUE))
-                   break;
                clear_tv(&di1->di_tv);
                copy_tv(&HI2DI(hi2)->di_tv, &di1->di_tv);
            }
@@ -10601,8 +10624,7 @@ f_extend(argvars, rettv)
 
        d1 = argvars[0].vval.v_dict;
        d2 = argvars[1].vval.v_dict;
-       if (d1 != NULL && !tv_check_lock(d1->dv_lock, arg_errmsg, TRUE)
-               && d2 != NULL)
+       if (d1 != NULL && d2 != NULL)
        {
            /* Check the third argument. */
            if (argvars[2].v_type != VAR_UNKNOWN)
diff -r 18d84ed365a5 src/testdir/test55.in
--- a/src/testdir/test55.in     Wed Apr 22 22:18:22 2015 +0200
+++ b/src/testdir/test55.in     Sat May 02 08:50:39 2015 +0200
@@ -408,6 +408,30 @@ let l = [0, 1, 2, 3]
 :endtry
 :$put =string(d)
 :"
+:$put ='extend() after lock on dict:'
+:unlet! d
+:let d = {'a': 99, 'b': 100, 'd': 101}
+:lockvar 1 d
+:try
+:  $put =string(extend(d, {'a': 123}))
+:  $put ='ok: did extend() existing item'
+:  $put =string(extend(d, {'x': 'new'}))
+:  $put ='wrong: did extend() with new item'
+:catch
+:  $put =v:exception[:14]
+:endtry
+:$put =string(d)
+:unlet! d2
+:let d2 = {'a': 789, 'b': 42, 'c': 'new', 'd': 52}
+:$put ='key order ok for all-or-nothing test: '.(keys(d2)[0] != 'c')
+:try
+:  $put =string(extend(d, d2))
+:catch
+:  $put =v:exception[:14]
+:endtry
+:$put =string(d)
+:unlet d d2
+:"
 :$put ='No remove() of write-protected scope-level variable:'
 :fun! Tfunc(this_is_a_loooooooooong_parameter_name)
 :  try
diff -r 18d84ed365a5 src/testdir/test55.ok
--- a/src/testdir/test55.ok     Wed Apr 22 22:18:22 2015 +0200
+++ b/src/testdir/test55.ok     Sat May 02 08:50:39 2015 +0200
@@ -138,6 +138,14 @@ did map()
 No extend() after lock on dict item:
 Vim(put):E741: 
 {'a': 99, 'b': 100}
+extend() after lock on dict:
+{'a': 123, 'b': 100, 'd': 101}
+ok: did extend() existing item
+Vim(put):E741: 
+{'a': 123, 'b': 100, 'd': 101}
+key order ok for all-or-nothing test: 1
+Vim(put):E741: 
+{'a': 123, 'b': 100, 'd': 101}
 No remove() of write-protected scope-level variable:
 Vim(put):E795: 
 No extend() of write-protected scope-level variable:

-- 
Olaf Dabrunz (oda <at> fctrace.org)

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to