Loading ...
Sorry, an error occurred while loading the content.

[BUG] Lockvar does not work well with slices

Expand Messages
  • ZyX
    1. List item locked with lockvar may be set using slice assignment: let l = [1, 2, 3, 4] lockvar! l unlockvar l[1] let l[0:1] = [0, 1] ^ Reports E741 echo
    Message 1 of 9 , Aug 17, 2014
    • 0 Attachment
      1. List item locked with lockvar may be set using slice assignment:

      let l = [1, 2, 3, 4]
      lockvar! l
      unlockvar l[1]
      let l[0:1] = [0, 1] " ^ Reports E741
      echo l " [1, 2, 3, 4]
      let l[1:2] = [0, 1]
      echo l " [1, 0, 1, 4]

      2. Same for :unlet: if the first item in the slice was not locked :unlet may delete the whole slice even though other values in the slice are locked.

      --
      --
      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+unsubscribe@....
      For more options, visit https://groups.google.com/d/optout.
    • Bram Moolenaar
      ... So, what do you suggest? Do an or of the lock flag for all the items in the slice? -- My sister Cecilia opened a computer store in Hawaii. She sells C
      Message 2 of 9 , Aug 17, 2014
      • 0 Attachment
        ZyX wrote:

        > 1. List item locked with lockvar may be set using slice assignment:
        >
        > let l = [1, 2, 3, 4]
        > lockvar! l
        > unlockvar l[1]
        > let l[0:1] = [0, 1] " ^ Reports E741
        > echo l " [1, 2, 3, 4]
        > let l[1:2] = [0, 1]
        > echo l " [1, 0, 1, 4]
        >
        > 2. Same for :unlet: if the first item in the slice was not locked
        > :unlet may delete the whole slice even though other values in the
        > slice are locked.

        So, what do you suggest? Do an "or" of the lock flag for all the items
        in the slice?

        --
        My sister Cecilia opened a computer store in Hawaii.
        She sells C shells by the seashore.

        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
        /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
        \\\ an exciting new programming language -- http://www.Zimbu.org ///
        \\\ help me help AIDS victims -- http://ICCF-Holland.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+unsubscribe@....
        For more options, visit https://groups.google.com/d/optout.
      • ZyX
        ... I see three possibilities: 1. Check locks in the same cycle listitem_remove is called (so items that are not locked will be removed). 2. Add cycle just
        Message 3 of 9 , Aug 17, 2014
        • 0 Attachment
          > So, what do you suggest? Do an "or" of the lock flag for all the items
          > in the slice?

          I see three possibilities:

          1. Check locks in the same cycle listitem_remove is called (so items that are not locked will be removed).
          2. Add cycle just before it which will check locks before removal (so items will be removed only if all items are not locked).
          3. Do not check locks at all (happens for `:unlet var`, but not for `:unlet g:['var']`).

          First and third are the easiest ones (I briefly checked only second one!):

          First:

          diff -r 18fd959b07ef src/eval.c
          --- a/src/eval.c Ср авг 13 22:05:54 2014 +0200
          +++ b/src/eval.c Вс авг 17 21:19:53 2014 +0400
          @@ -3651,7 +3651,8 @@ do_unlet_var(lp, name_end, forceit)
          while (lp->ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= lp->ll_n1))
          {
          li = lp->ll_li->li_next;
          - listitem_remove(lp->ll_list, lp->ll_li);
          + if (!tv_check_lock(li->li_tv.v_lock, lp->ll_name))
          + listitem_remove(lp->ll_list, lp->ll_li);
          lp->ll_li = li;
          ++lp->ll_n1;
          }

          Second:

          diff -r 18fd959b07ef src/eval.c
          --- a/src/eval.c Ср авг 13 22:05:54 2014 +0200
          +++ b/src/eval.c Вс авг 17 21:25:13 2014 +0400
          @@ -3646,6 +3646,17 @@ do_unlet_var(lp, name_end, forceit)
          else if (lp->ll_range)
          {
          listitem_T *li;
          + listitem_T *ll_li = lp->ll_li;
          + int ll_n1 = lp->ll_n1;
          +
          + while (ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= ll_n1))
          + {
          + li = ll_li->li_next;
          + if (tv_check_lock(li->li_tv.v_lock, lp->ll_name))
          + return FAIL;
          + ll_li = li;
          + ++ll_n1;
          + }

          /* Delete a range of List items. */
          while (lp->ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= lp->ll_n1))


          Third:

          diff -r 18fd959b07ef src/eval.c
          --- a/src/eval.c Ср авг 13 22:05:54 2014 +0200
          +++ b/src/eval.c Вс авг 17 21:17:53 2014 +0400
          @@ -3641,8 +3641,6 @@ do_unlet_var(lp, name_end, forceit)
          ret = FAIL;
          *name_end = cc;
          }
          - else if (tv_check_lock(lp->ll_tv->v_lock, lp->ll_name))
          - return FAIL;
          else if (lp->ll_range)
          {
          listitem_T *li;

          --
          --
          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+unsubscribe@....
          For more options, visit https://groups.google.com/d/optout.
        • ZyX
          And fourth: behave like map: do its job until lock occurs and fail at the first lock. * The above code was for :unlet, for :let slice assignment there should
          Message 4 of 9 , Aug 17, 2014
          • 0 Attachment
            And fourth: behave like map: do its job until lock occurs and fail at the first lock.

            * The above code was for :unlet, for :let slice assignment there should be something similar (except that option 3. will no longer be consistent with anything).

            --
            --
            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+unsubscribe@....
            For more options, visit https://groups.google.com/d/optout.
          • ZyX
            ... Full patch for fourth approach (with tests): diff -r 18fd959b07ef src/eval.c ... +++ b/src/eval.c Sun Aug 17 21:44:48 2014 +0400 @@ -2950,6 +2950,8 @@
            Message 5 of 9 , Aug 17, 2014
            • 0 Attachment
              On Sunday, August 17, 2014 9:28:25 PM UTC+4, ZyX wrote:
              > And fourth: behave like map: do its job until lock occurs and fail at the first lock.
              > * The above code was for :unlet, for :let slice assignment there should be something similar (except that option 3. will no longer be consistent with anything).

              Full patch for fourth approach (with tests):

              diff -r 18fd959b07ef src/eval.c
              --- a/src/eval.c Wed Aug 13 22:05:54 2014 +0200
              +++ b/src/eval.c Sun Aug 17 21:44:48 2014 +0400
              @@ -2950,6 +2950,8 @@ set_var_lval(lp, endp, rettv, copy, op)
              */
              for (ri = rettv->vval.v_list->lv_first; ri != NULL; )
              {
              + if (tv_check_lock(lp->ll_li->li_tv.v_lock, lp->ll_name))
              + return;
              if (op != NULL && *op != '=')
              tv_op(&lp->ll_li->li_tv, &ri->li_tv, op);
              else
              @@ -3651,6 +3653,8 @@ do_unlet_var(lp, name_end, forceit)
              while (lp->ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= lp->ll_n1))
              {
              li = lp->ll_li->li_next;
              + if (tv_check_lock(lp->ll_li->li_tv.v_lock, lp->ll_name))
              + return FAIL;
              listitem_remove(lp->ll_list, lp->ll_li);
              lp->ll_li = li;
              ++lp->ll_n1;
              diff -r 18fd959b07ef src/testdir/test55.in
              --- a/src/testdir/test55.in Wed Aug 13 22:05:54 2014 +0200
              +++ b/src/testdir/test55.in Sun Aug 17 21:44:48 2014 +0400
              @@ -282,6 +282,21 @@ let l = [0, 1, 2, 3]
              : $put =ps
              : endfor
              :endfor
              +:unlet l
              +:let l = [1, 2, 3, 4]
              +:lockvar! l
              +:$put =string(l)
              +:unlockvar l[1]
              +:unlet l[0:1]
              +:$put =string(l)
              +:unlet l[1:2]
              +:$put =string(l)
              +:unlockvar l[1]
              +:let l[0:1] = [0, 1]
              +:$put =string(l)
              +:let l[1:2] = [0, 1]
              +:$put =string(l)
              +:unlet l
              :" :lockvar/islocked() triggering script autoloading
              :set rtp+=./sautest
              :lockvar g:footest#x
              diff -r 18fd959b07ef src/testdir/test55.ok
              --- a/src/testdir/test55.ok Wed Aug 13 22:05:54 2014 +0200
              +++ b/src/testdir/test55.ok Sun Aug 17 21:44:48 2014 +0400
              @@ -86,6 +86,11 @@ 0011-011
              FFpFFpp
              0000-000
              ppppppp
              +[1, 2, 3, 4]
              +[1, 2, 3, 4]
              +[1, 3, 4]
              +[1, 3, 4]
              +[1, 0, 4]
              locked g:footest#x:-1
              exists g:footest#x:0
              g:footest#x: 1

              --
              --
              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+unsubscribe@....
              For more options, visit https://groups.google.com/d/optout.
            • Bram Moolenaar
              ... Thanks for providing so many alternatives. In my opinion what most people expect is that it either works, removes all items, or fails, does not remove any
              Message 6 of 9 , Aug 18, 2014
              • 0 Attachment
                ZyX wrote:

                > On Sunday, August 17, 2014 9:28:25 PM UTC+4, ZyX wrote:
                > > And fourth: behave like map: do its job until lock occurs and fail at the first lock.
                > > * The above code was for :unlet, for :let slice assignment there should be something similar (except that option 3. will no longer be consistent with anything).
                >
                > Full patch for fourth approach (with tests):

                Thanks for providing so many alternatives. In my opinion what most
                people expect is that it either works, removes all items, or fails, does
                not remove any items.

                Running map is different, since it's clear the function is called for
                one item at a time. While removing is expected to do the whole slice at
                once.

                I understand that the second solution is more work to implement, but
                user expectation is more important.

                Can you make a test for the second solution? The test for the fourth
                solution should already be close.


                --
                hundred-and-one symptoms of being an internet addict:
                38. You wake up at 3 a.m. to go to the bathroom and stop and check your e-mail
                on the way back to bed.

                /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                \\\ an exciting new programming language -- http://www.Zimbu.org ///
                \\\ help me help AIDS victims -- http://ICCF-Holland.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+unsubscribe@....
                For more options, visit https://groups.google.com/d/optout.
              • Павлов Николай Алекса
                ... Hash: SHA512 ... I would not say so for map(). In most languages it creates a new list, so it is expected to exhibit do everything or fail having done
                Message 7 of 9 , Aug 18, 2014
                • 0 Attachment
                  -----BEGIN PGP SIGNED MESSAGE-----
                  Hash: SHA512

                  On August 18, 2014 5:16:44 PM GMT+03:00, Bram Moolenaar <Bram@...> wrote:
                  >
                  >ZyX wrote:
                  >
                  >> On Sunday, August 17, 2014 9:28:25 PM UTC+4, ZyX wrote:
                  >> > And fourth: behave like map: do its job until lock occurs and fail
                  >at the first lock.
                  >> > * The above code was for :unlet, for :let slice assignment there
                  >should be something similar (except that option 3. will no longer be
                  >consistent with anything).
                  >>
                  >> Full patch for fourth approach (with tests):
                  >
                  >Thanks for providing so many alternatives. In my opinion what most
                  >people expect is that it either works, removes all items, or fails,
                  >does
                  >not remove any items.
                  >
                  >Running map is different, since it's clear the function is called for
                  >one item at a time. While removing is expected to do the whole slice
                  >at
                  >once.

                  I would not say so for map(). In most languages it creates a new list, so it is expected to exhibit "do everything or fail having done nothing" behavior. If it is to be thought about as a :for cycle replacement then it is expected to exhibit "do everything what can be done (unless in :try)". There are also languages where map() returns an iterator in which case (depending on code) it may be even "do everything what can be done" (though with iterator you normally may have any behavior you want, but usual uses exhibit same "stop at first error" as Vim... if "usual use" is foreach cycle: other usual use is creating a list in which case it is the first variant). In any case I would not expect it to stop at first error, still having done something: either do not stop or do not do.

                  Of course this only applies to pure (without side effects) expressions in map() second argument.

                  And Vim constantly exhibits this behavior: does what it can until error occurs, sometimes even after error occurs. E.g. consider

                  let var = (system('some-cmd-with-side-effect')

                  : command will be run, but due to missing parenthesis var will not be assigned anything. Since VimL is the only language I use that does not have a parsing stage this behavior is highly unexpected: all other languages will at least not execute anything here, not to mention the vast majority that will not execute any code in the same file at all. So fourth variant may be not expected, but it definitely is consistent, not only with map(). After working with VimL a few years this will even be expected.

                  >
                  >I understand that the second solution is more work to implement, but
                  >user expectation is more important.

                  It is rather trivial as well: "more work" is only by comparison. Most of work (finding a place to fix for :let slice assignment) was done anyway.

                  >
                  >Can you make a test for the second solution? The test for the fourth
                  >solution should already be close.

                  -----BEGIN PGP SIGNATURE-----
                  Version: APG v1.1.1

                  iQI1BAEBCgAfBQJT8hh6GBxaeVggPHp5eC52aW1AZ21haWwuY29tPgAKCRCf3UKj
                  HhHSvjY6EACH47yFwYOcDU/2+DFYajs+DekQgf5DC6RL+wt2PoFHGW73qfYDF2Aa
                  Ic/T7arGrJvrLneD1zfnZKJQS4w5WlWg2JG8eU6NUvqKfKH5/dtAAnXRRDcQ3oZY
                  RVkmxEh+MtgLv33xjzgLIO698WpG926SxpjNBWt9OxwDM/g4wZni+dZ8ORFsN8EK
                  HflXCSP8jDtu5TiWKqb+n4s2aY2p6fWHINpv9jEnHYEZNUDeG6IaDS7AjNI+/KBT
                  ctwW0hpSt6BUgaZXoegufWeGFFWV3IkEv6jKuDw3kgaYOrwqxeL1iZB+O/aUnbOn
                  yHYte2FXTBWCSUlpfQZDrOVGHY7XXxzZFA3XNEGlyH/n6m7aJzevIDXVhQ/deXyR
                  gApuhmL2od0XD2i0x6I2BRhyDT2mglUmB8TbCC6O8AzTbbgwKgCxa/8zKyq3BE2A
                  e7bQz5DjOtfgvLYEI+UpZaBD1JbK+LzrjHIfpFkRSiMYC+3ZWohss6d+Czw9h3Qv
                  plQLikpWfYvphoJfBOVBIFwOdjFfY2ryO/8Y/VlxOUwXFNQCoXFoEgwaPXQsfDxq
                  K2VOi7Rsc5yrF2GUuGpNtdLEwXzUDjmW6ZlSM2d4YJ+t7Ym+K4ihcUooR83eQfDS
                  FaJ7mRnCaSckNRFx7yqjhc41O+l7iz1bFNxNKw/PF0Q6QuCi7/iA5A==
                  =bMl9
                  -----END PGP SIGNATURE-----

                  --
                  --
                  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+unsubscribe@....
                  For more options, visit https://groups.google.com/d/optout.
                • ZyX
                  Updated patch with second approach: # HG changeset patch # User ZyX # Date 1409287235 -14400 # Fri Aug 29 08:40:35 2014 +0400 # Node ID
                  Message 8 of 9 , Aug 28, 2014
                  • 0 Attachment
                    Updated patch with second approach:

                    # HG changeset patch
                    # User ZyX <kp-pav@...>
                    # Date 1409287235 -14400
                    # Fri Aug 29 08:40:35 2014 +0400
                    # Node ID 93db73f6960f25b2ffb10b12010d524252b0ac4a
                    # Parent 0ccbf92e36c043ec944614b02f4c83f0f41929d6
                    Fix slice operations on partially locked list

                    Patch from https://groups.google.com/forum/#!topic/vim_dev/-IYtF6S4EjM

                    diff -r 0ccbf92e36c0 -r 93db73f6960f src/eval.c
                    --- a/src/eval.c Sun Aug 10 13:46:36 2014 +0200
                    +++ b/src/eval.c Fri Aug 29 08:40:35 2014 +0400
                    @@ -2945,6 +2945,23 @@
                    ;
                    else if (lp->ll_range)
                    {
                    + listitem_T *ll_li = lp->ll_li;
                    + int ll_n1 = lp->ll_n1;
                    +
                    + /*
                    + * Check whether any of the list items is locked
                    + */
                    + for (ri = rettv->vval.v_list->lv_first; ri != NULL; )
                    + {
                    + if (tv_check_lock(ll_li->li_tv.v_lock, lp->ll_name))
                    + return;
                    + ri = ri->li_next;
                    + if (ri == NULL || (!lp->ll_empty2 && lp->ll_n2 == ll_n1))
                    + break;
                    + ll_li = ll_li->li_next;
                    + ++ll_n1;
                    + }
                    +
                    /*
                    * Assign the List values to the list items.
                    */
                    @@ -3646,6 +3663,17 @@
                    else if (lp->ll_range)
                    {
                    listitem_T *li;
                    + listitem_T *ll_li = lp->ll_li;
                    + int ll_n1 = lp->ll_n1;
                    +
                    + while (ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= ll_n1))
                    + {
                    + li = ll_li->li_next;
                    + if (tv_check_lock(ll_li->li_tv.v_lock, lp->ll_name))
                    + return FAIL;
                    + ll_li = li;
                    + ++ll_n1;
                    + }

                    /* Delete a range of List items. */
                    while (lp->ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= lp->ll_n1))
                    diff -r 0ccbf92e36c0 -r 93db73f6960f src/testdir/test55.in
                    --- a/src/testdir/test55.in Sun Aug 10 13:46:36 2014 +0200
                    +++ b/src/testdir/test55.in Fri Aug 29 08:40:35 2014 +0400
                    @@ -282,6 +282,21 @@
                    : $put =ps
                    : endfor
                    :endfor
                    +:unlet l
                    +:let l = [1, 2, 3, 4]
                    +:lockvar! l
                    +:$put =string(l)
                    +:unlockvar l[1]
                    +:unlet l[0:1]
                    +:$put =string(l)
                    +:unlet l[1:2]
                    +:$put =string(l)
                    +:unlockvar l[1]
                    +:let l[0:1] = [0, 1]
                    +:$put =string(l)
                    +:let l[1:2] = [0, 1]
                    +:$put =string(l)
                    +:unlet l
                    :" :lockvar/islocked() triggering script autoloading
                    :set rtp+=./sautest
                    :lockvar g:footest#x
                    diff -r 0ccbf92e36c0 -r 93db73f6960f src/testdir/test55.ok
                    --- a/src/testdir/test55.ok Sun Aug 10 13:46:36 2014 +0200
                    +++ b/src/testdir/test55.ok Fri Aug 29 08:40:35 2014 +0400
                    @@ -86,6 +86,11 @@
                    FFpFFpp
                    0000-000
                    ppppppp
                    +[1, 2, 3, 4]
                    +[1, 2, 3, 4]
                    +[1, 2, 3, 4]
                    +[1, 2, 3, 4]
                    +[1, 2, 3, 4]
                    locked g:footest#x:-1
                    exists g:footest#x:0
                    g:footest#x: 1

                    --
                    --
                    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+unsubscribe@....
                    For more options, visit https://groups.google.com/d/optout.
                  • Bram Moolenaar
                    ... Thanks! It s a bit more work, but I think this is the best solution. We ll see if any plugin has a problem with this, I doubt it though. --
                    Message 9 of 9 , Aug 29, 2014
                    • 0 Attachment
                      ZyX wrote:

                      > Updated patch with second approach:
                      >
                      > # HG changeset patch
                      > # User ZyX <kp-pav@...>
                      > # Date 1409287235 -14400
                      > # Fri Aug 29 08:40:35 2014 +0400
                      > # Node ID 93db73f6960f25b2ffb10b12010d524252b0ac4a
                      > # Parent 0ccbf92e36c043ec944614b02f4c83f0f41929d6
                      > Fix slice operations on partially locked list
                      >
                      > Patch from https://groups.google.com/forum/#!topic/vim_dev/-IYtF6S4EjM

                      Thanks! It's a bit more work, but I think this is the best solution.

                      We'll see if any plugin has a problem with this, I doubt it though.

                      --
                      hundred-and-one symptoms of being an internet addict:
                      89. In addition to your e-mail address being on your business
                      cards you even have your own domain.

                      /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                      /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                      \\\ an exciting new programming language -- http://www.Zimbu.org ///
                      \\\ help me help AIDS victims -- http://ICCF-Holland.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+unsubscribe@....
                      For more options, visit https://groups.google.com/d/optout.
                    Your message has been successfully submitted and would be delivered to recipients shortly.