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

[BUG] handle_subscript is unable to handle (expr1)(args) expression while skipping

Expand Messages
  • ZyX
    Try the following code: vim -u NONE -N -c echo (0 && (function( tr ))(1, 2, 3)) . This will throw ôE110: Missing ) ö error while it should throw nothing
    Message 1 of 9 , Oct 14, 2013
    • 0 Attachment
      Try the following code:

      vim -u NONE -N -c "echo (0 && (function('tr'))(1, 2, 3))"

      . This will throw “E110: Missing ')'” error while it should throw nothing (after changing 0 to 1 it works fine effectively showing that I have no errors on && right side). I guess we must just allow handle_subscript handle any kind of subscripts when skipping (disable checks for rettv->v_type): this should also solve problem with “dict.0key_that_starts_with_number”.

      --
      --
      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/groups/opt_out.
    • ZyX
      ... For expr7() such solution seem to work. For expr7.key this is not as simple: I immediately found problems with dot used for string concatenation. Patch for
      Message 2 of 9 , Oct 15, 2013
      • 0 Attachment
        On Monday, October 14, 2013 11:40:48 PM UTC+4, ZyX wrote:
        > Try the following code:
        >
        > vim -u NONE -N -c "echo (0 && (function('tr'))(1, 2, 3))"
        >
        > . This will throw “E110: Missing ')'” error while it should throw nothing (after changing 0 to 1 it works fine effectively showing that I have no errors on && right side). I guess we must just allow handle_subscript handle any kind of subscripts when skipping (disable checks for rettv->v_type): this should also solve problem with “dict.0key_that_starts_with_number”.

        For expr7() such solution seem to work. For expr7.key this is not as simple: I immediately found problems with dot used for string concatenation. Patch for expr7(key) is attached.

        Note: patch is known to pass all tests, but I have not checked it under valgrind (seems to not introduce any problems). No tests yet: not sure where to put it. Test is as simple as checking that `echo (0 && (function('tr'))(1, 2, 3))` echoes zero and no errors: output captured with `:redir` should contain only a few newlines and a zero.

        # HG changeset patch
        # User ZyX <kp-pav@...>
        # Date 1381860915 -14400
        # Tue Oct 15 22:15:15 2013 +0400
        # Node ID 3f2e288daa4b92ea22139b213621e90b20f500f0
        # Parent 92c9748e0ccbc42a5e28ce8fb9b8818e756a06da
        When skipping allow any expression to pretend being a function

        diff -r 92c9748e0ccb -r 3f2e288daa4b src/eval.c
        --- a/src/eval.c Sun Oct 06 17:46:56 2013 +0200
        +++ b/src/eval.c Tue Oct 15 22:15:15 2013 +0400
        @@ -19817,24 +19817,30 @@
        while (ret == OK
        && (**arg == '['
        || (**arg == '.' && rettv->v_type == VAR_DICT)
        - || (**arg == '(' && rettv->v_type == VAR_FUNC))
        + || (**arg == '(' && (!evaluate || rettv->v_type == VAR_FUNC)))
        && !vim_iswhite(*(*arg - 1)))
        {
        if (**arg == '(')
        {
        /* need to copy the funcref so that we can clear rettv */
        - functv = *rettv;
        - rettv->v_type = VAR_UNKNOWN;
        -
        - /* Invoke the function. Recursive! */
        - s = functv.vval.v_string;
        + if (evaluate)
        + {
        + functv = *rettv;
        + rettv->v_type = VAR_UNKNOWN;
        +
        + /* Invoke the function. Recursive! */
        + s = functv.vval.v_string;
        + }
        + else
        + s = "";
        ret = get_func_tv(s, (int)STRLEN(s), rettv, arg,
        curwin->w_cursor.lnum, curwin->w_cursor.lnum,
        &len, evaluate, selfdict);

        /* Clear the funcref afterwards, so that deleting it while
        * evaluating the arguments is possible (see test55). */
        - clear_tv(&functv);
        + if (evaluate)
        + clear_tv(&functv);

        /* Stop the expression evaluation when immediately aborting on
        * error, or when an interrupt occurred or an exception was thrown

        --
        --
        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/groups/opt_out.
      • ZyX
        ... By the way, test75 has confusing header: it claims it contains ôTests for functionsö, but it actually tests only maparg(). Its header also does not
        Message 3 of 9 , Oct 15, 2013
        • 0 Attachment
          > Note: patch is known to pass all tests, but I have not checked it under valgrind (seems to not introduce any problems). No tests yet: not sure where to put it. Test is as simple as checking that `echo (0 && (function('tr'))(1, 2, 3))` echoes zero and no errors: output captured with `:redir` should contain only a few newlines and a zero.

          By the way, test75 has confusing header: it claims it contains “Tests for functions”, but it actually tests only maparg(). Its header also does not conform other tests: it starts with a `"`.

          --
          --
          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/groups/opt_out.
        • Bram Moolenaar
          ... Thanks for the patch. Since you are fixing something that is an easily reproducible problem, please add a test for it. -- LAUNCELOT: I am, sir. I am a
          Message 4 of 9 , Oct 20, 2013
          • 0 Attachment
            ZyX wrote:

            > On Monday, October 14, 2013 11:40:48 PM UTC+4, ZyX wrote:
            > > Try the following code:
            > >
            > > vim -u NONE -N -c "echo (0 && (function('tr'))(1, 2, 3))"
            > >
            > > . This will throw “E110: Missing ')'” error while it should throw
            > > nothing (after changing 0 to 1 it works fine effectively showing
            > > that I have no errors on && right side). I guess we must just allow
            > > handle_subscript handle any kind of subscripts when skipping
            > > (disable checks for rettv->v_type): this should also solve problem
            > > with “dict.0key_that_starts_with_number”.
            >
            > For expr7() such solution seem to work. For expr7.key this is not as
            > simple: I immediately found problems with dot used for string
            > concatenation. Patch for expr7(key) is attached.
            >
            > Note: patch is known to pass all tests, but I have not checked it
            > under valgrind (seems to not introduce any problems). No tests yet:
            > not sure where to put it. Test is as simple as checking that `echo (0
            > && (function('tr'))(1, 2, 3))` echoes zero and no errors: output
            > captured with `:redir` should contain only a few newlines and a zero.

            Thanks for the patch.

            Since you are fixing something that is an easily reproducible problem,
            please add a test for it.


            --
            LAUNCELOT: I am, sir. I am a Knight of King Arthur.
            FATHER: 'Mm ... very nice castle, Camelot ... very good pig country....
            "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

            /// 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/groups/opt_out.
          • Bram Moolenaar
            ... I ll fix that. -- GUEST: He s killed the best man! SECOND GUEST: (holding a limp WOMAN) He s killed my auntie. FATHER: No, please! This is
            Message 5 of 9 , Oct 20, 2013
            • 0 Attachment
              ZyX wrote:

              > > Note: patch is known to pass all tests, but I have not checked it
              > > under valgrind (seems to not introduce any problems). No tests yet:
              > > not sure where to put it. Test is as simple as checking that `echo (0
              > > && (function('tr'))(1, 2, 3))` echoes zero and no errors: output
              > > captured with `:redir` should contain only a few newlines and a zero.
              >
              > By the way, test75 has confusing header: it claims it contains
              > “Tests for functions”, but it actually tests only maparg(). Its
              > header also does not conform other tests: it starts with a `"`.

              I'll fix that.

              --
              GUEST: He's killed the best man!
              SECOND GUEST: (holding a limp WOMAN) He's killed my auntie.
              FATHER: No, please! This is supposed to be a happy occasion! Let's
              not bicker and argue about who killed who ...
              "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

              /// 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/groups/opt_out.
            • Nikolay Pavlov
              ... That was the question: where? AFAIR I searched test headers for expr and func , but have not found anything appropriate. ... PICTURES LTD ... /// -- --
              Message 6 of 9 , Oct 20, 2013
              • 0 Attachment


                On Oct 20, 2013 7:24 PM, "Bram Moolenaar" <Bram@...> wrote:
                >
                >
                > ZyX wrote:
                >
                > > On Monday, October 14, 2013 11:40:48 PM UTC+4, ZyX wrote:
                > > > Try the following code:
                > > >
                > > >     vim -u NONE -N -c "echo (0 && (function('tr'))(1, 2, 3))"
                > > >
                > > > . This will throw “E110: Missing ')'” error while it should throw
                > > > nothing (after changing 0 to 1 it works fine effectively showing
                > > > that I have no errors on && right side). I guess we must just allow
                > > > handle_subscript handle any kind of subscripts when skipping
                > > > (disable checks for rettv->v_type): this should also solve problem
                > > > with “dict.0key_that_starts_with_number”.
                > >
                > > For expr7() such solution seem to work. For expr7.key this is not as
                > > simple: I immediately found problems with dot used for string
                > > concatenation. Patch for expr7(key) is attached.
                > >
                > > Note: patch is known to pass all tests, but I have not checked it
                > > under valgrind (seems to not introduce any problems). No tests yet:
                > > not sure where to put it. Test is as simple as checking that `echo (0
                > > && (function('tr'))(1, 2, 3))` echoes zero and no errors: output
                > > captured with `:redir` should contain only a few newlines and a zero.
                >
                > Thanks for the patch.
                >
                > Since you are fixing something that is an easily reproducible problem,
                > please add a test for it.

                That was the question: where? AFAIR I searched test headers for "expr" and "func", but have not found anything appropriate.

                >
                > --
                > LAUNCELOT: I am, sir. I am a Knight of King Arthur.
                > FATHER:    'Mm ... very nice castle, Camelot ... very good pig country....
                >                  "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD
                >
                >  /// 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/groups/opt_out.
              • Bram Moolenaar
                ... Perhaps in test 26? It has some other eval stuff. Or test 34, it s about calling functions. -- Be nice to your kids... they ll be the ones choosing your
                Message 7 of 9 , Oct 20, 2013
                • 0 Attachment
                  ZyX wrote:

                  > On Oct 20, 2013 7:24 PM, "Bram Moolenaar" <Bram@...> wrote:
                  > >
                  > > ZyX wrote:
                  > >
                  > > > On Monday, October 14, 2013 11:40:48 PM UTC+4, ZyX wrote:
                  > > > > Try the following code:
                  > > > >
                  > > > > vim -u NONE -N -c "echo (0 && (function('tr'))(1, 2, 3))"
                  > > > >
                  > > > > . This will throw =93E110: Missing ')'=94 error while it should throw
                  > > > > nothing (after changing 0 to 1 it works fine effectively showing
                  > > > > that I have no errors on && right side). I guess we must just allow
                  > > > > handle_subscript handle any kind of subscripts when skipping
                  > > > > (disable checks for rettv->v_type): this should also solve problem
                  > > > > with =93dict.0key_that_starts_with_number=94.
                  > > >
                  > > > For expr7() such solution seem to work. For expr7.key this is not as
                  > > > simple: I immediately found problems with dot used for string
                  > > > concatenation. Patch for expr7(key) is attached.
                  > > >
                  > > > Note: patch is known to pass all tests, but I have not checked it
                  > > > under valgrind (seems to not introduce any problems). No tests yet:
                  > > > not sure where to put it. Test is as simple as checking that `echo (0
                  > > > && (function('tr'))(1, 2, 3))` echoes zero and no errors: output
                  > > > captured with `:redir` should contain only a few newlines and a zero.
                  > >
                  > > Thanks for the patch.
                  > >
                  > > Since you are fixing something that is an easily reproducible problem,
                  > > please add a test for it.
                  >
                  > That was the question: where? AFAIR I searched test headers for "expr" and
                  > "func", but have not found anything appropriate.

                  Perhaps in test 26? It has some other eval stuff.
                  Or test 34, it's about calling functions.

                  --
                  Be nice to your kids... they'll be the ones choosing your nursing home.

                  /// 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/groups/opt_out.
                • ZyX
                  Here is the updated patch: # HG changeset patch # User ZyX # Date 1381860915 -14400 # Tue Oct 15 22:15:15 2013 +0400 # Node ID
                  Message 8 of 9 , Oct 23, 2013
                  • 0 Attachment
                    Here is the updated patch:

                    # HG changeset patch
                    # User ZyX <kp-pav@...>
                    # Date 1381860915 -14400
                    # Tue Oct 15 22:15:15 2013 +0400
                    # Node ID 3f2e288daa4b92ea22139b213621e90b20f500f0
                    # Parent 92c9748e0ccbc42a5e28ce8fb9b8818e756a06da
                    When skipping allow any expression to pretend being a function

                    diff -r 92c9748e0ccb -r 3f2e288daa4b src/eval.c
                    --- a/src/eval.c Sun Oct 06 17:46:56 2013 +0200
                    +++ b/src/eval.c Tue Oct 15 22:15:15 2013 +0400
                    @@ -19817,24 +19817,30 @@
                    while (ret == OK
                    && (**arg == '['
                    || (**arg == '.' && rettv->v_type == VAR_DICT)
                    - || (**arg == '(' && rettv->v_type == VAR_FUNC))
                    + || (**arg == '(' && (!evaluate || rettv->v_type == VAR_FUNC)))
                    && !vim_iswhite(*(*arg - 1)))
                    {
                    if (**arg == '(')
                    {
                    /* need to copy the funcref so that we can clear rettv */
                    - functv = *rettv;
                    - rettv->v_type = VAR_UNKNOWN;
                    -
                    - /* Invoke the function. Recursive! */
                    - s = functv.vval.v_string;
                    + if (evaluate)
                    + {
                    + functv = *rettv;
                    + rettv->v_type = VAR_UNKNOWN;
                    +
                    + /* Invoke the function. Recursive! */
                    + s = functv.vval.v_string;
                    + }
                    + else
                    + s = "";
                    ret = get_func_tv(s, (int)STRLEN(s), rettv, arg,
                    curwin->w_cursor.lnum, curwin->w_cursor.lnum,
                    &len, evaluate, selfdict);

                    /* Clear the funcref afterwards, so that deleting it while
                    * evaluating the arguments is possible (see test55). */
                    - clear_tv(&functv);
                    + if (evaluate)
                    + clear_tv(&functv);

                    /* Stop the expression evaluation when immediately aborting on
                    * error, or when an interrupt occurred or an exception was thrown
                    # HG changeset patch
                    # User ZyX <kp-pav@...>
                    # Date 1382540204 -14400
                    # Wed Oct 23 18:56:44 2013 +0400
                    # Node ID 9573bf9b8634a5c7d315e7009e911fbe4577c847
                    # Parent 3f2e288daa4b92ea22139b213621e90b20f500f0
                    Add regression test

                    diff -r 3f2e288daa4b -r 9573bf9b8634 src/testdir/test34.in
                    --- a/src/testdir/test34.in Tue Oct 15 22:15:15 2013 +0400
                    +++ b/src/testdir/test34.in Wed Oct 23 18:56:44 2013 +0400
                    @@ -1,6 +1,7 @@
                    Test for user functions.
                    Also test an <expr> mapping calling a function.
                    Also test that a builtin function cannot be replaced.
                    +Also test for regression when calling arbitrary expression.

                    STARTTEST
                    :so small.vim
                    @@ -62,7 +63,17 @@
                    [(one again :call append(line('$'), max([1, 2, 3]))
                    :call extend(g:, {'max': function('min')})
                    :call append(line('$'), max([1, 2, 3]))
                    -:$-7,$w! test.out
                    +:try
                    +: " Regression: the first line below used to throw “E110: Missing ')'”
                    +: " Second is here just to prove that this line is correct when not skipping
                    +: " rhs of &&.
                    +: $put =(0&&(function('tr'))(1, 2, 3))
                    +: $put =(1&&(function('tr'))(1, 2, 3))
                    +:catch
                    +: $put ='!!! Unexpected exception:'
                    +: $put =v:exception
                    +:endtry
                    +:$-9,$w! test.out
                    :delfunc Table
                    :delfunc Compute
                    :delfunc Expr1
                    diff -r 3f2e288daa4b -r 9573bf9b8634 src/testdir/test34.ok
                    --- a/src/testdir/test34.ok Tue Oct 15 22:15:15 2013 +0400
                    +++ b/src/testdir/test34.ok Wed Oct 23 18:56:44 2013 +0400
                    @@ -6,3 +6,5 @@
                    1. one again
                    3
                    3
                    +0
                    +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/groups/opt_out.
                  • Bram Moolenaar
                    ... Thanks! -- Save the plankton - eat a whale. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net /// sponsor Vim, vote for
                    Message 9 of 9 , Oct 23, 2013
                    • 0 Attachment
                      ZyX wrote:

                      > Here is the updated patch:
                      >
                      > # HG changeset patch
                      > # User ZyX <kp-pav@...>
                      > # Date 1381860915 -14400
                      > # Tue Oct 15 22:15:15 2013 +0400
                      > # Node ID 3f2e288daa4b92ea22139b213621e90b20f500f0
                      > # Parent 92c9748e0ccbc42a5e28ce8fb9b8818e756a06da
                      > When skipping allow any expression to pretend being a function

                      Thanks!

                      --
                      Save the plankton - eat a whale.

                      /// 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/groups/opt_out.
                    Your message has been successfully submitted and would be delivered to recipients shortly.