[Cocci] Change function invocations argument

[Cocci] Change function invocations argument

[Eliseo Martínez]

Hi, 

For a given function, I want to change all invocations in this way:
Turn the second argument of the form ?(long_u)(expression)? into ?(uintmax_t)expression?.
I?ve done a patch for that but is failing with a not very much informative message error.
Here is it:

```
eliseo at ubuntu:~/projects/os/neovim/neovim$ cat sample.cocci 
@@ expression e1, e2, e3; @@

  put_bytes(
    e1,
-   (long_u)(e2),
+   (uintmax_t)e2,
    e3)

eliseo at ubuntu:~/projects/os/neovim/neovim$ spatch --parse-cocci sample.cocci 
init_defs_builtins: /usr/share/coccinelle/standard.h
84 86
Fatal error: exception Failure("plus: parse error: 
 = File "sample.cocci", line 6, column 15,  charpos = 84
    around = 'e2', whole content = +   (uintmax_t)e2,
?)
```

What I?m doing wrong here??
Thanks.

[Cocci] Change function invocations argument

[Julia Lawall]

On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:

> Hi, 
> 
> For a given function, I want to change all invocations in this way:
> Turn the second argument of the form ?(long_u)(expression)? into ?(uintmax_t)expression?.
> I?ve done a patch for that but is failing with a not very much informative message error.
> Here is it:
> 
> ```
> eliseo at ubuntu:~/projects/os/neovim/neovim$ cat sample.cocci 
> @@ expression e1, e2, e3; @@
> 
>   put_bytes(
>     e1,
> -   (long_u)(e2),
> +   (uintmax_t)e2,
>     e3)

long_u and uintmax_t are not part of the C language, and Coccinelle is not 
always able to infer typedefs.  Just add:

typedef long_u, uintmax_t;

in your metavariable list.  This should only be done in the first rule in 
which the typename is relevant.

julia

[Cocci] Change function invocations argument

[Eliseo Martínez]

Great. That worked. Just one more thing, please.
Some of the replacements done by the rule before leave literals of the form ?(uintmax_t)34?.
I would like to use an unsigned constant in those cases. This is, something like ?34u?. 
So I add the following rule:

```
@@ expression e1, e3; constant c2; @@
 30 
 31 - put_bytes(e1, (uintmax_t)c2, e3)
 32 + put_bytes(e1, c2, e3)
```

That does the work, except adding the ?u?. How could I do that?
Is it possible adding something literally in the output?
Is it possible to somehow say in line 32 ?render c2 but with u suffix??

Thanks.
> On 20 Dec 2014, at 21:32, Julia Lawall <julia.lawall@lip6.fr> wrote:
> 
> On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:
> 
>> Hi, 
>> 
>> For a given function, I want to change all invocations in this way:
>> Turn the second argument of the form ?(long_u)(expression)? into ?(uintmax_t)expression?.
>> I?ve done a patch for that but is failing with a not very much informative message error.
>> Here is it:
>> 
>> ```
>> eliseo at ubuntu:~/projects/os/neovim/neovim$ cat sample.cocci 
>> @@ expression e1, e2, e3; @@
>> 
>>  put_bytes(
>>    e1,
>> -   (long_u)(e2),
>> +   (uintmax_t)e2,
>>    e3)
> 
> long_u and uintmax_t are not part of the C language, and Coccinelle is not 
> always able to infer typedefs.  Just add:
> 
> typedef long_u, uintmax_t;
> 
> in your metavariable list.  This should only be done in the first rule in 
> which the typename is relevant.
> 
> julia

[Cocci] Change function invocations argument

[Julia Lawall]

On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:

> Great. That worked. Just one more thing, please.
> Some of the replacements done by the rule before leave literals of the form ?(uintmax_t)34?.
> I would like to use an unsigned constant in those cases. This is, something like ?34u?. 
> So I add the following rule:
> 
> ```
> @@ expression e1, e3; constant c2; @@
>  30 
>  31 - put_bytes(e1, (uintmax_t)c2, e3)
>  32 + put_bytes(e1, c2, e3)
> ```
> 
> That does the work, except adding the ?u?. How could I do that?
> Is it possible adding something literally in the output?
> Is it possible to somehow say in line 32 ?render c2 but with u suffix??

It is possible, but somewhat awkward.  Here is a simpler example that you 
can adapt to your case:

@r@
typedef uintmax_t;
constant c;
@@

(uintmax_t)c

@script:python s@
c << r.c;
cu;
@@

coccinelle.cu = "%su" % (c)

@@
constant r.c;
identifier s.cu;
@@

- (uintmax_t)c
+ cu

julia

[Cocci] Change function invocations argument

[Eliseo Martínez]

Ok. Finally I have it working. There?s still a gotcha, though.
The first version of my spatch contained these rules:

```
@@ typedef long_u, uintmax_t; expression e1, e2, e3; @@

-   put_bytes(e1, (long_u)(e2), e3)
+   put_bytes(e1, (uintmax_t)e2, e3)

@@ expression e1, e2, e3; @@

-   put_bytes(e1, (long_u)e2, e3)
+   put_bytes(e1, (uintmax_t)e2, e3)

@@ expression e1, e2, e3; @@

-   put_bytes(e1, e2, (int)(e3))
+   put_bytes(e1, e2, (unsigned int)e3)

@@ expression e1, e2, e3; @@

-   put_bytes(e1, e2, (int)e3)
+   put_bytes(e1, e2, (unsigned int)e3)

```

Those work ok.
Then, I tried a second version, trying to compact those rules into a single, more expressive one:

```
@@ typedef long_u, uintmax_t; expression e1, e2, e3; @@

    put_bytes(
        e1,
(
-       (long_u)(e2)
|
-       (long_u)e2
)\x10
+       (uintmax_t)e2
        ,
(
-       (int)(e3)
|
-       (int)e3
)
+       (unsigned int)e3
    )

```

That parses ok but just does nothing. So, it seems it doesn?t match anything, but I don?t understand why.
Could you please explain?

> On 20 Dec 2014, at 21:51, Julia Lawall <julia.lawall@lip6.fr> wrote:
> 
> 
> 
> On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:
> 
>> Great. That worked. Just one more thing, please.
>> Some of the replacements done by the rule before leave literals of the form ?(uintmax_t)34?.
>> I would like to use an unsigned constant in those cases. This is, something like ?34u?. 
>> So I add the following rule:
>> 
>> ```
>> @@ expression e1, e3; constant c2; @@
>> 30 
>> 31 - put_bytes(e1, (uintmax_t)c2, e3)
>> 32 + put_bytes(e1, c2, e3)
>> ```
>> 
>> That does the work, except adding the ?u?. How could I do that?
>> Is it possible adding something literally in the output?
>> Is it possible to somehow say in line 32 ?render c2 but with u suffix??
> 
> It is possible, but somewhat awkward.  Here is a simpler example that you 
> can adapt to your case:
> 
> @r@
> typedef uintmax_t;
> constant c;
> @@
> 
> (uintmax_t)c
> 
> @script:python s@
> c << r.c;
> cu;
> @@
> 
> coccinelle.cu = "%su" % (c)
> 
> @@
> constant r.c;
> identifier s.cu;
> @@
> 
> - (uintmax_t)c
> + cu
> 
> julia

[Cocci] Change function invocations argument

[Julia Lawall]

On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:

> Ok. Finally I have it working. There?s still a gotcha, though.
> The first version of my spatch contained these rules:
> 
> ```
> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
> 
> -   put_bytes(e1, (long_u)(e2), e3)
> +   put_bytes(e1, (uintmax_t)e2, e3)
> 
> @@ expression e1, e2, e3; @@
> 
> -   put_bytes(e1, (long_u)e2, e3)
> +   put_bytes(e1, (uintmax_t)e2, e3)
> 
> @@ expression e1, e2, e3; @@
> 
> -   put_bytes(e1, e2, (int)(e3))
> +   put_bytes(e1, e2, (unsigned int)e3)
> 
> @@ expression e1, e2, e3; @@
> 
> -   put_bytes(e1, e2, (int)e3)
> +   put_bytes(e1, e2, (unsigned int)e3)
> 
> ```
> 
> Those work ok.
> Then, I tried a second version, trying to compact those rules into a single, more expressive one:
> 
> ```
> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
> 
>     put_bytes(
>         e1,
> (
> -       (long_u)(e2)
> |
> -       (long_u)e2
> )\x10
> +       (uintmax_t)e2
>         ,
> (
> -       (int)(e3)
> |
> -       (int)e3
> )
> +       (unsigned int)e3
>     )
> 
> ```
> 
> That parses ok but just does nothing. So, it seems it doesn?t match anything, but I don?t understand why.
> Could you please explain?

I'm surprised that it parses OK.  Normally, you can't put + code on a 
disjunction.  You have to propagate it into each of the branches.  Try 
that.

julia


> 
> > On 20 Dec 2014, at 21:51, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > 
> > 
> > 
> > On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:
> > 
> >> Great. That worked. Just one more thing, please.
> >> Some of the replacements done by the rule before leave literals of the form ?(uintmax_t)34?.
> >> I would like to use an unsigned constant in those cases. This is, something like ?34u?. 
> >> So I add the following rule:
> >> 
> >> ```
> >> @@ expression e1, e3; constant c2; @@
> >> 30 
> >> 31 - put_bytes(e1, (uintmax_t)c2, e3)
> >> 32 + put_bytes(e1, c2, e3)
> >> ```
> >> 
> >> That does the work, except adding the ?u?. How could I do that?
> >> Is it possible adding something literally in the output?
> >> Is it possible to somehow say in line 32 ?render c2 but with u suffix??
> > 
> > It is possible, but somewhat awkward.  Here is a simpler example that you 
> > can adapt to your case:
> > 
> > @r@
> > typedef uintmax_t;
> > constant c;
> > @@
> > 
> > (uintmax_t)c
> > 
> > @script:python s@
> > c << r.c;
> > cu;
> > @@
> > 
> > coccinelle.cu = "%su" % (c)
> > 
> > @@
> > constant r.c;
> > identifier s.cu;
> > @@
> > 
> > - (uintmax_t)c
> > + cu
> > 
> > julia
> 
> 

[Cocci] Change function invocations argument

[Eliseo Martínez]

Sorry, forget about previous message. Everything?s ok.
As it happens sometimes, I had run over this a thousand times, but I didn?t see the pitfall until trying to explain to someone else.
The point is that, in fact, there are no (int)(e3) or (int)e3 occurrences in my code. I hadn?t realized that before.
First version worked right because each independent rule did it?s work on its own (rules about e3 doing nothing, but being applied after rules about e2 had already done their work).
Second version doesn?t work because it forces a match in both e2 and e3 variants.
This is, both approaches are not exactly equivalent, and second version doesn?t in fact match anything in my case.

So, I could just remove everything related to e3. But just for the sake of learning, I slightly modified the second version this way:

```
@@ typedef long_u, uintmax_t; expression e1, e2, e3; @@

    put_bytes(
        e1,
(
-       (long_u)(e2)
|
-       (long_u)e2
)
+       (uintmax_t)e2
        ,
(
-       (int)(e3)
+       (unsigned int)e3
|
-       (int)e3
+       (unsigned int)e3
|
        e3
)
    )

```

And now it works great.
Regarding what you say about moving plus lines within branches, I had indeed suspected that, and tried both versions, with identical results.
Last version of the patch above still has a plus outside branches when dealing with e2, and it?s working great? 
>From your comment, though, it seems that?s not expected usage. So, what do you advice? should I not count of that in the future, and always replicate plus lines within branches?

> On 21 Dec 2014, at 11:57, Julia Lawall <julia.lawall@lip6.fr> wrote:
> 
> 
> 
> On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:
> 
>> Ok. Finally I have it working. There?s still a gotcha, though.
>> The first version of my spatch contained these rules:
>> 
>> ```
>> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
>> 
>> -   put_bytes(e1, (long_u)(e2), e3)
>> +   put_bytes(e1, (uintmax_t)e2, e3)
>> 
>> @@ expression e1, e2, e3; @@
>> 
>> -   put_bytes(e1, (long_u)e2, e3)
>> +   put_bytes(e1, (uintmax_t)e2, e3)
>> 
>> @@ expression e1, e2, e3; @@
>> 
>> -   put_bytes(e1, e2, (int)(e3))
>> +   put_bytes(e1, e2, (unsigned int)e3)
>> 
>> @@ expression e1, e2, e3; @@
>> 
>> -   put_bytes(e1, e2, (int)e3)
>> +   put_bytes(e1, e2, (unsigned int)e3)
>> 
>> ```
>> 
>> Those work ok.
>> Then, I tried a second version, trying to compact those rules into a single, more expressive one:
>> 
>> ```
>> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
>> 
>>    put_bytes(
>>        e1,
>> (
>> -       (long_u)(e2)
>> |
>> -       (long_u)e2
>> )\x10
>> +       (uintmax_t)e2
>>        ,
>> (
>> -       (int)(e3)
>> |
>> -       (int)e3
>> )
>> +       (unsigned int)e3
>>    )
>> 
>> ```
>> 
>> That parses ok but just does nothing. So, it seems it doesn?t match anything, but I don?t understand why.
>> Could you please explain?
> 
> I'm surprised that it parses OK.  Normally, you can't put + code on a 
> disjunction.  You have to propagate it into each of the branches.  Try 
> that.
> 
> julia
> 
> 
>> 
>>> On 20 Dec 2014, at 21:51, Julia Lawall <julia.lawall@lip6.fr> wrote:
>>> 
>>> 
>>> 
>>> On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:
>>> 
>>>> Great. That worked. Just one more thing, please.
>>>> Some of the replacements done by the rule before leave literals of the form ?(uintmax_t)34?.
>>>> I would like to use an unsigned constant in those cases. This is, something like ?34u?. 
>>>> So I add the following rule:
>>>> 
>>>> ```
>>>> @@ expression e1, e3; constant c2; @@
>>>> 30 
>>>> 31 - put_bytes(e1, (uintmax_t)c2, e3)
>>>> 32 + put_bytes(e1, c2, e3)
>>>> ```
>>>> 
>>>> That does the work, except adding the ?u?. How could I do that?
>>>> Is it possible adding something literally in the output?
>>>> Is it possible to somehow say in line 32 ?render c2 but with u suffix??
>>> 
>>> It is possible, but somewhat awkward.  Here is a simpler example that you 
>>> can adapt to your case:
>>> 
>>> @r@
>>> typedef uintmax_t;
>>> constant c;
>>> @@
>>> 
>>> (uintmax_t)c
>>> 
>>> @script:python s@
>>> c << r.c;
>>> cu;
>>> @@
>>> 
>>> coccinelle.cu = "%su" % (c)
>>> 
>>> @@
>>> constant r.c;
>>> identifier s.cu;
>>> @@
>>> 
>>> - (uintmax_t)c
>>> + cu
>>> 
>>> julia
>> 

[Cocci] Change function invocations argument

[Julia Lawall]

On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:

> Sorry, forget about previous message. Everything?s ok.
> As it happens sometimes, I had run over this a thousand times, but I didn?t see the pitfall until trying to explain to someone else.
> The point is that, in fact, there are no (int)(e3) or (int)e3 occurrences in my code. I hadn?t realized that before.
> First version worked right because each independent rule did it?s work on its own (rules about e3 doing nothing, but being applied after rules about e2 had already done their work).
> Second version doesn?t work because it forces a match in both e2 and e3 variants.
> This is, both approaches are not exactly equivalent, and second version doesn?t in fact match anything in my case.
> 
> So, I could just remove everything related to e3. But just for the sake of learning, I slightly modified the second version this way:
> 
> ```
> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
> 
>     put_bytes(
>         e1,
> (
> -       (long_u)(e2)
> |
> -       (long_u)e2
> )
> +       (uintmax_t)e2
>         ,
> (
> -       (int)(e3)
> +       (unsigned int)e3
> |
> -       (int)e3
> +       (unsigned int)e3
> |
>         e3
> )
>     )
> 
> ```
> 
> And now it works great.
> Regarding what you say about moving plus lines within branches, I had indeed suspected that, and tried both versions, with identical results.
> Last version of the patch above still has a plus outside branches when dealing with e2, and it?s working great? 
> From your comment, though, it seems that?s not expected usage. So, what do you advice? should I not count of that in the future, and always replicate plus lines within branches?

Perhaps it work for expressions, but not for statements.

Anyway, it is safer to duplicate.

julia


> 
> > On 21 Dec 2014, at 11:57, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > 
> > 
> > 
> > On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:
> > 
> >> Ok. Finally I have it working. There?s still a gotcha, though.
> >> The first version of my spatch contained these rules:
> >> 
> >> ```
> >> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
> >> 
> >> -   put_bytes(e1, (long_u)(e2), e3)
> >> +   put_bytes(e1, (uintmax_t)e2, e3)
> >> 
> >> @@ expression e1, e2, e3; @@
> >> 
> >> -   put_bytes(e1, (long_u)e2, e3)
> >> +   put_bytes(e1, (uintmax_t)e2, e3)
> >> 
> >> @@ expression e1, e2, e3; @@
> >> 
> >> -   put_bytes(e1, e2, (int)(e3))
> >> +   put_bytes(e1, e2, (unsigned int)e3)
> >> 
> >> @@ expression e1, e2, e3; @@
> >> 
> >> -   put_bytes(e1, e2, (int)e3)
> >> +   put_bytes(e1, e2, (unsigned int)e3)
> >> 
> >> ```
> >> 
> >> Those work ok.
> >> Then, I tried a second version, trying to compact those rules into a single, more expressive one:
> >> 
> >> ```
> >> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
> >> 
> >>    put_bytes(
> >>        e1,
> >> (
> >> -       (long_u)(e2)
> >> |
> >> -       (long_u)e2
> >> )\x10
> >> +       (uintmax_t)e2
> >>        ,
> >> (
> >> -       (int)(e3)
> >> |
> >> -       (int)e3
> >> )
> >> +       (unsigned int)e3
> >>    )
> >> 
> >> ```
> >> 
> >> That parses ok but just does nothing. So, it seems it doesn?t match anything, but I don?t understand why.
> >> Could you please explain?
> > 
> > I'm surprised that it parses OK.  Normally, you can't put + code on a 
> > disjunction.  You have to propagate it into each of the branches.  Try 
> > that.
> > 
> > julia
> > 
> > 
> >> 
> >>> On 20 Dec 2014, at 21:51, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >>> 
> >>> 
> >>> 
> >>> On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:
> >>> 
> >>>> Great. That worked. Just one more thing, please.
> >>>> Some of the replacements done by the rule before leave literals of the form ?(uintmax_t)34?.
> >>>> I would like to use an unsigned constant in those cases. This is, something like ?34u?. 
> >>>> So I add the following rule:
> >>>> 
> >>>> ```
> >>>> @@ expression e1, e3; constant c2; @@
> >>>> 30 
> >>>> 31 - put_bytes(e1, (uintmax_t)c2, e3)
> >>>> 32 + put_bytes(e1, c2, e3)
> >>>> ```
> >>>> 
> >>>> That does the work, except adding the ?u?. How could I do that?
> >>>> Is it possible adding something literally in the output?
> >>>> Is it possible to somehow say in line 32 ?render c2 but with u suffix??
> >>> 
> >>> It is possible, but somewhat awkward.  Here is a simpler example that you 
> >>> can adapt to your case:
> >>> 
> >>> @r@
> >>> typedef uintmax_t;
> >>> constant c;
> >>> @@
> >>> 
> >>> (uintmax_t)c
> >>> 
> >>> @script:python s@
> >>> c << r.c;
> >>> cu;
> >>> @@
> >>> 
> >>> coccinelle.cu = "%su" % (c)
> >>> 
> >>> @@
> >>> constant r.c;
> >>> identifier s.cu;
> >>> @@
> >>> 
> >>> - (uintmax_t)c
> >>> + cu
> >>> 
> >>> julia
> >> 
> 
> 

[Cocci] Change function invocations argument

[Eliseo Martínez]

Ok.

Thanks very much.
Coccinelle is going to save me much manual work.
And what is more important: many massive code-quality improving refactorings, which probably wouldn?t be done if manually, will be done this way.
As I said at the beginning, excellent tool.
Congrats!

> On 21 Dec 2014, at 12:16, Julia Lawall <julia.lawall@lip6.fr> wrote:
> 
> 
> 
> On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:
> 
>> Sorry, forget about previous message. Everything?s ok.
>> As it happens sometimes, I had run over this a thousand times, but I didn?t see the pitfall until trying to explain to someone else.
>> The point is that, in fact, there are no (int)(e3) or (int)e3 occurrences in my code. I hadn?t realized that before.
>> First version worked right because each independent rule did it?s work on its own (rules about e3 doing nothing, but being applied after rules about e2 had already done their work).
>> Second version doesn?t work because it forces a match in both e2 and e3 variants.
>> This is, both approaches are not exactly equivalent, and second version doesn?t in fact match anything in my case.
>> 
>> So, I could just remove everything related to e3. But just for the sake of learning, I slightly modified the second version this way:
>> 
>> ```
>> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
>> 
>>    put_bytes(
>>        e1,
>> (
>> -       (long_u)(e2)
>> |
>> -       (long_u)e2
>> )
>> +       (uintmax_t)e2
>>        ,
>> (
>> -       (int)(e3)
>> +       (unsigned int)e3
>> |
>> -       (int)e3
>> +       (unsigned int)e3
>> |
>>        e3
>> )
>>    )
>> 
>> ```
>> 
>> And now it works great.
>> Regarding what you say about moving plus lines within branches, I had indeed suspected that, and tried both versions, with identical results.
>> Last version of the patch above still has a plus outside branches when dealing with e2, and it?s working great? 
>> From your comment, though, it seems that?s not expected usage. So, what do you advice? should I not count of that in the future, and always replicate plus lines within branches?
> 
> Perhaps it work for expressions, but not for statements.
> 
> Anyway, it is safer to duplicate.
> 
> julia
> 
> 
>> 
>>> On 21 Dec 2014, at 11:57, Julia Lawall <julia.lawall@lip6.fr> wrote:
>>> 
>>> 
>>> 
>>> On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:
>>> 
>>>> Ok. Finally I have it working. There?s still a gotcha, though.
>>>> The first version of my spatch contained these rules:
>>>> 
>>>> ```
>>>> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
>>>> 
>>>> -   put_bytes(e1, (long_u)(e2), e3)
>>>> +   put_bytes(e1, (uintmax_t)e2, e3)
>>>> 
>>>> @@ expression e1, e2, e3; @@
>>>> 
>>>> -   put_bytes(e1, (long_u)e2, e3)
>>>> +   put_bytes(e1, (uintmax_t)e2, e3)
>>>> 
>>>> @@ expression e1, e2, e3; @@
>>>> 
>>>> -   put_bytes(e1, e2, (int)(e3))
>>>> +   put_bytes(e1, e2, (unsigned int)e3)
>>>> 
>>>> @@ expression e1, e2, e3; @@
>>>> 
>>>> -   put_bytes(e1, e2, (int)e3)
>>>> +   put_bytes(e1, e2, (unsigned int)e3)
>>>> 
>>>> ```
>>>> 
>>>> Those work ok.
>>>> Then, I tried a second version, trying to compact those rules into a single, more expressive one:
>>>> 
>>>> ```
>>>> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
>>>> 
>>>>   put_bytes(
>>>>       e1,
>>>> (
>>>> -       (long_u)(e2)
>>>> |
>>>> -       (long_u)e2
>>>> )\x10
>>>> +       (uintmax_t)e2
>>>>       ,
>>>> (
>>>> -       (int)(e3)
>>>> |
>>>> -       (int)e3
>>>> )
>>>> +       (unsigned int)e3
>>>>   )
>>>> 
>>>> ```
>>>> 
>>>> That parses ok but just does nothing. So, it seems it doesn?t match anything, but I don?t understand why.
>>>> Could you please explain?
>>> 
>>> I'm surprised that it parses OK.  Normally, you can't put + code on a 
>>> disjunction.  You have to propagate it into each of the branches.  Try 
>>> that.
>>> 
>>> julia
>>> 
>>> 
>>>> 
>>>>> On 20 Dec 2014, at 21:51, Julia Lawall <julia.lawall@lip6.fr> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> On Sat, 20 Dec 2014, Eliseo Mart?nez wrote:
>>>>> 
>>>>>> Great. That worked. Just one more thing, please.
>>>>>> Some of the replacements done by the rule before leave literals of the form ?(uintmax_t)34?.
>>>>>> I would like to use an unsigned constant in those cases. This is, something like ?34u?. 
>>>>>> So I add the following rule:
>>>>>> 
>>>>>> ```
>>>>>> @@ expression e1, e3; constant c2; @@
>>>>>> 30 
>>>>>> 31 - put_bytes(e1, (uintmax_t)c2, e3)
>>>>>> 32 + put_bytes(e1, c2, e3)
>>>>>> ```
>>>>>> 
>>>>>> That does the work, except adding the ?u?. How could I do that?
>>>>>> Is it possible adding something literally in the output?
>>>>>> Is it possible to somehow say in line 32 ?render c2 but with u suffix??
>>>>> 
>>>>> It is possible, but somewhat awkward.  Here is a simpler example that you 
>>>>> can adapt to your case:
>>>>> 
>>>>> @r@
>>>>> typedef uintmax_t;
>>>>> constant c;
>>>>> @@
>>>>> 
>>>>> (uintmax_t)c
>>>>> 
>>>>> @script:python s@
>>>>> c << r.c;
>>>>> cu;
>>>>> @@
>>>>> 
>>>>> coccinelle.cu = "%su" % (c)
>>>>> 
>>>>> @@
>>>>> constant r.c;
>>>>> identifier s.cu;
>>>>> @@
>>>>> 
>>>>> - (uintmax_t)c
>>>>> + cu
>>>>> 
>>>>> julia

[Cocci] Change function invocations argument

[Julia Lawall]

On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:

> Ok.
> 
> Thanks very much.
> Coccinelle is going to save me much manual work.
> And what is more important: many massive code-quality improving refactorings, which probably wouldn?t be done if manually, will be done this way.
> As I said at the beginning, excellent tool.
> Congrats!

Thanks :)

julia

[Cocci] Change function invocations argument

[SF Markus Elfring]

> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
> 
>     put_bytes(
>         e1,
> (
> -       (long_u)(e2)
> |
> -       (long_u)e2
> )
> +       (uintmax_t)e2
>         ,
> (
> -       (int)(e3)
> +       (unsigned int)e3
> |
> -       (int)e3
> +       (unsigned int)e3
> |
>         e3
> )
>     )

Can it be that you need to fiddle with SmPL disjunctions a bit less here
because the metavariable type (or an isomorphism) will take care for
additional parentheses around the affected expressions?

How do you think about the applicability of the following approach?

@parameter_cast_replacement@
expression e1, e2, e3;
typedef long_u, uintmax_t;
@@
 put_bytes(e1,
-          (long_u)
+          (uintmax_t)
           e2,
(
-          (int)
+          (unsigned int)
           e3
|
           e3
)
          )

Regards,
Markus

[Cocci] Change function invocations argument

[Julia Lawall]

On Sun, 21 Dec 2014, SF Markus Elfring wrote:

> > @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
> > 
> >     put_bytes(
> >         e1,
> > (
> > -       (long_u)(e2)
> > |
> > -       (long_u)e2
> > )
> > +       (uintmax_t)e2
> >         ,
> > (
> > -       (int)(e3)
> > +       (unsigned int)e3
> > |
> > -       (int)e3
> > +       (unsigned int)e3
> > |
> >         e3
> > )
> >     )
> 
> Can it be that you need to fiddle with SmPL disjunctions a bit less here
> because the metavariable type (or an isomorphism) will take care for
> additional parentheses around the affected expressions?
> 
> How do you think about the applicability of the following approach?
> 
> @parameter_cast_replacement@
> expression e1, e2, e3;
> typedef long_u, uintmax_t;
> @@
>  put_bytes(e1,
> -          (long_u)
> +          (uintmax_t)
>            e2,
> (
> -          (int)
> +          (unsigned int)
>            e3
> |
>            e3
> )
>           )

It looks like Eliseo wants to get rid of the parentheses, which your 
approach does not do.  On the other hand, Markus is right that 

> > (
> > -       (long_u)(e2)
> > |
> > -       (long_u)e2
> > )

could be just

-       (long_u)(e2)

and an isomorphism will allow it to match the case where the parentheses 
are already removed as well.

julia


> 
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
> 

[Cocci] Change function invocations argument

[Eliseo Martínez]

That?s right. I wanted to remove external parentheses, which wasn?t done if employing only e2/e3.
I hadn?t realized using (e2)/(e3) would take care of both cases (with/without parentheses), though.

BTW, is there an easy way to remove **all** unneeded parentheses, project wide?

Thanks.
> On 21 Dec 2014, at 13:04, Julia Lawall <julia.lawall@lip6.fr> wrote:
> 
> 
> 
> On Sun, 21 Dec 2014, SF Markus Elfring wrote:
> 
>>> @@ typedef long_u, uintmax_t; expression e1, e2, e3; @@
>>> 
>>>    put_bytes(
>>>        e1,
>>> (
>>> -       (long_u)(e2)
>>> |
>>> -       (long_u)e2
>>> )
>>> +       (uintmax_t)e2
>>>        ,
>>> (
>>> -       (int)(e3)
>>> +       (unsigned int)e3
>>> |
>>> -       (int)e3
>>> +       (unsigned int)e3
>>> |
>>>        e3
>>> )
>>>    )
>> 
>> Can it be that you need to fiddle with SmPL disjunctions a bit less here
>> because the metavariable type (or an isomorphism) will take care for
>> additional parentheses around the affected expressions?
>> 
>> How do you think about the applicability of the following approach?
>> 
>> @parameter_cast_replacement@
>> expression e1, e2, e3;
>> typedef long_u, uintmax_t;
>> @@
>> put_bytes(e1,
>> -          (long_u)
>> +          (uintmax_t)
>>           e2,
>> (
>> -          (int)
>> +          (unsigned int)
>>           e3
>> |
>>           e3
>> )
>>          )
> 
> It looks like Eliseo wants to get rid of the parentheses, which your 
> approach does not do.  On the other hand, Markus is right that 
> 
>>> (
>>> -       (long_u)(e2)
>>> |
>>> -       (long_u)e2
>>> )
> 
> could be just
> 
> -       (long_u)(e2)
> 
> and an isomorphism will allow it to match the case where the parentheses 
> are already removed as well.
> 
> julia
> 
> 
>> 
>> Regards,
>> Markus
>> _______________________________________________
>> Cocci mailing list
>> Cocci at systeme.lip6.fr
>> https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Change function invocations argument

[SF Markus Elfring]

> BTW, is there an easy way to remove **all** unneeded parentheses, project wide?

It depends ...

How specific would you like to become in your wording / programming where those
parentheses are really unnecessary?

Regards,
Markus

[Cocci] Change function invocations argument

[Eliseo Martínez]

Sorry, don?t understand your question? 

> On 21 Dec 2014, at 13:49, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> 
>> BTW, is there an easy way to remove **all** unneeded parentheses, project wide?
> 
> It depends ...
> 
> How specific would you like to become in your wording / programming where those
> parentheses are really unnecessary?
> 
> Regards,
> Markus

[Cocci] Change function invocations argument

[SF Markus Elfring]

> Sorry, don?t understand your question?

I guess that you need to become more specific in your wording / programming
about the source code places where parentheses are really unnecessary.

How many efforts would you like to invest to avoid false positives and other
unwanted software changes?

Regards,
Markus

[Cocci] Change function invocations argument

[Eliseo Martínez]

I?m afraid I still don?t get you.
What I?m asking is:

Given that coccinelle understands c structure, and knows about common isomorphisms, is there an easy way so that I can instruct it to analyze every expression in my source code, and remove parentheses at every place they?re not needed? By not needed, I mean that the c expression meaning (taking into account operator precedence and the like) is the same with and without parentheses.

I?m working with a 200000 LOC codebase. So, if there?s no dependable way to do this (without false positives and the like), then I?d have to manually review every change. Feasibility of that depends on how many instances are found, which I cannot now in advance. If they are too many and I have to review them manually, then this is probably not worth the trouble.

Regards, 
Eliseo.

> On 21 Dec 2014, at 13:55, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> 
>> Sorry, don?t understand your question?
> 
> I guess that you need to become more specific in your wording / programming
> about the source code places where parentheses are really unnecessary.
> 
> How many efforts would you like to invest to avoid false positives and other
> unwanted software changes?
> 
> Regards,
> Markus

[Cocci] Source code clean-up for parentheses?

[SF Markus Elfring]

> Given that coccinelle understands c structure, and knows about common isomorphisms,
> is there an easy way so that I can instruct it to analyze every expression in
> my source code, and remove parentheses at every place they?re not needed?

There are various preconditions to consider.


> By not needed, I mean that the c expression meaning (taking into account operator
> precedence and the like) is the same with and without parentheses.

Software developers and source code reviewers can have strong opinions about the
placement for their parentheses.

Coding style specifications often recommend a code structure for reasonable
readability and not what compilation software can handle in principle.


> I?m working with a 200000 LOC codebase. So, if there?s no dependable way to do this
> (without false positives and the like), then I?d have to manually review every change.

Which error rate can you tolerate for your source code analysis and
eventually automatic transformations?

Do you get further ideas from the section "4.2.1 How to remove useless parentheses?"
in Coccinelle's manual?


> Feasibility of that depends on how many instances are found, which I cannot now
> in advance. If they are too many and I have to review them manually,
> then this is probably not worth the trouble.

Do efforts naturally evolve with the desire to come closer to a kind of perfection?

Regards,
Markus

[Cocci] Source code clean-up for parentheses?

[SF Markus Elfring]

> Do you get further ideas from the section "4.2.1 How to remove useless parentheses?"
> in Coccinelle's manual?

Dear Julia,

Would you like to fix another dangling link in the published documentation?
http://coccinelle.lip6.fr/docs/main_grammar015.html#sec23

Regards,
Markus

[Cocci] Change function invocations argument

[Julia Lawall]

On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:

> That?s right. I wanted to remove external parentheses, which wasn?t done if employing only e2/e3.
> I hadn?t realized using (e2)/(e3) would take care of both cases (with/without parentheses), though.
> 
> BTW, is there an easy way to remove **all** unneeded parentheses, project wide?

Probably not.  You could remove all parentheses, but that might risk 
removing some needed ones.  On the other hand, Coccinelle does know 
something about precedednce, so you could try:

-(
 e1
-)

and see what happens.  Probably it will do more than you expect, though.

julia

[Cocci] Change function invocations argument

[Julia Lawall]

On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:

> I?m afraid I still don?t get you.
> What I?m asking is:
> 
> Given that coccinelle understands c structure, and knows about common isomorphisms, is there an easy way so that I can instruct it to analyze every expression in my source code, and remove parentheses at every place they?re not needed? By not needed, I mean that the c expression meaning (taking into account operator precedence and the like) is the same with and without parentheses.
> 
> I?m working with a 200000 LOC codebase. So, if there?s no dependable way to do this (without false positives and the like), then I?d have to manually review every change. Feasibility of that depends on how many instances are found, which I cannot now in advance. If they are too many and I have to review them manually, then this is probably not worth the trouble.

Given the size of the code, I think it would be reasonable to consider 
safe cases as they arise.  For example,

@@
identifier x;
@@

-(
x
-)

No idea if that is useful.

The previous suggestion with x as an expression removes too much.

I thought that the following might work, ie add parentheses:

@@
@@

- x
+ 3 + 4

But testing it on:

int main () {
  return (x * (5 + 7));
}

shows that it does not.

julia

[Cocci] Change function invocations argument

[Michael Stefaniuc]

On 12/21/2014 03:01 PM, Julia Lawall wrote:
> 
> 
> On Sun, 21 Dec 2014, Eliseo Mart?nez wrote:
> 
>> That?s right. I wanted to remove external parentheses, which
>> wasn?t done if employing only e2/e3. I hadn?t realized using
>> (e2)/(e3) would take care of both cases (with/without
>> parentheses), though.
>> 
>> BTW, is there an easy way to remove **all** unneeded parentheses,
>> project wide?
> 
> Probably not.  You could remove all parentheses, but that might
> risk removing some needed ones.  On the other hand, Coccinelle does
> know something about precedednce, so you could try:
> 
> -( e1 -)
> 
> and see what happens.  Probably it will do more than you expect,
> though.
Though it is easy to validate that the generated patch is truly a
no-op. I use a sha1sum on the generated object files without and with
the patch and compare those. Of course there is the danger that the
code touched was ifdef'ed out so some developer discretion is still
needed.

bye
	michael