[Cocci] convert if (x) stmt to if (x) {stmt}

[Cocci] convert if (x) stmt to if (x) {stmt}

[ron minnich]

we've found another one of these
if (x)
y
z

things in firmware that were intended to be
if (x) {
y
z
}

we're kind of tired of this and want to blanket require {} for all ifs,
even one liners.

We want to convert the entire code base such that all if (E) S becomes if
(E) {S} but of course we don't want to add extra {}.

I don't totally trust my rusty spatch-foo to get this right and was
wondering if there's already such a thing out there.

ron
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20180516/8e8c83ca/attachment.html>

[Cocci] convert if (x) stmt to if (x) {stmt}

[Julia Lawall]

On Wed, 16 May 2018, ron minnich wrote:

> we've found another one of theseif (x)
> y
> z
>
> things in firmware that were intended to be
> if (x) {
> y
> z
> }
>
> we're kind of tired of this and want to blanket require {} for all ifs, even
> one liners.
>
> We want to convert the entire code base such that all if (E) S becomes if
> (E) {S} but of course we don't want to add extra {}.?
>
> I don't totally trust my rusty spatch-foo to get this right and was
> wondering if there's already such a thing out there.

No, I don't know of such a thing.  However
linux/scripts/coccinelle/ifcol.cocci checks for an if header followed by
two statements preceded by the same number of whitespace characters.

J'ai fait un essaie pour ce que tu demande.  Pour l'instant, ca tourne...

julia


>
> ron
>
>

[Cocci] convert if (x) stmt to if (x) {stmt}

[Julia Lawall]

On Wed, 16 May 2018, Julia Lawall wrote:

>
>
> On Wed, 16 May 2018, ron minnich wrote:
>
> > we've found another one of theseif (x)
> > y
> > z
> >
> > things in firmware that were intended to be
> > if (x) {
> > y
> > z
> > }
> >
> > we're kind of tired of this and want to blanket require {} for all ifs, even
> > one liners.
> >
> > We want to convert the entire code base such that all if (E) S becomes if
> > (E) {S} but of course we don't want to add extra {}.?
> >
> > I don't totally trust my rusty spatch-foo to get this right and was
> > wondering if there's already such a thing out there.
>
> No, I don't know of such a thing.  However
> linux/scripts/coccinelle/ifcol.cocci checks for an if header followed by
> two statements preceded by the same number of whitespace characters.
>
> J'ai fait un essaie pour ce que tu demande.  Pour l'instant, ca tourne...

Sorry, I don't know where the answer in French came from...

Anyway, here is the rule I made:

@r disable braces1,braces2,braces3,braces4,neg_if @
position p;
statement S;
@@

if at p (...) { ... } else S

@disable braces1,braces2,braces3,braces4,neg_if @
position p != r.p;
statement S,S1;
@@

if at p (...)
+ {
  S1
+ }
  else S

@s disable braces1,braces2,braces3,braces4,neg_if @
position p;
statement S;
@@

if at p (...) S else { ... }

@disable braces1,braces2,braces3,braces4,neg_if @
position p != s.p;
statement S,S1;
@@

if at p (...) S else
+ {
  S1
+ }

I would suggest to proceed carefully...

julia

[Cocci] convert if (x) stmt to if (x) {stmt}

[Julia Lawall]

On Wed, 16 May 2018, Julia Lawall wrote:

>
>
> On Wed, 16 May 2018, Julia Lawall wrote:
>
> >
> >
> > On Wed, 16 May 2018, ron minnich wrote:
> >
> > > we've found another one of theseif (x)
> > > y
> > > z
> > >
> > > things in firmware that were intended to be
> > > if (x) {
> > > y
> > > z
> > > }
> > >
> > > we're kind of tired of this and want to blanket require {} for all ifs, even
> > > one liners.
> > >
> > > We want to convert the entire code base such that all if (E) S becomes if
> > > (E) {S} but of course we don't want to add extra {}.?
> > >
> > > I don't totally trust my rusty spatch-foo to get this right and was
> > > wondering if there's already such a thing out there.
> >
> > No, I don't know of such a thing.  However
> > linux/scripts/coccinelle/ifcol.cocci checks for an if header followed by
> > two statements preceded by the same number of whitespace characters.
> >
> > J'ai fait un essaie pour ce que tu demande.  Pour l'instant, ca tourne...
>
> Sorry, I don't know where the answer in French came from...
>
> Anyway, here is the rule I made:
>
> @r disable braces1,braces2,braces3,braces4,neg_if @
> position p;
> statement S;
> @@
>
> if at p (...) { ... } else S
>
> @disable braces1,braces2,braces3,braces4,neg_if @
> position p != r.p;
> statement S,S1;
> @@
>
> if at p (...)
> + {
>   S1
> + }
>   else S
>
> @s disable braces1,braces2,braces3,braces4,neg_if @
> position p;
> statement S;
> @@
>
> if at p (...) S else { ... }
>
> @disable braces1,braces2,braces3,braces4,neg_if @
> position p != s.p;
> statement S,S1;
> @@
>
> if at p (...) S else
> + {
>   S1
> + }
>
> I would suggest to proceed carefully...

In each rule that contains S1, you first could replace the S1 by e; where
e is an expression metavariable.  That would get a lot of simple cases out
of the way.  Then you could commit that, and then run the original rule,
thus being able to focus on the more complex results.

julia

[Cocci] convert if (x) stmt to if (x) {stmt}

[Julia Lawall]

On Thu, 17 May 2018, SF Markus Elfring wrote:

> >> I would suggest to proceed carefully...
> >
> > In each rule that contains S1, you first could replace the S1 by e;
> > where e is an expression metavariable.
>
> Please reconsider this suggestion.
>
> * I guess that there can statements occur which are not expressions.
>
> * Should return values trigger any more software development concerns?

I don't think you understood the idea at all.  The rule will likely make
hundreds or throusands of changes.  There is little risk in the case of
e;.  Getting all of those cases out of the way first will reduce the size
of the result that has to be checked carefully.

julia



>
>
> > Then you could commit that, and then run the original rule,
> > thus being able to focus on the more complex results.
>
> The application of additional SmPL script variants can help.
> I am curious on how the fine-tuning of corresponding implementation details
> will evolve further.
>
> Regards,
> Markus
>

[Cocci] convert if (x) stmt to if (x) {stmt}

[Julia Lawall]

On Fri, 18 May 2018, SF Markus Elfring wrote:

> >> I understood one suggestion in the way for a SmPL specification
> >> like the following.
> >>
> >> if (...)
> >> +{
> >>  S;
> >> +e;
> >> +}
> >
> > The rule has no goal of adding some unknown e;.
>
> How does this feedback fit to your wording ?In each rule that contains S1,
> you first could replace the S1 by e; ???

I didn't mean to write a semantic patch to remove S1 and add e;.  I meant
to put e; instead of S1 in the rule, producing the rule below.

julia

>
>
> > if (...)
> > + {
> >   e;
> > + }
> >   else S
> >
> > I don't see why one should consider whether e; should have been written in
> > a different way in making this transformation.
>
> It seems that you present another variant for a SmPL script.
>
> I assume that there are further constraints to consider for the discussed
> change pattern.
>
> Regards,
> Markus
>