[Cocci] how to add a few decalrations

[Cocci] how to add a few decalrations

[picca at synchrotron-soleil.fr]

Hello,

first a big thanks for this fantastic tool... I am not kidding and this is not 'pour ?tre bien vu'.

So here I am trying to do something like that

@@
identifier self, unit_type, error, f;
@@

f(...){
...
+ unsigned int n_values = darray_size(hkl_geometry_axes_names_get(self));
+ double values[n_values];
...
-hkl_geometry_set_values_v(self, unit_type, &error, ...)
+hkl_geometry_axes_values_set(self, values, n_values, unit_type, &error)
...
}


So the idea is to replace a method by another one but to be able to do that, I need to add two new variables n_values and values.

Indeed when I try this spatch, I got this kind of error

picca at ORD03037:~/Projets/hkl/scripts$ ./refactoring geometry.cocci ../tests/*.c
init_defs_builtins: /usr/share/coccinelle/standard.h
190 193
Fatal error: exception Failure("minus: parse error: 
 = File "geometry.cocci", line 11, column 0,  charpos = 190
    around = '...', whole content = ...
")
../tests/hkl3d-test-t.c, ../tests/hkl-axis-t.c


I do not understand what's going on, but I am ready to learn with your help.

thanks a lot

Fr?d?ric

[Cocci] How to add a few declarations?

[SF Markus Elfring]

> So the idea is to replace a method by another one but to be able to do that, I need to add two new variables n_values and values.

Have you got any expectations on the placement for the added variables?
Do any other scopes (or separate statement blocks) matter here?

Regards,
Markus

[Cocci] How to add a few declarations?

[picca at synchrotron-soleil.fr]

On Thu, Sep 11, 2014 at 03:04:07PM +0200, SF Markus Elfring wrote:
> > So the idea is to replace a method by another one but to be able to do that, I need to add two new variables n_values and values.
> 
> Have you got any expectations on the placement for the added variables?

I imagine at the begining of the scope where the ..._set_v method is located.

> Do any other scopes (or separate statement blocks) matter here?

what do you mean exactly by other scopes matter ?

REgards

Frederic

[Cocci] How to add a few declarations?

[Julia Lawall]

On Thu, 11 Sep 2014, picca at synchrotron-soleil.fr wrote:

> On Thu, Sep 11, 2014 at 03:04:07PM +0200, SF Markus Elfring wrote:
> > > So the idea is to replace a method by another one but to be able to do that, I need to add two new variables n_values and values.
> >
> > Have you got any expectations on the placement for the added variables?
>
> I imagine at the begining of the scope where the ..._set_v method is located.

My solution was to put them all at the beginning of the function.

The innermost enclosing scope would be a bit hard to specify.

If you want to just add a new scope around the replaced call, that would
be easiest, and would not require the fresh identifier names.

julia

[Cocci] how to add a few decalrations

[Julia Lawall]

On Thu, 11 Sep 2014, picca at synchrotron-soleil.fr wrote:

> Hello,
>
> first a big thanks for this fantastic tool... I am not kidding and this is not 'pour ?tre bien vu'.
>
> So here I am trying to do something like that
>
> @@
> identifier self, unit_type, error, f;
> @@
>
> f(...){
> ...
> + unsigned int n_values = darray_size(hkl_geometry_axes_names_get(self));
> + double values[n_values];

The problem is here.  You have some + code that is between two ...s.  So
Coccinelle does not know where you want to add it.  If you want to add
these variables at the beginning of the list of variable declarations,
then just drop the first ...  If you want to add it at the end of the list
of variable declarations, then it is more complicated.  Basically, you
would want to add it just before the fist statement that is not a
declaration.  This could be matched like this:

f(...){
  ... when != S
+ stuff to add
  S1
  ...
}

But S1 could coincide with the position of hkl_geometry_set_values_v.  And
also the rule above will modify every function, not necessarily the ones
with hkl_geometry_set_values_v.

Another issue is that there might be more than one call to
hkl_geometry_set_values_v in the function, and so you might want more than
one n_values variable.

In your subsequent email I see that you are OK with putting the new
declarations at the beginning of the function.  You can try the following:

@@
fresh identifier n_values = "n_values";
fresh identifier values = "values";
expression self, unit_type, error;
identifier f;
@@

f(...) {
++ unsigned int n_values = darray_size(hkl_geometry_axes_names_get(self));
++ double values[n_values];
<...
-hkl_geometry_set_values_v(self, unit_type, &error, ...)
+hkl_geometry_axes_values_set(self, values, n_values, unit_type, &error)
...>
}

julia

> ...
> -hkl_geometry_set_values_v(self, unit_type, &error, ...)
> +hkl_geometry_axes_values_set(self, values, n_values, unit_type, &error)
> ...
> }
>
>
> So the idea is to replace a method by another one but to be able to do that, I need to add two new variables n_values and values.
>
> Indeed when I try this spatch, I got this kind of error
>
> picca at ORD03037:~/Projets/hkl/scripts$ ./refactoring geometry.cocci ../tests/*.c
> init_defs_builtins: /usr/share/coccinelle/standard.h
> 190 193
> Fatal error: exception Failure("minus: parse error:
>  = File "geometry.cocci", line 11, column 0,  charpos = 190
>     around = '...', whole content = ...
> ")
> ../tests/hkl3d-test-t.c, ../tests/hkl-axis-t.c
>
>
> I do not understand what's going on, but I am ready to learn with your help.
>
> thanks a lot
>
> Fr?d?ric
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[Cocci] how to add a few decalrations

[picca at synchrotron-soleil.fr]

> @@
> fresh identifier n_values = "n_values";
> fresh identifier values = "values";
> expression self, unit_type, error;
> identifier f;
> @@
> 
> f(...) {
> ++ unsigned int n_values = darray_size(hkl_geometry_axes_names_get(self));
> ++ double values[n_values];
> <...
> -hkl_geometry_set_values_v(self, unit_type, &error, ...)
> +hkl_geometry_axes_values_set(self, values, n_values, unit_type, &error)
> ...>
> }

I understand better but now I get this error.

I will try to make this spatch evolve with your help until it does the right things.
So be prepare to have a few email :)

picca at ORD03037:~/Projets/hkl/scripts$ ./refactoring geometry.cocci ../tests/*.c
init_defs_builtins: /usr/share/coccinelle/standard.h
HANDLING: ../tests/hkl3d-test-t.c ../tests/hkl-axis-t.c ../tests/hkl-bench-t.c ../tests/hkl-detector-t.c ../tests/hkl-geometry-t.c ../tests/hkl-interval-t.c ../tests/hkl-lattice-t.c ../tests/hkl-matrix-t.c ../tests/hkl-parameter-t.c ../tests/hkl-pseudoaxis-e4ch-t.c ../tests/hkl-pseudoaxis-e4cv-t.c ../tests/hkl-pseudoaxis-e6c-t.c ../tests/hkl-pseudoaxis-k4cv-t.c ../tests/hkl-pseudoaxis-k6c-t.c ../tests/hkl-pseudoaxis-soleil-sixs-med-t.c ../tests/hkl-pseudoaxis-t.c ../tests/hkl-pseudoaxis-zaxis-t.c ../tests/hkl-quaternion-t.c ../tests/hkl-sample-t.c ../tests/hkl-source-t.c ../tests/hkl-unit-t.c ../tests/hkl-vector-t.c ../tests/runtests.c
Fatal error: exception Failure("SP line 11: Not found a value in env for: self")
../tests/hkl3d-test-t.c, ../tests/hkl-axis-t.c

Frederic

[Cocci] how to add a few decalrations

[Julia Lawall]

On Thu, 11 Sep 2014, picca at synchrotron-soleil.fr wrote:

> > @@
> > fresh identifier n_values = "n_values";
> > fresh identifier values = "values";
> > expression self, unit_type, error;
> > identifier f;
> > @@
> >
> > f(...) {
> > ++ unsigned int n_values = darray_size(hkl_geometry_axes_names_get(self));
> > ++ double values[n_values];
> > <...
> > -hkl_geometry_set_values_v(self, unit_type, &error, ...)
> > +hkl_geometry_axes_values_set(self, values, n_values, unit_type, &error)
> > ...>
> > }
>
> I understand better but now I get this error.
>
> I will try to make this spatch evolve with your help until it does the right things.
> So be prepare to have a few email :)
>
> picca at ORD03037:~/Projets/hkl/scripts$ ./refactoring geometry.cocci ../tests/*.c
> init_defs_builtins: /usr/share/coccinelle/standard.h
> HANDLING: ../tests/hkl3d-test-t.c ../tests/hkl-axis-t.c ../tests/hkl-bench-t.c ../tests/hkl-detector-t.c ../tests/hkl-geometry-t.c ../tests/hkl-interval-t.c ../tests/hkl-lattice-t.c ../tests/hkl-matrix-t.c ../tests/hkl-parameter-t.c ../tests/hkl-pseudoaxis-e4ch-t.c ../tests/hkl-pseudoaxis-e4cv-t.c ../tests/hkl-pseudoaxis-e6c-t.c ../tests/hkl-pseudoaxis-k4cv-t.c ../tests/hkl-pseudoaxis-k6c-t.c ../tests/hkl-pseudoaxis-soleil-sixs-med-t.c ../tests/hkl-pseudoaxis-t.c ../tests/hkl-pseudoaxis-zaxis-t.c ../tests/hkl-quaternion-t.c ../tests/hkl-sample-t.c ../tests/hkl-source-t.c ../tests/hkl-unit-t.c ../tests/hkl-vector-t.c ../tests/runtests.c
> Fatal error: exception Failure("SP line 11: Not found a value in env for: self")
> ../tests/hkl3d-test-t.c, ../tests/hkl-axis-t.c

OK, it seems that it is trying to add the definitions even if it does not
have any matches for the part inside the <... ...>.  <... ...> eans 0 or
more.  Try changing it to <+... ...+>, which means at least one.

julia

[Cocci] how to add a few decalrations

[picca at synchrotron-soleil.fr]

> OK, it seems that it is trying to add the definitions even if it does not
> have any matches for the part inside the <... ...>.  <... ...> eans 0 or
> more.  Try changing it to <+... ...+>, which means at least one.

with this modification I have no error, but I have also no transformation.

Here I put an exemple of method where I want to do the transformation

static void getter(void)
{
        int res = TRUE;
        HklEngineList *engines;
        HklEngine *engine;
        const HklFactory *factory;
        HklGeometry *geometry;
        HklDetector *detector;
        HklSample *sample;

        factory = hkl_factory_get_by_name("E6C", NULL);
        geometry = hkl_factory_create_new_geometry(factory);
        sample = hkl_sample_new("test");

        detector = hkl_detector_factory_new(HKL_DETECTOR_TYPE_0D);

        engines = hkl_factory_create_new_engine_list(factory);
        hkl_engine_list_init(engines, geometry, detector, sample);

        engine = hkl_engine_list_engine_get_by_name(engines, "hkl", NULL);

        /* geometry -> pseudo */
        hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., 0., 0., 60.);
        res &= DIAG(check_pseudoaxes_v(engine, 0., 0., 1.));

        hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., 90., 0., 60.);
        res &= DIAG(check_pseudoaxes_v(engine, 1., 0., 0.));

        hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., -90., 0., 60.);
        res &= DIAG(check_pseudoaxes_v(engine, -1., 0., 0.));

        hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., 180., 0., 60.);
        res &= DIAG(check_pseudoaxes_v(engine, 0., 0., -1.));

        hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 45., 0., 135., 0., 90.);
        res &= DIAG(check_pseudoaxes_v(engine, 1., 0., -1.));

        ok(res == TRUE, "getter");

        hkl_engine_list_free(engines);
        hkl_detector_free(detector);
        hkl_sample_free(sample);
        hkl_geometry_free(geometry);
}


to be fare, here is the signature of the method to replace

HKLAPI int hkl_geometry_set_values_v(HklGeometry *self, HklUnitEnum unit_type,
				     GError **error, ...) HKL_ARG_NONNULL(1) HKL_WARN_UNUSED_RESULT;


Frederic

[Cocci] how to add a few decalrations

[picca at synchrotron-soleil.fr]

> {
>         int res = TRUE;
>         HklEngineList *engines;
>         HklEngine *engine;
>         const HklFactory *factory;
>         HklGeometry *geometry;
>         HklDetector *detector;
>         HklSample *sample;
> 
>         factory = hkl_factory_get_by_name("E6C", NULL);
>         geometry = hkl_factory_create_new_geometry(factory);
>         sample = hkl_sample_new("test");
> 
>         detector = hkl_detector_factory_new(HKL_DETECTOR_TYPE_0D);
> 
>         engines = hkl_factory_create_new_engine_list(factory);
>         hkl_engine_list_init(engines, geometry, detector, sample);
> 
>         engine = hkl_engine_list_engine_get_by_name(engines, "hkl", NULL);
> 
>         /* geometry -> pseudo */
>         hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., 0., 0., 60.);

re-reading the code I realize that I need to do transformation like this
... (vaargs) -> double values[] = { ... (values) ...};


is it possible ?

[Cocci] how to add a few decalrations

[Julia Lawall]

On Thu, 11 Sep 2014, picca at synchrotron-soleil.fr wrote:

> > {
> >         int res = TRUE;
> >         HklEngineList *engines;
> >         HklEngine *engine;
> >         const HklFactory *factory;
> >         HklGeometry *geometry;
> >         HklDetector *detector;
> >         HklSample *sample;
> >
> >         factory = hkl_factory_get_by_name("E6C", NULL);
> >         geometry = hkl_factory_create_new_geometry(factory);
> >         sample = hkl_sample_new("test");
> >
> >         detector = hkl_detector_factory_new(HKL_DETECTOR_TYPE_0D);
> >
> >         engines = hkl_factory_create_new_engine_list(factory);
> >         hkl_engine_list_init(engines, geometry, detector, sample);
> >
> >         engine = hkl_engine_list_engine_get_by_name(engines, "hkl", NULL);
> >
> >         /* geometry -> pseudo */
> >         hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., 0., 0., 60.);
>
> re-reading the code I realize that I need to do transformation like this
> ... (vaargs) -> double values[] = { ... (values) ...};
>
>
> is it possible ?
I'm not completely sure to understand.  Perhaps send an example of the
code you have and want to produce.

But the more immediate problem is that your pattern has &error and you
code always has NULL in that position.  Do you want to generalize the
pattern (eg expression error;) or do you want to try some other code?

julia

[Cocci] how to add a few decalrations

[picca at synchrotron-soleil.fr]

On Thu, Sep 11, 2014 at 04:16:09PM +0200, Julia Lawall wrote:
> 
> 
> On Thu, 11 Sep 2014, picca at synchrotron-soleil.fr wrote:
> 
> > > {
> > >         int res = TRUE;
> > >         HklEngineList *engines;
> > >         HklEngine *engine;
> > >         const HklFactory *factory;
> > >         HklGeometry *geometry;
> > >         HklDetector *detector;
> > >         HklSample *sample;
> > >
> > >         factory = hkl_factory_get_by_name("E6C", NULL);
> > >         geometry = hkl_factory_create_new_geometry(factory);
> > >         sample = hkl_sample_new("test");
> > >
> > >         detector = hkl_detector_factory_new(HKL_DETECTOR_TYPE_0D);
> > >
> > >         engines = hkl_factory_create_new_engine_list(factory);
> > >         hkl_engine_list_init(engines, geometry, detector, sample);
> > >
> > >         engine = hkl_engine_list_engine_get_by_name(engines, "hkl", NULL);
> > >
> > >         /* geometry -> pseudo */
> > >         hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., 0., 0., 60.);
> >
> > re-reading the code I realize that I need to do transformation like this
> > ... (vaargs) -> double values[] = { ... (values) ...};
> >
> >
> > is it possible ?
> I'm not completely sure to understand.  Perhaps send an example of the
> code you have and want to produce.

I take the previous line and I want 

f(
...

/* for each */
  hkl_geometry_set_values_v(geometry, HKL_UNIT_USER, NULL, 0., 30., 0., 0., 0., 60.);

...
)

->

f{
+ unsigned int n_values; /* only once it is identical for all the set_v methods */
...
geometry = hkl_factory_create_new_geometry(factory); /* initialisation of geometry */
+ n_values = darray_size(*hkl_engine_geometry_axes_names_get(geometry));

...

/* for each */
{
double values[n_values] = {0., 30., 0., 0., 0., 60.};
hkl_geometry_axes_values_set(geometry, values, n_values, HKl_UNIT_USER, NULL);
}

> But the more immediate problem is that your pattern has &error and you
> code always has NULL in that position.  Do you want to generalize the
> pattern (eg expression error;) or do you want to try some other code?

Yes the error is a GError** or NULL;

is it clearer ?

Frederic

[Cocci] how to add a few decalrations

[Julia Lawall]

> > But the more immediate problem is that your pattern has &error and you
> > code always has NULL in that position.  Do you want to generalize the
> > pattern (eg expression error;) or do you want to try some other code?
>
> Yes the error is a GError** or NULL;

I would just use expression error.  I'm not sure thereis a need to be more
specific than that.

julia

[Cocci] how to add a few decalrations

[picca at synchrotron-soleil.fr]

On Thu, Sep 11, 2014 at 04:34:34PM +0200, Julia Lawall wrote:
> > > But the more immediate problem is that your pattern has &error and you
> > > code always has NULL in that position.  Do you want to generalize the
> > > pattern (eg expression error;) or do you want to try some other code?
> >
> > Yes the error is a GError** or NULL;
> 
> I would just use expression error.  I'm not sure thereis a need to be more
> specific than that.

Yes it is better with expression error.

is there exemple of use of transformation on the vaargs part of a method ?


Frederic

[Cocci] how to add a few decalrations

[Julia Lawall]

> is there exemple of use of transformation on the vaargs part of a method ?

Sorry, I don't really know what you want here.  Could you write an example 
of your code before the transformation and your code after?

thanks,
julia

[Cocci] How to add a few declarations?

[SF Markus Elfring]

> is there exemple of use of transformation on the vaargs part of a method ?

Are you looking for a way to adjust source code with functions that
contain an ellipsis in their signature?
Would you like to inspect parameter lists?

Regards,
Markus

[Cocci] How to add a few declarations?

[picca at synchrotron-soleil.fr]

On Fri, Sep 12, 2014 at 10:21:04AM +0200, SF Markus Elfring wrote:
> > is there exemple of use of transformation on the vaargs part of a method ?
> 
> Are you looking for a way to adjust source code with functions that
> contain an ellipsis in their signature?
> Would you like to inspect parameter lists?

Yes exactly.

[Cocci] How to add a few declarations?

[Julia Lawall]

On Fri, 12 Sep 2014, picca at synchrotron-soleil.fr wrote:

> On Fri, Sep 12, 2014 at 10:21:04AM +0200, SF Markus Elfring wrote:
> > > is there exemple of use of transformation on the vaargs part of a method ?
> >
> > Are you looking for a way to adjust source code with functions that
> > contain an ellipsis in their signature?
> > Would you like to inspect parameter lists?
>
> Yes exactly.

Sorry, but that is not clear enough for me to have an idea of what you
want to do.  Please send a concrete example.

julia

[Cocci] How to add a few declarations?

[picca at synchrotron-soleil.fr]

On Fri, Sep 12, 2014 at 10:43:55AM +0200, Julia Lawall wrote:
> On Fri, 12 Sep 2014, picca at synchrotron-soleil.fr wrote:
> 
> > On Fri, Sep 12, 2014 at 10:21:04AM +0200, SF Markus Elfring wrote:
> > > > is there exemple of use of transformation on the vaargs part of a method ?
> > >
> > > Are you looking for a way to adjust source code with functions that
> > > contain an ellipsis in their signature?
> > > Would you like to inspect parameter lists?
> >
> > Yes exactly.
> 
> Sorry, but that is not clear enough for me to have an idea of what you
> want to do.  Please send a concrete example.
> 
> julia

Hello,

I want to transform this method

int f(T1 x, T2 y, T3 z, ...);

called like this

f(x, y, z, 0.0, 6.0, 5.0)

into

{
	double values[] = {0.0, 6.0, 5.0};
	unsigned int n_values = ARRAY_SIZE(values);
	f2(x, values, n_values, y, z);
}

where the signature of f2 is

int f(T1 x, double[] v, unsigned int n_v, T2 y, T3, z)


the variadic part of the method contain only double in my case.

BEtter ?

Frederic

[Cocci] How to add a few declarations?

[Julia Lawall]

> I want to transform this method
>
> int f(T1 x, T2 y, T3 z, ...);
>
> called like this
>
> f(x, y, z, 0.0, 6.0, 5.0)
>
> into
>
> {
> 	double values[] = {0.0, 6.0, 5.0};
> 	unsigned int n_values = ARRAY_SIZE(values);
> 	f2(x, values, n_values, y, z);
> }
>
> where the signature of f2 is
>
> int f(T1 x, double[] v, unsigned int n_v, T2 y, T3, z)
>
>
> the variadic part of the method contain only double in my case.

At the moment you can't match against ... in a parameter list.  So you
would have to know which methods have this property.  However, you can
replace the parameter list and put in a new one.

Matching the call site is more feasible.  You could match the above call
as:

@@
expression x, y, z;
expression list es;
fresh identifier values, n_values;
@@

- f(x, y, z, es);
+ { double values[] = { es };
+   unsigned int n_values = ARRAY_SIZE(values);
+   f2(x, values, n_values, y, z);
+ }

I'm not completely sure that it will let you use an expression list in
this way.  If it doesn't work out, then write back and I will give you a
more hackish solution.

julia

[Cocci] How to add a few declarations?

[picca at synchrotron-soleil.fr]

> @@
> expression x, y, z;
> expression list es;
> fresh identifier values, n_values;
> @@
> 
> - f(x, y, z, es);
> + { double values[] = { es };
> +   unsigned int n_values = ARRAY_SIZE(values);
> +   f2(x, values, n_values, y, z);
> + }

thanks a lot I will try.
I will keep you informed.

Cheers

Frederic

[Cocci] Transformations around variadic functions

[SF Markus Elfring]

>> Are you looking for a way to adjust source code with functions that
>> contain an ellipsis in their signature?
>> Would you like to inspect parameter lists?
> Yes exactly.

There are some software development challenges in the corresponding
handling of variadic functions.
https://systeme.lip6.fr/pipermail/cocci/2014-March/000698.html
http://article.gmane.org/gmane.comp.version-control.coccinelle/3521

Have you got any more concrete imaginations for your use case?