Coccinelle Challenge Problem 2

Coccinelle Challenge Problem 2

[Gargi Sharma]

[-- Attachment #1: Type: text/plain, Size: 343 bytes --]

Hi,

Here's my coccinelle script to parentheses around the right hand side of an
assignment.

@@ expression e, e1, e2; @@
e =
-(
        e1 + e2
-)

@@ expression e, e1, e2; @@
e =
-(
        e1 - e2
-)

@@ expression e, e1;
contant c; @@
e =
-(
        e1 << c
-)

@@ expression e, e1;
constant c; @@
e =
-(
        e1 >> c
-)

thanks,
gargi

[-- Attachment #2: Type: text/html, Size: 574 bytes --]

Re: [Outreachy kernel] Coccinelle Challenge Problem 2

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1630 bytes --]



On Tue, 21 Feb 2017, Gargi Sharma wrote:

> Hi,
>
> Here's my coccinelle script to parentheses around the right hand side of an
> assignment.
>
> @@ expression e, e1, e2; @@  
> e =  
> -(  
>         e1 + e2
> -)  
>  
> @@ expression e, e1, e2; @@  
> e =  
> -(  
>         e1 - e2  
> -)
>  
> @@ expression e, e1;
> contant c; @@  
> e =  
> -(  
>         e1 << c  
> -)
>  
> @@ expression e, e1;
> constant c; @@  
> e =  
> -(  
>         e1 >> c  
> -)

It looks reasonable.  I don't think you need to restrict the right side of
>> and << to constants, though.

Also, the whole thing can actually be much shorter, because there is a
"binary operator" metavariable type.  So you could say:

@@
binary operator bop = {+,-,>>,<<};
expression e, e1, e2;
@@

e =
- (
  e1 bop e2
- )

Although, your version is probably more readable, because "bop" doesn't
really look like a binary operator.

Did you try, eg

e =
- (e1 + e2)
+ e1 + e2

?  This is not a good solution, but it could be good to think about why.

julia

> thanks,
> gargi
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CAOCi2DHM%2BX0ksQZ2ZBXPk
> bJu5uv5kerHRpaJaFEG8C36CA72jA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

Re: [Outreachy kernel] Coccinelle Challenge Problem 2

[Gargi Sharma]

[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]

On Tue, Feb 21, 2017 at 11:24 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
>
> On Tue, 21 Feb 2017, Gargi Sharma wrote:
>
> > Hi,
> >
> > Here's my coccinelle script to parentheses around the right hand side
of an
> > assignment.
> >
> > @@ expression e, e1, e2; @@
> > e =
> > -(
> >         e1 + e2
> > -)
> >
> > @@ expression e, e1, e2; @@
> > e =
> > -(
> >         e1 - e2
> > -)
> >
> > @@ expression e, e1;
> > contant c; @@
> > e =
> > -(
> >         e1 << c
> > -)
> >
> > @@ expression e, e1;
> > constant c; @@
> > e =
> > -(
> >         e1 >> c
> > -)
>
> It looks reasonable.  I don't think you need to restrict the right side of
> >> and << to constants, though.
>
> Also, the whole thing can actually be much shorter, because there is a
> "binary operator" metavariable type.  So you could say:
>


> @@
> binary operator bop = {+,-,>>,<<};
> expression e, e1, e2;
> @@

Awesome! This makes the patch so much more concise.

>
> e =
> - (
>   e1 bop e2
> - )
>
> Although, your version is probably more readable, because "bop" doesn't
> really look like a binary operator.
>
> Did you try, eg
>
> e =
> - (e1 + e2)
> + e1 + e2
>
> ?  This is not a good solution, but it could be good to think about why.

I tried this after reading this mail :). I think it's not a good solution
because
of the false positives that it will give. Also for the cases where there's
no
space around '+' will be replaced as well which is not the intended use case
for the patch. And all the cases where we have shifted the operands to avoid
the 80 character limit, will all be moved on the same line.

gargi
>
> julia
>
> > thanks,
> > gargi
> >
> > --
> > You received this message because you are subscribed to the Google
Groups
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
an
> > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visithttps://groups.google.com
/d/msgid/outreachy-kernel/CAOCi2DHM%2BX0ksQZ2ZBXPk
> > bJu5uv5kerHRpaJaFEG8C36CA72jA%40mail.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
> >
>
> --
> You received this message because you are subscribed to the Google Groups
"outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/ms
gid/outreachy-kernel/alpine.DEB.2.20.1702211850270.3448%40hadrien.
> For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: text/html, Size: 4533 bytes --]

Re: [Outreachy kernel] Coccinelle Challenge Problem 2

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]

> > Did you try, eg
> >
> > e =
> > - (e1 + e2)
> > + e1 + e2
> >
> > ?  This is not a good solution, but it could be good to think about why.
>
> I tried this after reading this mail :). I think it's not a good solution
> becauseof the false positives that it will give. Also for the cases where
> there's no
> space around '+' will be replaced as well which is not the intended use case
> for the patch. And all the cases where we have shifted the operands to avoid
> the 80 character limit, will all be moved on the same line.

Yes, it's all pretty sad :).  One conclusion is that it is better not to
remove code and then add it back.  When you add it back, Coccinelle takes
over the pretty printing, and Coccinelle is not super smart about pretty
printing binary operators.  It does try to do a good job for function
arguments, though.

But you probably noticed that it was changing things that didn't have
parentheses at all, which may be what you mean by false positives.  This is
because of the idea of an isomorphism.  Coccinelle considers that sometimes
people put parentheses and sometimes they don't.  And it is rather tiresome
to have to specify all of the possibilities by hand.  So if you put an
expression with parentheses that is either all annotated with - or that is
all unannotated, then it will consider the possibility that the parentheses
are not there as well.  You can see all of the patterns it is really trying
with spatch --parse-cocci foo.cocci.  But the way you wrote the rules, the
- was only on the (), so it considers that the parentheses are important,
so there is no problem.

julia