vim7: Expression with side effects passed to repeated parameter in macro

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

vim7: Expression with side effects passed to repeated parameter in macro

Walter Briscoe
After acceptance of my changes of extern to static with PC-lint, I am
emboldened to suggest other changes.

As far as I know, all the code I am focussing on works without problem.

I follow with two  examples of the sort of thing I mean:
1)
        if (!WHITECHAR(gchar_cursor())

After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
        if (!(((gchar_cursor()) == ' ' || (gchar_cursor()) == '\t') && ...
My change eliminates the double gchar_cursor() call.
The corresponding lines are:
        gc = gchar_cursor();
        if (!(((gc) == ' ' || (gc) == '\t') && ...

2)
                    putc(hist_type2char(type, TRUE), fp);
After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
                    (--(fp)->_cnt >= 0 ? 0xff & (*(fp)->_ptr++ =
(char)(hist_type2char(type, 1))) : _flsbuf((hist_type2char(type,
1)),(fp)));

The corresponding line after my change is:
                    fputc(hist_type2char(type, 1), fp);


--
Walter Briscoe

t.t.bz2 (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: vim7: Expression with side effects passed to repeated parameter in macro

Bram Moolenaar

Walter Briscoe wrote:

> After acceptance of my changes of extern to static with PC-lint, I am
> emboldened to suggest other changes.
>
> As far as I know, all the code I am focussing on works without problem.
>
> I follow with two  examples of the sort of thing I mean:
> 1)
>         if (!WHITECHAR(gchar_cursor())
>
> After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
>         if (!(((gchar_cursor()) == ' ' || (gchar_cursor()) == '\t') && ...
> My change eliminates the double gchar_cursor() call.
> The corresponding lines are:
>         gc = gchar_cursor();
>         if (!(((gc) == ' ' || (gc) == '\t') && ...
>
> 2)
>                     putc(hist_type2char(type, TRUE), fp);
> After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
>                     (--(fp)->_cnt >= 0 ? 0xff & (*(fp)->_ptr++ =
> (char)(hist_type2char(type, 1))) : _flsbuf((hist_type2char(type,
> 1)),(fp)));
>
> The corresponding line after my change is:
>                     fputc(hist_type2char(type, 1), fp);

Thanks.  Looks good, I'll include it.

--
How To Keep A Healthy Level Of Insanity:
14. Put mosquito netting around your work area. Play a tape of jungle
    sounds all day.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///
Reply | Threaded
Open this post in threaded view
|

Re: vim7: Expression with side effects passed to repeated parameter in macro

Walter Briscoe
In message <[hidden email]> of Fri, 3 Jun
2005 12:15:32 in , Bram Moolenaar <[hidden email]> writes
>
>Walter Briscoe wrote:
[snip]

>Thanks.  Looks good, I'll include it.

Wow! You have been busy! I just synchronised and noticed a couple of
things I want to question:

1) Why not this?
D:\wfb\vim\bld ) diff -u vim7.net/vim7/src/buffer.c vim70aa/vim7/src
--- vim7.net/vim7/src/buffer.c  Sun Jun  5 15:29:46 2005
+++ vim70aa/vim7/src/buffer.c   Sun Jun  5 13:29:48 2005
@@ -2450,7 +2450,7 @@
        else
            home_replace(buf, buf->b_fname, NameBuff, MAXPATHL, TRUE);

-       vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
+       len = vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
                buf->b_fnum,
                buf->b_p_bl ? ' ' : 'u',
                buf == curbuf ? '%' :
@@ -2463,7 +2463,6 @@
                NameBuff);

        /* put "line 999" in column 40 or after the file name */
-       len = STRLEN(IObuff);
        i = 40 - vim_strsize(IObuff);
        do
        {

D:\wfb\vim\bld ) diff -u vim7.net/vim7/src/mark.c vim70aa/vim7/src
--- vim7.net/vim7/src/mark.c    Sun Jun  5 15:33:54 2005
+++ vim70aa/vim7/src/mark.c     Sun Jun  5 13:37:54 2005
@@ -1445,7 +1445,7 @@
     char_u  *p;
     char_u  part[51];
     int            retval = FALSE;
-    int            n;
+    size_t  n;

     name = home_replace_save(NULL, name);
     if (name != NULL)

D:\wfb\vim\bld )

2) In removable() in mark.c, I declared a size_t variable which is used
as foo = STRLEN(part + 1). You changed the type to int. That seems
strange to me given
vim.h:#define STRLEN(s)     strlen((char *)(s))

It was REALLY interesting to see the other small changes you made to my
suggestions. It is just this last one I find strange.
--
Walter Briscoe
Reply | Threaded
Open this post in threaded view
|

Re: vim7: Expression with side effects passed to repeated parameter in macro

Bram Moolenaar

Walter Briscoe wrote:

> Wow! You have been busy! I just synchronised and noticed a couple of
> things I want to question:
>
> 1) Why not this?
> D:\wfb\vim\bld ) diff -u vim7.net/vim7/src/buffer.c vim70aa/vim7/src
> --- vim7.net/vim7/src/buffer.c  Sun Jun  5 15:29:46 2005
> +++ vim70aa/vim7/src/buffer.c   Sun Jun  5 13:29:48 2005
> @@ -2450,7 +2450,7 @@
>         else
>             home_replace(buf, buf->b_fname, NameBuff, MAXPATHL, TRUE);
>
> -       vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
> +       len = vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
>                 buf->b_fnum,
>                 buf->b_p_bl ? ' ' : 'u',
>                 buf == curbuf ? '%' :
> @@ -2463,7 +2463,6 @@
>                 NameBuff);
>
>         /* put "line 999" in column 40 or after the file name */
> -       len = STRLEN(IObuff);

Yes, the return value of this snprintf() is reliable, we can use it.

> --- vim7.net/vim7/src/mark.c    Sun Jun  5 15:33:54 2005
> +++ vim70aa/vim7/src/mark.c     Sun Jun  5 13:37:54 2005
> @@ -1445,7 +1445,7 @@
>      char_u  *p;
>      char_u  part[51];
>      int            retval = FALSE;
> -    int            n;
> +    size_t  n;
>
>      name = home_replace_save(NULL, name);
>      if (name != NULL)

Actually, we can do without "n" when moving the STRLEN() to inside
function call.

--
To define recursion, we must first define recursion.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///
Reply | Threaded
Open this post in threaded view
|

Re: vim7: Expression with side effects passed to repeated parameter in macro

Walter Briscoe
In message <[hidden email]> of Sun, 5 Jun
2005 21:07:12 in , Bram Moolenaar <[hidden email]> writes
>
>Walter Briscoe wrote:

[ Near buffer.c(2450), I suggested foo=vim_sprintf(bar, ...) better than
vim_sprintf(bar, ...); foo=STRLEN(bar) ]

>Yes, the return value of this snprintf() is reliable, we can use it.
Good!


[ Near mark.c(1445), I suggested size_t n; is better than int n; given
the usage of n is n = STRLEN(part + 1)]

>Actually, we can do without "n" when moving the STRLEN() to inside
>function call.

Currently, we have:
                n = STRLEN(part + 1);
                if (MB_STRNICMP(part + 1, name, n) == 0)
I think moving STRLEN into MB_STRNICMP is a bad idea as:
1) it is easier to control the current code in a debugger;
2) Code to call STRLEN is likely to be duplicated in MB_STRNICMP given:
vim.h:# define MB_STRNICMP(d, s, n)     (has_mbyte ? mb_strnicmp((char_u
*)(d), (char_u *)(s), (int)(n)) : STRNICMP((d), (s), (n)))

Perhaps a comment about the multiple evaluation should be added.
I did not know such comments might be needed when suggesting the change.

Should the FEAT_MBYTE MB_STRNICMP definition map to a function call so
that the logic complexity is not replicated by each use of MB_STRNICMP?
--
Walter Briscoe