* [Cocci] Change function invocations argument
@ 2014-12-20 20:21 Eliseo Martínez
2014-12-20 20:32 ` Julia Lawall
0 siblings, 1 reply; 22+ messages in thread
From: Eliseo Martínez @ 2014-12-20 20:21 UTC (permalink / raw)
To: cocci
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.
^ permalink raw reply [flat|nested] 22+ messages in thread* [Cocci] Change function invocations argument 2014-12-20 20:21 [Cocci] Change function invocations argument Eliseo Martínez @ 2014-12-20 20:32 ` Julia Lawall 2014-12-20 20:45 ` Eliseo Martínez 0 siblings, 1 reply; 22+ messages in thread From: Julia Lawall @ 2014-12-20 20:32 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-20 20:32 ` Julia Lawall @ 2014-12-20 20:45 ` Eliseo Martínez 2014-12-20 20:51 ` Julia Lawall 0 siblings, 1 reply; 22+ messages in thread From: Eliseo Martínez @ 2014-12-20 20:45 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-20 20:45 ` Eliseo Martínez @ 2014-12-20 20:51 ` Julia Lawall 2014-12-21 10:43 ` Eliseo Martínez 0 siblings, 1 reply; 22+ messages in thread From: Julia Lawall @ 2014-12-20 20:51 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-20 20:51 ` Julia Lawall @ 2014-12-21 10:43 ` Eliseo Martínez 2014-12-21 10:57 ` Julia Lawall 0 siblings, 1 reply; 22+ messages in thread From: Eliseo Martínez @ 2014-12-21 10:43 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 10:43 ` Eliseo Martínez @ 2014-12-21 10:57 ` Julia Lawall 2014-12-21 11:14 ` Eliseo Martínez 0 siblings, 1 reply; 22+ messages in thread From: Julia Lawall @ 2014-12-21 10:57 UTC (permalink / raw) To: cocci 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 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 10:57 ` Julia Lawall @ 2014-12-21 11:14 ` Eliseo Martínez 2014-12-21 11:16 ` Julia Lawall 2014-12-21 11:55 ` SF Markus Elfring 0 siblings, 2 replies; 22+ messages in thread From: Eliseo Martínez @ 2014-12-21 11:14 UTC (permalink / raw) To: cocci 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 >> ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 11:14 ` Eliseo Martínez @ 2014-12-21 11:16 ` Julia Lawall 2014-12-21 11:19 ` Eliseo Martínez 2014-12-21 11:55 ` SF Markus Elfring 1 sibling, 1 reply; 22+ messages in thread From: Julia Lawall @ 2014-12-21 11:16 UTC (permalink / raw) To: cocci 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 > >> > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 11:16 ` Julia Lawall @ 2014-12-21 11:19 ` Eliseo Martínez 2014-12-21 11:20 ` Julia Lawall 0 siblings, 1 reply; 22+ messages in thread From: Eliseo Martínez @ 2014-12-21 11:19 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 11:19 ` Eliseo Martínez @ 2014-12-21 11:20 ` Julia Lawall 0 siblings, 0 replies; 22+ messages in thread From: Julia Lawall @ 2014-12-21 11:20 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 11:14 ` Eliseo Martínez 2014-12-21 11:16 ` Julia Lawall @ 2014-12-21 11:55 ` SF Markus Elfring 2014-12-21 12:04 ` Julia Lawall 1 sibling, 1 reply; 22+ messages in thread From: SF Markus Elfring @ 2014-12-21 11:55 UTC (permalink / raw) To: cocci > @@ 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 11:55 ` SF Markus Elfring @ 2014-12-21 12:04 ` Julia Lawall 2014-12-21 12:24 ` Eliseo Martínez 0 siblings, 1 reply; 22+ messages in thread From: Julia Lawall @ 2014-12-21 12:04 UTC (permalink / raw) To: cocci 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 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 12:04 ` Julia Lawall @ 2014-12-21 12:24 ` Eliseo Martínez 2014-12-21 12:49 ` SF Markus Elfring 2014-12-21 14:01 ` Julia Lawall 0 siblings, 2 replies; 22+ messages in thread From: Eliseo Martínez @ 2014-12-21 12:24 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 12:24 ` Eliseo Martínez @ 2014-12-21 12:49 ` SF Markus Elfring 2014-12-21 12:50 ` Eliseo Martínez 2014-12-21 14:01 ` Julia Lawall 1 sibling, 1 reply; 22+ messages in thread From: SF Markus Elfring @ 2014-12-21 12:49 UTC (permalink / raw) To: cocci > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 12:49 ` SF Markus Elfring @ 2014-12-21 12:50 ` Eliseo Martínez 2014-12-21 12:55 ` SF Markus Elfring 0 siblings, 1 reply; 22+ messages in thread From: Eliseo Martínez @ 2014-12-21 12:50 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 12:50 ` Eliseo Martínez @ 2014-12-21 12:55 ` SF Markus Elfring 2014-12-21 13:12 ` Eliseo Martínez 0 siblings, 1 reply; 22+ messages in thread From: SF Markus Elfring @ 2014-12-21 12:55 UTC (permalink / raw) To: cocci > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 12:55 ` SF Markus Elfring @ 2014-12-21 13:12 ` Eliseo Martínez 2014-12-21 13:44 ` [Cocci] Source code clean-up for parentheses? SF Markus Elfring 2014-12-21 14:13 ` [Cocci] Change function invocations argument Julia Lawall 0 siblings, 2 replies; 22+ messages in thread From: Eliseo Martínez @ 2014-12-21 13:12 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Source code clean-up for parentheses? 2014-12-21 13:12 ` Eliseo Martínez @ 2014-12-21 13:44 ` SF Markus Elfring 2014-12-21 13:48 ` SF Markus Elfring 2014-12-21 14:13 ` [Cocci] Change function invocations argument Julia Lawall 1 sibling, 1 reply; 22+ messages in thread From: SF Markus Elfring @ 2014-12-21 13:44 UTC (permalink / raw) To: cocci > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Source code clean-up for parentheses? 2014-12-21 13:44 ` [Cocci] Source code clean-up for parentheses? SF Markus Elfring @ 2014-12-21 13:48 ` SF Markus Elfring 0 siblings, 0 replies; 22+ messages in thread From: SF Markus Elfring @ 2014-12-21 13:48 UTC (permalink / raw) To: cocci > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 13:12 ` Eliseo Martínez 2014-12-21 13:44 ` [Cocci] Source code clean-up for parentheses? SF Markus Elfring @ 2014-12-21 14:13 ` Julia Lawall 1 sibling, 0 replies; 22+ messages in thread From: Julia Lawall @ 2014-12-21 14:13 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 12:24 ` Eliseo Martínez 2014-12-21 12:49 ` SF Markus Elfring @ 2014-12-21 14:01 ` Julia Lawall 2014-12-22 13:33 ` Michael Stefaniuc 1 sibling, 1 reply; 22+ messages in thread From: Julia Lawall @ 2014-12-21 14:01 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Cocci] Change function invocations argument 2014-12-21 14:01 ` Julia Lawall @ 2014-12-22 13:33 ` Michael Stefaniuc 0 siblings, 0 replies; 22+ messages in thread From: Michael Stefaniuc @ 2014-12-22 13:33 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-12-22 13:33 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-20 20:21 [Cocci] Change function invocations argument Eliseo Martínez 2014-12-20 20:32 ` Julia Lawall 2014-12-20 20:45 ` Eliseo Martínez 2014-12-20 20:51 ` Julia Lawall 2014-12-21 10:43 ` Eliseo Martínez 2014-12-21 10:57 ` Julia Lawall 2014-12-21 11:14 ` Eliseo Martínez 2014-12-21 11:16 ` Julia Lawall 2014-12-21 11:19 ` Eliseo Martínez 2014-12-21 11:20 ` Julia Lawall 2014-12-21 11:55 ` SF Markus Elfring 2014-12-21 12:04 ` Julia Lawall 2014-12-21 12:24 ` Eliseo Martínez 2014-12-21 12:49 ` SF Markus Elfring 2014-12-21 12:50 ` Eliseo Martínez 2014-12-21 12:55 ` SF Markus Elfring 2014-12-21 13:12 ` Eliseo Martínez 2014-12-21 13:44 ` [Cocci] Source code clean-up for parentheses? SF Markus Elfring 2014-12-21 13:48 ` SF Markus Elfring 2014-12-21 14:13 ` [Cocci] Change function invocations argument Julia Lawall 2014-12-21 14:01 ` Julia Lawall 2014-12-22 13:33 ` Michael Stefaniuc
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.