public inbox for cocci@systeme.lip6.fr
 help / color / mirror / Atom feed
From: julia.lawall@lip6.fr (Julia Lawall)
To: cocci@systeme.lip6.fr
Subject: [Cocci] Checking patches for questionable comma expressions in if conditions
Date: Wed, 12 Sep 2018 23:20:32 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1809122318240.2437@hadrien> (raw)
In-Reply-To: <d16d096c-2598-32c3-e489-d7f00b2aec51@users.sourceforge.net>



On Wed, 12 Sep 2018, SF Markus Elfring wrote:

> > The below spatch works for me - and finds the cases I was looking
> > for in report mode.
>
> This is nice.
>
>
> > In patch mode it fixes some in a bad way though due to some additional "bugs"
> > in the if statement like:
> ?
> > -       if ((notify->event = event), event->refs) {
> > +       (notify->event = event);
> > +       if (event->refs) {
>
> I am curious on how software development considerations will evolve further
> for such generated patches.
>
> Will the shown script for the semantic patch language need any more fine-tuning?
>
> Would the following transformation variant result in desirable differences
> (after the specification of extra parentheses)?
>
>
> @badif@
> position P;
> statement S;
> expression E1,E2;
> @@
>  if at P ((E1),E2) S
>
> ?
>
> @fixbadif depends on patch && badif@
> position badif.P;
> statement S;
> expression badif.E1,badif.E2;
> @@
> +E1;
>  if at P (
> -      (E1),
>        E2)
>       S

Alternatively, I suspect that one could just do

- (
  E
- )
  ;

Or the original rule could be

+E1;
 if (
- (E1),
  E2) S1 else S2

>
>
> > -       if (mask = 0, data = 0, ram->diff.rammap_11_0a_03fe) {
> > +       mask = 0, data = 0;
> > +       if (ram->diff.rammap_11_0a_03fe) {
> >
> > In futher cases it is not clear if the unconditional part really was
> > intended to take effect outside the conditional code so it is not
> > clear if the placement before the if () is technically correct

I'm not sure to understand the problem here.  You want to also change the
, on the added line to a semicolon?

julia

      reply	other threads:[~2018-09-12 21:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-12 12:33 [Cocci] checking and fixing comma operator in if condition Nicholas Mc Guire
2018-09-12 18:33 ` [Cocci] Checking patches for questionable comma expressions in if conditions SF Markus Elfring
2018-09-12 21:20   ` Julia Lawall [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1809122318240.2437@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=cocci@systeme.lip6.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox