[cocci] Possible bug in a semantic patch

[cocci] Possible bug in a semantic patch

[Elia Pinto]

Greetings to all

I am trying to apply this semantic patch

****sp.cocci file ***********************************

/ * SPDX-License-Identifier: LGPL-2.1-or-later * /

@@

type T;

identifier I;

statement S;

expression E1, E2, E3;

@@



- T I;

  ... when != I

- for (I = E1; E2; E3)

+ for (T I = E1; E2; E3)

    S

  ... when != I



****************************************

to this repository

https://github.com/git/git.git

The semantic patch works on almost any code. However, it produces an
incorrect transformation on the refs.c file
https://github.com/git/git/blob/master/refs.c

Doing

spatch --sp-file sp.cocci refs.c -o refs.after.c

An incorrect transformation is created:

diff -Nu refs.c refs.after.c

.....

@@ -733.12 +730.12 @@

 int expand_ref (struct repository * repo, const char * str, int len,

               struct object_id * oid, char ** ref)

 {

- const char ** p, * r;

+ const char ** r;       ç========================= NOTE



......

which erroneously changes the type of the variable r.

Is it a bug or should the semantic patch be done better?

Thanks in advance



Elia

Re: [cocci] Possible bug in a semantic patch

[Julia Lawall]

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



On Wed, 9 Nov 2022, Elia Pinto wrote:

> Greetings to all
>
> I am trying to apply this semantic patch
>
> ****sp.cocci file ***********************************
>
> / * SPDX-License-Identifier: LGPL-2.1-or-later * /
>
> @@
>
> type T;
>
> identifier I;
>
> statement S;
>
> expression E1, E2, E3;
>
> @@
>
>
>
> - T I;
>
>   ... when != I
>
> - for (I = E1; E2; E3)
>
> + for (T I = E1; E2; E3)
>
>     S
>
>   ... when != I
>
>
>
> ****************************************
>
> to this repository
>
> https://github.com/git/git.git
>
> The semantic patch works on almost any code. However, it produces an
> incorrect transformation on the refs.c file
> https://github.com/git/git/blob/master/refs.c
>
> Doing
>
> spatch --sp-file sp.cocci refs.c -o refs.after.c
>
> An incorrect transformation is created:
>
> diff -Nu refs.c refs.after.c
>
> .....
>
> @@ -733.12 +730.12 @@
>
>  int expand_ref (struct repository * repo, const char * str, int len,
>
>                struct object_id * oid, char ** ref)
>
>  {
>
> - const char ** p, * r;
>
> + const char ** r;       ç========================= NOTE
>
>
>
> ......
>
> which erroneously changes the type of the variable r.
>
> Is it a bug or should the semantic patch be done better?

Sorry, definitely a bug.  Thanks for the report.

julia

>
> Thanks in advance
>
>
>
> Elia
>

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

> The semantic patch works on almost any code. However, it produces an
> incorrect transformation on the refs.c file
> https://github.com/git/git/blob/master/refs.c
>
> Doing
>
> spatch --sp-file sp.cocci refs.c -o refs.after.c
>
> An incorrect transformation is created:
…
> which erroneously changes the type of the variable r.
>
> Is it a bug


You found a detail which will hopefully trigger further collateral evolution.

Another test result:

Markus_Elfring@Sonne:…/Projekte/Coccinelle/Probe> spatch suggestion_for_Elia_Pinto-20221109.cocci expand_ref-test.c
…
@@ -1,6 +1,6 @@
 void my_expand_ref_variant(void)
 {
- const char **p, *r;
- for (p = ref_rev_parse_rules; *p; ++p)
+ const char **r;
+ for (const char **p = ref_rev_parse_rules; *p; ++p)
  { /* loop content */ }
 }


> or should the semantic patch be done better?

How do you think about software development possibilities with the following
script variant for the semantic patch language?


@movement@
type T;
identifier I;
statement S;
expression E1, E2, E3;
@@
(
-T I = E1;
 ... when != I
 for (
+     T
      I = E1; E2; E3
     )
 S
 ... when != I
|
-T I;
 ... when != I
 for (
+     T
      I = E1; E2; E3
     )
 S
 ... when != I
)


Regards,
Markus

Re: [cocci] Reducing the scope for variables with SmPL

[Markus Elfring]

>> or should the semantic patch be done better?
> How do you think about software development possibilities with the following
> script variant for the semantic patch language?


@movement disable decl_init@
type T;
identifier I;
initializer init;
expression E1, E2, E3;
statement S;
@@
(
-T I = init;
 ... when != I
 for (
+     T
      I =
-         E1
+         init
      ; E2; E3
     )
 S
 ... when != I
|
-T I;
 ... when != I
 for (
+     T
      I = E1; E2; E3
     )
 S
 ... when != I
)



Would you like to reduce the scope for any more variables?
https://refactoring.com/catalog/reduceScopeOfVariable.html


Does such a source code transformation approach point also any opportunities out
for further improvements of the corresponding software documentation?

Regards,
Markus

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

> Is it a bug

Would you like to clarify a related bug report for the software combination “Coccinelle 1.1.1”?

SmPL script:
@moving disable decl_init@
type t;
identifier x;
@@
-t x;
+t
 x = 'x';

Source file example:
void my_test_for_declarators(void)
{
 char o, *p;
 o = 'x';
}

Another test result:

Markus_Elfring@Sonne:…/Projekte/Coccinelle/Probe> spatch move_a_variable_declaration-20221112.cocci test_for_declarators1.c
…


Now I wonder why the expected diff output is not generated so far.

Should data like the following be presented?

-char o, *p;
+char *p;
-o = 'x';
+char o = 'x';


Regards,
Markus

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

> Would you like to clarify a related bug report for the software combination “Coccinelle 1.1.1”?

Can another variant make sense for a transformation specification
if you would like to increase the change precision?

@moving2 disable decl_init@
type t;
identifier x, y;
@@
 t
-x,
 * y;
+t
 x = 'x';


Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Coccinelle/Probe> spatch --parse-cocci move_a_variable_declaration2-20221112.cocci
…
minus: parse error:
  File "move_a_variable_declaration2-20221112.cocci", line 7, column 1, charpos = 64
  around = '*',
  whole content =  * y;


How will the handling of declarators evolve further?
https://en.cppreference.com/w/c/language/declarations

Regards,
Markus

Re: [cocci] Fixing the adjustment of variable declarations

[Julia Lawall]

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



On Sat, 12 Nov 2022, Markus Elfring wrote:

>
> > Would you like to clarify a related bug report for the software combination “Coccinelle 1.1.1”?
>
> Can another variant make sense for a transformation specification
> if you would like to increase the change precision?
>
> @moving2 disable decl_init@
> type t;
> identifier x, y;
> @@
>  t
> -x,
>  * y;
> +t
>  x = 'x';

Have you actually tried this?  Because multiple declarations, ie of both x
and y, are not supported in SmPL.  Please actually try suggestions before
posting them.

julia

>
>
> Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Coccinelle/Probe> spatch --parse-cocci move_a_variable_declaration2-20221112.cocci
> …
> minus: parse error:
>   File "move_a_variable_declaration2-20221112.cocci", line 7, column 1, charpos = 64
>   around = '*',
>   whole content =  * y;
>
>
> How will the handling of declarators evolve further?
> https://en.cppreference.com/w/c/language/declarations
>
> Regards,
> Markus
>

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

>> Can another variant make sense for a transformation specification
>> if you would like to increase the change precision?
>>
>> @moving2 disable decl_init@
>> type t;
>> identifier x, y;
>> @@
>>  t
>> -x,
>>  * y;
>> +t
>>  x = 'x';
> Have you actually tried this?


Yes (in principle).

I stumbled on the message “minus: parse error:” accordingly.


> Because multiple declarations, ie of both x and y, are not supported in SmPL.

Can this software status be reconsidered?
https://en.cppreference.com/w/c/language/declarations

Regards,
Markus

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

> @moving disable decl_init@
> type t;
> identifier x;
> @@
> -t x;
> +t
>  x = 'x';

I would appreciate further feedback also for this tiny SmPL script variant.
I got the impression that additional clarification will be helpful
for some aspects.
https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/768bd6eb0a2cbede8f1390009cc997ff57c339a4/docs/manual/cocci_syntax.tex#L1022

This transformation approach would require that a data type can be determined
from a source code fragment which should be adjusted before the same type
can be added to a subsequent statement.
How good can such changes be supported by the Coccinelle software?

Regards,
Markus

Re: [cocci] Fixing the adjustment of variable declarations

[Julia Lawall]

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



On Sun, 13 Nov 2022, Markus Elfring wrote:

> > @moving disable decl_init@
> > type t;
> > identifier x;
> > @@
> > -t x;
> > +t
> >  x = 'x';
>
> I would appreciate further feedback also for this tiny SmPL script variant.
> I got the impression that additional clarification will be helpful
> for some aspects.
> https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/768bd6eb0a2cbede8f1390009cc997ff57c339a4/docs/manual/cocci_syntax.tex#L1022
>
> This transformation approach would require that a data type can be determined
> from a source code fragment which should be adjusted before the same type
> can be added to a subsequent statement.
> How good can such changes be supported by the Coccinelle software?

What is the problem with this semantic patch?  You didn't have the
thoughfulness to provide a test case, but I tried one and it worked fine.

int main () {
  int x;
  x = 'x';
}

julia

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

> What is the problem with this semantic patch?

It tried to express a reduced transformation approach also according to
a clarification attempt by Elia Pinto.



> You didn't have the thoughfulness to provide a test case,

I published one yesterday.
https://lore.kernel.org/cocci/b73dc7c1-d5ad-dd23-f559-7b73939ed710@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2022-11/msg00040.html


> but I tried one and it worked fine.
>
> int main () {
>   int x;
>   x = 'x';
> }

I can get a related test result.

Markus_Elfring@Sonne:…/Projekte/Coccinelle/Probe> spatch move_a_variable_declaration-20221112.cocci test_for_declarators2.c
…
@@ -1,5 +1,5 @@
 void my_test_for_one_declaration(void)
 {
- char o;
+ char
  o = 'x';
 }


Does it also mean that information from code deletions can generally be captured
for subsequent code additions?
https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/768bd6eb0a2cbede8f1390009cc997ff57c339a4/docs/manual/cocci_syntax.tex#L1022

Why do I observe test results which are different in comparison to
the previously mentioned SmPL code variant “-T I;”?

Regards,
Markus

Re: [cocci] Fixing the adjustment of variable declarations

[Julia Lawall]

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



On Sun, 13 Nov 2022, Markus Elfring wrote:

> > What is the problem with this semantic patch?
>
> It tried to express a reduced transformation approach also according to
> a clarification attempt by Elia Pinto.
>
>
>
> > You didn't have the thoughfulness to provide a test case,
>
> I published one yesterday.
> https://lore.kernel.org/cocci/b73dc7c1-d5ad-dd23-f559-7b73939ed710@web.de/
> https://sympa.inria.fr/sympa/arc/cocci/2022-11/msg00040.html
>
>
> > but I tried one and it worked fine.
> >
> > int main () {
> >   int x;
> >   x = 'x';
> > }
>
> I can get a related test result.
>
> Markus_Elfring@Sonne:…/Projekte/Coccinelle/Probe> spatch move_a_variable_declaration-20221112.cocci test_for_declarators2.c
> …
> @@ -1,5 +1,5 @@
>  void my_test_for_one_declaration(void)
>  {
> - char o;
> + char
>   o = 'x';
>  }
>
>
> Does it also mean that information from code deletions can generally be captured
> for subsequent code additions?
> https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/768bd6eb0a2cbede8f1390009cc997ff57c339a4/docs/manual/cocci_syntax.tex#L1022
>
> Why do I observe test results which are different in comparison to
> the previously mentioned SmPL code variant “-T I;”?

Elia Pinto's problem is related to the management of *s.  You can't see
the problem on a simple type like int.

julia


>
> Regards,
> Markus
>

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

>> Why do I observe test results which are different in comparison to
>> the previously mentioned SmPL code variant “-T I;”?
> Elia Pinto's problem is related to the management of *s.

Do you notice really only open issues with the handling of asterisks
for pointer data types so far?



> You can't see the problem on a simple type like int.

I find that my mentioned test case points further questionable software behaviour out.
https://lore.kernel.org/cocci/b73dc7c1-d5ad-dd23-f559-7b73939ed710@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2022-11/msg00040.html

How will development considerations evolve accordingly?

Regards,
Markus

Re: [cocci] Fixing the adjustment of variable declarations

[Julia Lawall]

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



On Sun, 13 Nov 2022, Markus Elfring wrote:

> >> Why do I observe test results which are different in comparison to
> >> the previously mentioned SmPL code variant “-T I;”?
> > Elia Pinto's problem is related to the management of *s.
>
> Do you notice really only open issues with the handling of asterisks
> for pointer data types so far?
>
>
>
> > You can't see the problem on a simple type like int.
>
> I find that my mentioned test case points further questionable software behaviour out.
> https://lore.kernel.org/cocci/b73dc7c1-d5ad-dd23-f559-7b73939ed710@web.de/
> https://sympa.inria.fr/sympa/arc/cocci/2022-11/msg00040.html
>
> How will development considerations evolve accordingly?

If you use the option --debug, you get an explanation:

     (ONCE) compress.c: 2: More than one variable in the declaration, and
so it cannot be transformed.  Check that there is no transformation on the
type or the ;.  Consider using ++ for an addition.

julia

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

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

>> How will development considerations evolve accordingly?
> If you use the option --debug, you get an explanation:
>
>      (ONCE) compress.c: 2: More than one variable in the declaration, and
> so it cannot be transformed.  Check that there is no transformation on the
> type or the ;.  …


Markus_Elfring@Sonne:…/Projekte/Coccinelle/Probe> spatch --debug suggestion3_for_Elia_Pinto-20221111.cocci expand_ref-test.c
…
@@ -1,6 +1,6 @@
void my_expand_ref_variant(void)
{
- const char **p, *r;
- for (p = ref_rev_parse_rules; *p; ++p)
+ const char **r;
+ for (const char **p = ref_rev_parse_rules; *p; ++p)
 { /* loop content */ }
}
…


The error message was not displayed for such a source code variant.

I got the impression instead that multiple declarators could be handled
to some degree for changeable variable declarations also by the means of
the semantic patch language.

Regards,
Markus

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

Re: [cocci] Fixing the adjustment of variable declarations

[Julia Lawall]

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



On Sun, 13 Nov 2022, Markus Elfring wrote:

>
> How will development considerations evolve accordingly?
>
> If you use the option --debug, you get an explanation:
>
>      (ONCE) compress.c: 2: More than one variable in the declaration, and
> so it cannot be transformed.  Check that there is no transformation on the
> type or the ;.  …
>
>
> Markus_Elfring@Sonne:…/Projekte/Coccinelle/Probe> spatch --debug suggestion3_for_Elia_Pinto-20221111.cocci expand_ref-test.c
> …
> @@ -1,6 +1,6 @@
> void my_expand_ref_variant(void)
> {
> - const char **p, *r;
> - for (p = ref_rev_parse_rules; *p; ++p)
> + const char **r;
> + for (const char **p = ref_rev_parse_rules; *p; ++p)
>  { /* loop content */ }
> }
> …
>
>
> The error message was not displayed for such a source code variant.
>
> I got the impression instead that multiple declarators could be handled
> to some degree for changeable variable declarations also by the means of
> the semantic patch language.

That is correct.  They can be handled to some degree.  That doesn't
contradict the message provided.

julia

Re: [cocci] Fixing the adjustment of variable declarations

[Markus Elfring]

>> The error message was not displayed for such a source code variant.
>>
>> I got the impression instead that multiple declarators could be handled
>> to some degree for changeable variable declarations also by the means of
>> the semantic patch language.
> That is correct.  They can be handled to some degree.


Thanks for your acknowledgement.



> That doesn't contradict the message provided.

I interpret the discussed software situation differently.

Should the source code fragment “char **p, *r;” be treated as an indication for
the declaration of two variables with different (pointer) data types?

Does hinder anything to extend the support for all other data types accordingly?

Regards,
Markus