Re: [PATCH 5/5] graph: use SWAP macro

From: René Scharfe <l.s.r@web.de>
Date: Jan 30, 2017, 17:41 UTC
Message-ID: <63f1dcc0-729c-955b-63fa-859e441406bd@web.de>
In-Reply-To: <alpine.DEB.2.20.1701301714450.3469@virtualbox>

Am 30.01.2017 um 17:16 schrieb Johannes Schindelin:

Show 15 quoted lines

> Hi René,
>
> On Sat, 28 Jan 2017, René Scharfe wrote:
>
>> Exchange the values of graph->columns and graph->new_columns using the
>> macro SWAP instead of hand-rolled code.  The result is shorter and
>> easier to read.
>>
>> This transformation was not done by the semantic patch swap.cocci
>> because there's an unrelated statement between the second and the last
>> step of the exchange, so it didn't match the expected pattern.
>
> Is it really true that Coccinelle cannot be told to look for a code block
> that declares a variable that is then used *only* in the lines we want to
> match and replace?

Hope I parsed your question correctly; my answer would be that it can't be true because that's basically what the proposed semantic patch does:

	@ swap @
	type T;
	T tmp, a, b;
	@@
	- tmp = a;
	- a = b;
	- b = tmp;
	+ SWAP(a, b);

	@ extends swap @
	identifier unused;
	@@
	  {
	  ...
	- T unused;
	  ... when != unused
	  }

The first part (up to the "+") looks for a opportunities to use SWAP, and the second part looks for blocks where that transformation was done and we declare identifiers that are/became unused.

It did not match the code in graph.c because the pattern was basically:

	tmp = a;
	a = b;
	something = totally_different;
	b = tmp;

Coccinelle can be told to ignore such unrelated code by adding "... when != tmp" etc. (which matches context lines that don't reference tmp), but that's slooow. (Perhaps I just did it wrong, though.)

René