[Cocci] remove redundant const qualifiers from function arguments

[Cocci] remove redundant const qualifiers from function arguments

[Sören Brinkmann]

Hi,

I'm trying to remove 'const' qualifiers from function arguments that are
passed by value.

e.g. I want to convert:

void foobar(const int bar, const int *baz);
to
void foobar(int bar, const int *baz);

I came up with this semantic patch:
/* Remove const from arguments passed by value */
@ rule1 @
identifier fi, I;
type T;
@@
 fi ( ...,
-	const T
+	T
	I
	,...
 )
 { ... }

That seems to work fine on function declarations, but does not cover
function prototypes. Extending the patch to function prototypes I ran
into some trouble. As complete separate patch, I evolved the above to
(just replacing the function body with a ';'):
/* Remove const from arguments passed by value */
@ rule1 @
identifier fi, I;
type T;
@@
 fi ( ...,
-	const T
+	T
	I
	,...
 ) ;

But that is not parsed by spatch, causing this error:
  init_defs_builtins: /usr/local/lib/coccinelle/standard.h
  minus: parse error:
    File "const_proto.cocci", line 9, column 1, charpos = 115
    around = 'I',
    whole content = '     I'

Also, using the first version of a patch on a c-file with matching function
declarations that also includes function prototypes, results in both,
declarations and prototypes, while using it on a header with prototypes
only doesn't result in any replacements.

I attach the patches and some test files for reference.
const.cocci apparently correctly removes all const keywords from the
test.c file, but does not change the .h at all (which is correct, just
weird that it changes the prototype in the .c).

The const_proto.cocci file causes the above-mentioned error.

Does anybody know what I'm missing here?

	Thanks,
	S?ren
-------------- next part --------------
/* Remove const from arguments passed by value */
@ rule1 @
identifier fi, I;
type T;
@@
 fi ( ...,
-	const T
+	T
	I
	,...
 )
 { ... }

-------------- next part --------------
/* Remove const from arguments passed by value */
@ rule1 @
identifier fi, I;
type T;
@@
 fi ( ...,
-	const T
+	T
	I
	,...
 ) ;

-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.c
Type: text/x-csrc
Size: 635 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160510/24ef54e3/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.h
Type: text/x-chdr
Size: 46 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160510/24ef54e3/attachment-0001.bin>

[Cocci] remove redundant const qualifiers from function arguments

[Julia Lawall]

On Tue, 10 May 2016, S?ren Brinkmann wrote:

> Hi,
>
> I'm trying to remove 'const' qualifiers from function arguments that are
> passed by value.
>
> e.g. I want to convert:
>
> void foobar(const int bar, const int *baz);
> to
> void foobar(int bar, const int *baz);
>
> I came up with this semantic patch:
> /* Remove const from arguments passed by value */
> @ rule1 @
> identifier fi, I;
> type T;
> @@
>  fi ( ...,
> -	const T
> +	T
> 	I
> 	,...
>  )
>  { ... }
>
> That seems to work fine on function declarations, but does not cover
> function prototypes. Extending the patch to function prototypes I ran
> into some trouble. As complete separate patch, I evolved the above to
> (just replacing the function body with a ';'):
> /* Remove const from arguments passed by value */
> @ rule1 @
> identifier fi, I;
> type T;
> @@
>  fi ( ...,
> -	const T
> +	T
> 	I
> 	,...
>  ) ;
>
> But that is not parsed by spatch, causing this error:
>   init_defs_builtins: /usr/local/lib/coccinelle/standard.h
>   minus: parse error:
>     File "const_proto.cocci", line 9, column 1, charpos = 115
>     around = 'I',
>     whole content = '     I'
>
> Also, using the first version of a patch on a c-file with matching function
> declarations that also includes function prototypes, results in both,
> declarations and prototypes, while using it on a header with prototypes
> only doesn't result in any replacements.
>
> I attach the patches and some test files for reference.
> const.cocci apparently correctly removes all const keywords from the
> test.c file, but does not change the .h at all (which is correct, just
> weird that it changes the prototype in the .c).
>
> The const_proto.cocci file causes the above-mentioned error.
>
> Does anybody know what I'm missing here?

By default, when you change a function header, Coccinell changes the
prototype accordingly.  But to be able to do that it needs to have access
to the prototype.  So you may need to give an argument like --all-includes
(include all the .h files mentioned in the .c file) or --recursive-incudes
(include all the .h files recurively included by any .h file mentioned in
the .c file).  --recursive-includes at least may be slow, but things will
be done in a consistent manner.

I will check on why the expliciti prototype rule doesn't parse.

julia

[Cocci] remove redundant const qualifiers from function arguments

[Sören Brinkmann]

Hi Julia,

On Wed, 2016-05-11 at 07:41:20 +0200, Julia Lawall wrote:
> 
> 
> On Tue, 10 May 2016, S?ren Brinkmann wrote:
> 
> > Hi,
> >
> > I'm trying to remove 'const' qualifiers from function arguments that are
> > passed by value.
> >
> > e.g. I want to convert:
> >
> > void foobar(const int bar, const int *baz);
> > to
> > void foobar(int bar, const int *baz);
> >
> > I came up with this semantic patch:
> > /* Remove const from arguments passed by value */
> > @ rule1 @
> > identifier fi, I;
> > type T;
> > @@
> >  fi ( ...,
> > -	const T
> > +	T
> > 	I
> > 	,...
> >  )
> >  { ... }
> >
> > That seems to work fine on function declarations, but does not cover
> > function prototypes. Extending the patch to function prototypes I ran
> > into some trouble. As complete separate patch, I evolved the above to
> > (just replacing the function body with a ';'):
> > /* Remove const from arguments passed by value */
> > @ rule1 @
> > identifier fi, I;
> > type T;
> > @@
> >  fi ( ...,
> > -	const T
> > +	T
> > 	I
> > 	,...
> >  ) ;
> >
> > But that is not parsed by spatch, causing this error:
> >   init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> >   minus: parse error:
> >     File "const_proto.cocci", line 9, column 1, charpos = 115
> >     around = 'I',
> >     whole content = '     I'
> >
> > Also, using the first version of a patch on a c-file with matching function
> > declarations that also includes function prototypes, results in both,
> > declarations and prototypes, while using it on a header with prototypes
> > only doesn't result in any replacements.
> >
> > I attach the patches and some test files for reference.
> > const.cocci apparently correctly removes all const keywords from the
> > test.c file, but does not change the .h at all (which is correct, just
> > weird that it changes the prototype in the .c).
> >
> > The const_proto.cocci file causes the above-mentioned error.
> >
> > Does anybody know what I'm missing here?
> 
> By default, when you change a function header, Coccinell changes the
> prototype accordingly.  But to be able to do that it needs to have access
> to the prototype.  So you may need to give an argument like --all-includes
> (include all the .h files mentioned in the .c file) or --recursive-incudes
> (include all the .h files recurively included by any .h file mentioned in
> the .c file).  --recursive-includes at least may be slow, but things will
> be done in a consistent manner.

Interesting. I tried different variations of the include switches on the
real code base, but none resulted in the headers to be updated. Though,
with those switches I get 'different modification result for foo.h' warnings.

For the test files, including the header in the c file and running
spatch with --local-includes works as expected.

Guess I'll dig into the headers and check if there are any unusual
constructs causing the problem. Any hints would be appreciated :)

> I will check on why the expliciti prototype rule doesn't parse.

Thanks for looking into it.

	S?ren

[Cocci] remove redundant const qualifiers from function arguments

[Julia Lawall]

On Tue, 10 May 2016, S?ren Brinkmann wrote:

> Hi Julia,
>
> On Wed, 2016-05-11 at 07:41:20 +0200, Julia Lawall wrote:
> >
> >
> > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> >
> > > Hi,
> > >
> > > I'm trying to remove 'const' qualifiers from function arguments that are
> > > passed by value.
> > >
> > > e.g. I want to convert:
> > >
> > > void foobar(const int bar, const int *baz);
> > > to
> > > void foobar(int bar, const int *baz);
> > >
> > > I came up with this semantic patch:
> > > /* Remove const from arguments passed by value */
> > > @ rule1 @
> > > identifier fi, I;
> > > type T;
> > > @@
> > >  fi ( ...,
> > > -	const T
> > > +	T
> > > 	I
> > > 	,...
> > >  )
> > >  { ... }
> > >
> > > That seems to work fine on function declarations, but does not cover
> > > function prototypes. Extending the patch to function prototypes I ran
> > > into some trouble. As complete separate patch, I evolved the above to
> > > (just replacing the function body with a ';'):
> > > /* Remove const from arguments passed by value */
> > > @ rule1 @
> > > identifier fi, I;
> > > type T;
> > > @@
> > >  fi ( ...,
> > > -	const T
> > > +	T
> > > 	I
> > > 	,...
> > >  ) ;
> > >
> > > But that is not parsed by spatch, causing this error:
> > >   init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> > >   minus: parse error:
> > >     File "const_proto.cocci", line 9, column 1, charpos = 115
> > >     around = 'I',
> > >     whole content = '     I'
> > >
> > > Also, using the first version of a patch on a c-file with matching function
> > > declarations that also includes function prototypes, results in both,
> > > declarations and prototypes, while using it on a header with prototypes
> > > only doesn't result in any replacements.
> > >
> > > I attach the patches and some test files for reference.
> > > const.cocci apparently correctly removes all const keywords from the
> > > test.c file, but does not change the .h at all (which is correct, just
> > > weird that it changes the prototype in the .c).
> > >
> > > The const_proto.cocci file causes the above-mentioned error.
> > >
> > > Does anybody know what I'm missing here?
> >
> > By default, when you change a function header, Coccinell changes the
> > prototype accordingly.  But to be able to do that it needs to have access
> > to the prototype.  So you may need to give an argument like --all-includes
> > (include all the .h files mentioned in the .c file) or --recursive-incudes
> > (include all the .h files recurively included by any .h file mentioned in
> > the .c file).  --recursive-includes at least may be slow, but things will
> > be done in a consistent manner.
>
> Interesting. I tried different variations of the include switches on the
> real code base, but none resulted in the headers to be updated. Though,
> with those switches I get 'different modification result for foo.h' warnings.
>
> For the test files, including the header in the c file and running
> spatch with --local-includes works as expected.
>
> Guess I'll dig into the headers and check if there are any unusual
> constructs causing the problem. Any hints would be appreciated :)
>
> > I will check on why the expliciti prototype rule doesn't parse.
>
> Thanks for looking into it.

Could you send an example of a .c and .h file where the result is not as
expected?

thanks,
julia

[Cocci] remove redundant const qualifiers from function arguments

[Sören Brinkmann]

On Wed, 2016-05-11 at 07:59:06 +0200, Julia Lawall wrote:
> 
> 
> On Tue, 10 May 2016, S?ren Brinkmann wrote:
> 
> > Hi Julia,
> >
> > On Wed, 2016-05-11 at 07:41:20 +0200, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> > >
> > > > Hi,
> > > >
> > > > I'm trying to remove 'const' qualifiers from function arguments that are
> > > > passed by value.
> > > >
> > > > e.g. I want to convert:
> > > >
> > > > void foobar(const int bar, const int *baz);
> > > > to
> > > > void foobar(int bar, const int *baz);
> > > >
> > > > I came up with this semantic patch:
> > > > /* Remove const from arguments passed by value */
> > > > @ rule1 @
> > > > identifier fi, I;
> > > > type T;
> > > > @@
> > > >  fi ( ...,
> > > > -	const T
> > > > +	T
> > > > 	I
> > > > 	,...
> > > >  )
> > > >  { ... }
> > > >
> > > > That seems to work fine on function declarations, but does not cover
> > > > function prototypes. Extending the patch to function prototypes I ran
> > > > into some trouble. As complete separate patch, I evolved the above to
> > > > (just replacing the function body with a ';'):
> > > > /* Remove const from arguments passed by value */
> > > > @ rule1 @
> > > > identifier fi, I;
> > > > type T;
> > > > @@
> > > >  fi ( ...,
> > > > -	const T
> > > > +	T
> > > > 	I
> > > > 	,...
> > > >  ) ;
> > > >
> > > > But that is not parsed by spatch, causing this error:
> > > >   init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> > > >   minus: parse error:
> > > >     File "const_proto.cocci", line 9, column 1, charpos = 115
> > > >     around = 'I',
> > > >     whole content = '     I'
> > > >
> > > > Also, using the first version of a patch on a c-file with matching function
> > > > declarations that also includes function prototypes, results in both,
> > > > declarations and prototypes, while using it on a header with prototypes
> > > > only doesn't result in any replacements.
> > > >
> > > > I attach the patches and some test files for reference.
> > > > const.cocci apparently correctly removes all const keywords from the
> > > > test.c file, but does not change the .h at all (which is correct, just
> > > > weird that it changes the prototype in the .c).
> > > >
> > > > The const_proto.cocci file causes the above-mentioned error.
> > > >
> > > > Does anybody know what I'm missing here?
> > >
> > > By default, when you change a function header, Coccinell changes the
> > > prototype accordingly.  But to be able to do that it needs to have access
> > > to the prototype.  So you may need to give an argument like --all-includes
> > > (include all the .h files mentioned in the .c file) or --recursive-incudes
> > > (include all the .h files recurively included by any .h file mentioned in
> > > the .c file).  --recursive-includes at least may be slow, but things will
> > > be done in a consistent manner.
> >
> > Interesting. I tried different variations of the include switches on the
> > real code base, but none resulted in the headers to be updated. Though,
> > with those switches I get 'different modification result for foo.h' warnings.
> >
> > For the test files, including the header in the c file and running
> > spatch with --local-includes works as expected.
> >
> > Guess I'll dig into the headers and check if there are any unusual
> > constructs causing the problem. Any hints would be appreciated :)
> >
> > > I will check on why the expliciti prototype rule doesn't parse.
> >
> > Thanks for looking into it.
> 
> Could you send an example of a .c and .h file where the result is not as
> expected?

It works now... It seems something is handled significantly different
depending on how the target files are specified on the command line.

Initially I ran:
  spatch --sp-file const.cocci --local-includes --in-place --dir .
and now with your hints, I changed to
  spatch --sp-file const.cocci --local-includes --in-place *.c

That way headers are updated as expected and the warnings are gone.

I can try to condense a small test case tomorrow, if you're interested/necessary.

	Thanks,
	S?ren

[Cocci] remove redundant const qualifiers from function arguments

[Julia Lawall]

On Tue, 10 May 2016, S?ren Brinkmann wrote:

> On Wed, 2016-05-11 at 07:59:06 +0200, Julia Lawall wrote:
> >
> >
> > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> >
> > > Hi Julia,
> > >
> > > On Wed, 2016-05-11 at 07:41:20 +0200, Julia Lawall wrote:
> > > >
> > > >
> > > > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > I'm trying to remove 'const' qualifiers from function arguments that are
> > > > > passed by value.
> > > > >
> > > > > e.g. I want to convert:
> > > > >
> > > > > void foobar(const int bar, const int *baz);
> > > > > to
> > > > > void foobar(int bar, const int *baz);
> > > > >
> > > > > I came up with this semantic patch:
> > > > > /* Remove const from arguments passed by value */
> > > > > @ rule1 @
> > > > > identifier fi, I;
> > > > > type T;
> > > > > @@
> > > > >  fi ( ...,
> > > > > -	const T
> > > > > +	T
> > > > > 	I
> > > > > 	,...
> > > > >  )
> > > > >  { ... }
> > > > >
> > > > > That seems to work fine on function declarations, but does not cover
> > > > > function prototypes. Extending the patch to function prototypes I ran
> > > > > into some trouble. As complete separate patch, I evolved the above to
> > > > > (just replacing the function body with a ';'):
> > > > > /* Remove const from arguments passed by value */
> > > > > @ rule1 @
> > > > > identifier fi, I;
> > > > > type T;
> > > > > @@
> > > > >  fi ( ...,
> > > > > -	const T
> > > > > +	T
> > > > > 	I
> > > > > 	,...
> > > > >  ) ;
> > > > >
> > > > > But that is not parsed by spatch, causing this error:
> > > > >   init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> > > > >   minus: parse error:
> > > > >     File "const_proto.cocci", line 9, column 1, charpos = 115
> > > > >     around = 'I',
> > > > >     whole content = '     I'
> > > > >
> > > > > Also, using the first version of a patch on a c-file with matching function
> > > > > declarations that also includes function prototypes, results in both,
> > > > > declarations and prototypes, while using it on a header with prototypes
> > > > > only doesn't result in any replacements.
> > > > >
> > > > > I attach the patches and some test files for reference.
> > > > > const.cocci apparently correctly removes all const keywords from the
> > > > > test.c file, but does not change the .h at all (which is correct, just
> > > > > weird that it changes the prototype in the .c).
> > > > >
> > > > > The const_proto.cocci file causes the above-mentioned error.
> > > > >
> > > > > Does anybody know what I'm missing here?
> > > >
> > > > By default, when you change a function header, Coccinell changes the
> > > > prototype accordingly.  But to be able to do that it needs to have access
> > > > to the prototype.  So you may need to give an argument like --all-includes
> > > > (include all the .h files mentioned in the .c file) or --recursive-incudes
> > > > (include all the .h files recurively included by any .h file mentioned in
> > > > the .c file).  --recursive-includes at least may be slow, but things will
> > > > be done in a consistent manner.
> > >
> > > Interesting. I tried different variations of the include switches on the
> > > real code base, but none resulted in the headers to be updated. Though,
> > > with those switches I get 'different modification result for foo.h' warnings.
> > >
> > > For the test files, including the header in the c file and running
> > > spatch with --local-includes works as expected.
> > >
> > > Guess I'll dig into the headers and check if there are any unusual
> > > constructs causing the problem. Any hints would be appreciated :)
> > >
> > > > I will check on why the expliciti prototype rule doesn't parse.
> > >
> > > Thanks for looking into it.
> >
> > Could you send an example of a .c and .h file where the result is not as
> > expected?
>
> It works now... It seems something is handled significantly different
> depending on how the target files are specified on the command line.
>
> Initially I ran:
>   spatch --sp-file const.cocci --local-includes --in-place --dir .
> and now with your hints, I changed to
>   spatch --sp-file const.cocci --local-includes --in-place *.c
>
> That way headers are updated as expected and the warnings are gone.
>
> I can try to condense a small test case tomorrow, if you're interested/necessary.

--dir is the proper solution.  Since it is not working, I would be
interested in having a test case.  I guess it should be a directory with
multiple .c files.

When you put multiple files on the command line, you are treating them all
at once, ie they are all parsed and in memory at the same time.  This is
good if there are references between .c files that have to be taken into
account.  But obviously it doesn't scale.  On the other hand, .h files
should be treated at the same time as the .c files that reference them, so
it should work.

julia

[Cocci] remove redundant const qualifiers from function arguments

[Sören Brinkmann]

Hi Julia,

On Wed, 2016-05-11 at 11:51:43 +0200, Julia Lawall wrote:
> 
> 
> On Tue, 10 May 2016, S?ren Brinkmann wrote:
> 
> > On Wed, 2016-05-11 at 07:59:06 +0200, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> > >
> > > > Hi Julia,
> > > >
> > > > On Wed, 2016-05-11 at 07:41:20 +0200, Julia Lawall wrote:
> > > > >
> > > > >
> > > > > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I'm trying to remove 'const' qualifiers from function arguments that are
> > > > > > passed by value.
> > > > > >
> > > > > > e.g. I want to convert:
> > > > > >
> > > > > > void foobar(const int bar, const int *baz);
> > > > > > to
> > > > > > void foobar(int bar, const int *baz);
> > > > > >
> > > > > > I came up with this semantic patch:
> > > > > > /* Remove const from arguments passed by value */
> > > > > > @ rule1 @
> > > > > > identifier fi, I;
> > > > > > type T;
> > > > > > @@
> > > > > >  fi ( ...,
> > > > > > -	const T
> > > > > > +	T
> > > > > > 	I
> > > > > > 	,...
> > > > > >  )
> > > > > >  { ... }
> > > > > >
> > > > > > That seems to work fine on function declarations, but does not cover
> > > > > > function prototypes. Extending the patch to function prototypes I ran
> > > > > > into some trouble. As complete separate patch, I evolved the above to
> > > > > > (just replacing the function body with a ';'):
> > > > > > /* Remove const from arguments passed by value */
> > > > > > @ rule1 @
> > > > > > identifier fi, I;
> > > > > > type T;
> > > > > > @@
> > > > > >  fi ( ...,
> > > > > > -	const T
> > > > > > +	T
> > > > > > 	I
> > > > > > 	,...
> > > > > >  ) ;
> > > > > >
> > > > > > But that is not parsed by spatch, causing this error:
> > > > > >   init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> > > > > >   minus: parse error:
> > > > > >     File "const_proto.cocci", line 9, column 1, charpos = 115
> > > > > >     around = 'I',
> > > > > >     whole content = '     I'
> > > > > >
> > > > > > Also, using the first version of a patch on a c-file with matching function
> > > > > > declarations that also includes function prototypes, results in both,
> > > > > > declarations and prototypes, while using it on a header with prototypes
> > > > > > only doesn't result in any replacements.
> > > > > >
> > > > > > I attach the patches and some test files for reference.
> > > > > > const.cocci apparently correctly removes all const keywords from the
> > > > > > test.c file, but does not change the .h at all (which is correct, just
> > > > > > weird that it changes the prototype in the .c).
> > > > > >
> > > > > > The const_proto.cocci file causes the above-mentioned error.
> > > > > >
> > > > > > Does anybody know what I'm missing here?
> > > > >
> > > > > By default, when you change a function header, Coccinell changes the
> > > > > prototype accordingly.  But to be able to do that it needs to have access
> > > > > to the prototype.  So you may need to give an argument like --all-includes
> > > > > (include all the .h files mentioned in the .c file) or --recursive-incudes
> > > > > (include all the .h files recurively included by any .h file mentioned in
> > > > > the .c file).  --recursive-includes at least may be slow, but things will
> > > > > be done in a consistent manner.
> > > >
> > > > Interesting. I tried different variations of the include switches on the
> > > > real code base, but none resulted in the headers to be updated. Though,
> > > > with those switches I get 'different modification result for foo.h' warnings.
> > > >
> > > > For the test files, including the header in the c file and running
> > > > spatch with --local-includes works as expected.
> > > >
> > > > Guess I'll dig into the headers and check if there are any unusual
> > > > constructs causing the problem. Any hints would be appreciated :)
> > > >
> > > > > I will check on why the expliciti prototype rule doesn't parse.
> > > >
> > > > Thanks for looking into it.
> > >
> > > Could you send an example of a .c and .h file where the result is not as
> > > expected?
> >
> > It works now... It seems something is handled significantly different
> > depending on how the target files are specified on the command line.
> >
> > Initially I ran:
> >   spatch --sp-file const.cocci --local-includes --in-place --dir .
> > and now with your hints, I changed to
> >   spatch --sp-file const.cocci --local-includes --in-place *.c
> >
> > That way headers are updated as expected and the warnings are gone.
> >
> > I can try to condense a small test case tomorrow, if you're interested/necessary.
> 
> --dir is the proper solution.  Since it is not working, I would be
> interested in having a test case.  I guess it should be a directory with
> multiple .c files.
> 
> When you put multiple files on the command line, you are treating them all
> at once, ie they are all parsed and in memory at the same time.  This is
> good if there are references between .c files that have to be taken into
> account.  But obviously it doesn't scale.  On the other hand, .h files
> should be treated at the same time as the .c files that reference them, so
> it should work.

I think I have a test case that is minimal.
I attach the following files:
 - module_a.c  -- c source
 - module_a.h  -- c header providing prototypes for functions in module_a.c
 - module_b.c  -- c source including module_a.h
 - const.cocci  -- semantic patch using fundecl syntax
 - const_proto.cocci  -- semantic patch using funproto syntax

The patch with funproto syntax fails as described above and is also
redundant when headers are included in the transformation the other
patch is doing.

When I run:
  spatch --sp-file const.cocci --local-includes --in-place *.c
things work fine and both, module_a[ch], get updated as intended.

When I run
  spatch --sp-file const.cocci --local-includes --in-place --dir .
then only the c files are updated (in-place). But, the patch output
includes the correct header transformation (I was expecting headers to
be updated in-place too, just as in the first case).
Also, removing module_b.c from the test, makes spatch update both
module.[ch] in place.

	Thanks,
	S?ren

-------------- next part --------------
A non-text attachment was scrubbed...
Name: module_a.c
Type: text/x-csrc
Size: 73 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160511/92bfe6b1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: module_a.h
Type: text/x-chdr
Size: 96 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160511/92bfe6b1/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: module_b.c
Type: text/x-csrc
Size: 22 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160511/92bfe6b1/attachment-0002.bin>
-------------- next part --------------
/* Remove const from arguments passed by value */
@ rule1 @
identifier fi, I;
type T;
@@
 fi ( ...,
-	const T
+	T
	I
	,...
 )
 { ... }

-------------- next part --------------
/* Remove const from arguments passed by value */
@ rule1 @
identifier fi, I;
type T;
@@
 fi ( ...,
-	const T
+	T
	I
	,...
 ) ;

[Cocci] remove redundant const qualifiers from function arguments

[SF Markus Elfring]

> I think I have a test case that is minimal.

How do you think about to try a bit more fine-tuning out for the shown
Coccinelle scripts?

How much will it matter for your change pattern to specify only the
deletion of the qualifier "const" from a data type of a function
parameter while leaving the corresponding metavariable "T" untouched?

@removal@
identifier F, I;
type T;
@@
 F(...,
-  const
   T I,
  ...
 );


By the way: Do you care how often a "const" might appear within a data type?

Regards,
Markus

[Cocci] remove redundant const qualifiers from function arguments

[Sören Brinkmann]

Hi Markus,

On Thu, 2016-05-12 at 05:35:58 +0200, SF Markus Elfring wrote:
> > I think I have a test case that is minimal.
> 
> How do you think about to try a bit more fine-tuning out for the shown
> Coccinelle scripts?
>
> How much will it matter for your change pattern to specify only the
> deletion of the qualifier "const" from a data type of a function
> parameter while leaving the corresponding metavariable "T" untouched?

Well, is there anything wrong with my patches? I don't argue that they
could be optimized but as long as there is nothing wrong with them
things should work, shouldn't they?
Does your changed version behave correctly in all cases?

> 
> @removal@
> identifier F, I;
> type T;
> @@
>  F(...,
> -  const
>    T I,
>   ...
>  );

I guess that is a valid optimization. But does this go through the
parsing step? And even if, why doesn't mine?

> By the way: Do you care how often a "const" might appear within a data type?

No, I don't. My goal was to get rid of all const qualifiers that affect
local variables only. E.g.:
 - const int foo => int foo
 - const int *foo => const int *foo (data pointed to shall remain const)
 - const int * const foo => const int *foo

	S?ren

[Cocci] remove redundant const qualifiers from function arguments

[SF Markus Elfring]

> I guess that is a valid optimization. But does this go through the
> parsing step? And even if, why doesn't mine?

Would you like to try the following SmPL approach once more?

@prototype_adjustment@
identifier F, I;
type R, T;
@@
 R F(...,
-    const
     T I,
     ...
    );

Regards,
Markus

[Cocci] remove redundant const qualifiers from function arguments

[Julia Lawall]

> > @removal@
> > identifier F, I;
> > type T;
> > @@
> >  F(...,
> > -  const
> >    T I,
> >   ...
> >  );

Oops, now I see why this doesn't parse.  When you make a rule for a
prototype, you need to specify the return type (which could be just T1).
Otherwise, the parser can't tell the difference between this and a
function call.

julia

[Cocci] remove redundant const qualifiers from function arguments

[Sören Brinkmann]

On Thu, 2016-05-12 at 09:06:45 +0200, Julia Lawall wrote:
> > > @removal@
> > > identifier F, I;
> > > type T;
> > > @@
> > >  F(...,
> > > -  const
> > >    T I,
> > >   ...
> > >  );
> 
> Oops, now I see why this doesn't parse.  When you make a rule for a
> prototype, you need to specify the return type (which could be just T1).
> Otherwise, the parser can't tell the difference between this and a
> function call.

You're right that does it. Looks to me then that the grammar
documentation should not indicate the fn_ctype as optional for function
prototypes (http://coccinelle.lip6.fr/docs/main_grammar006.html)?

	Thanks,
	S?ren

[Cocci] remove redundant const qualifiers from function arguments

[Julia Lawall]

On Thu, 12 May 2016, S?ren Brinkmann wrote:

> On Thu, 2016-05-12 at 09:06:45 +0200, Julia Lawall wrote:
> > > > @removal@
> > > > identifier F, I;
> > > > type T;
> > > > @@
> > > >  F(...,
> > > > -  const
> > > >    T I,
> > > >   ...
> > > >  );
> >
> > Oops, now I see why this doesn't parse.  When you make a rule for a
> > prototype, you need to specify the return type (which could be just T1).
> > Otherwise, the parser can't tell the difference between this and a
> > function call.
>
> You're right that does it. Looks to me then that the grammar
> documentation should not indicate the fn_ctype as optional for function
> prototypes (http://coccinelle.lip6.fr/docs/main_grammar006.html)?

Indeed.  I'll fix that.  Thanks!

julia

[Cocci] remove redundant const qualifiers from function arguments

[Julia Lawall]

On Wed, 11 May 2016, S?ren Brinkmann wrote:

> Hi Julia,
>
> On Wed, 2016-05-11 at 11:51:43 +0200, Julia Lawall wrote:
> >
> >
> > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> >
> > > On Wed, 2016-05-11 at 07:59:06 +0200, Julia Lawall wrote:
> > > >
> > > >
> > > > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> > > >
> > > > > Hi Julia,
> > > > >
> > > > > On Wed, 2016-05-11 at 07:41:20 +0200, Julia Lawall wrote:
> > > > > >
> > > > > >
> > > > > > On Tue, 10 May 2016, S?ren Brinkmann wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I'm trying to remove 'const' qualifiers from function arguments that are
> > > > > > > passed by value.
> > > > > > >
> > > > > > > e.g. I want to convert:
> > > > > > >
> > > > > > > void foobar(const int bar, const int *baz);
> > > > > > > to
> > > > > > > void foobar(int bar, const int *baz);
> > > > > > >
> > > > > > > I came up with this semantic patch:
> > > > > > > /* Remove const from arguments passed by value */
> > > > > > > @ rule1 @
> > > > > > > identifier fi, I;
> > > > > > > type T;
> > > > > > > @@
> > > > > > >  fi ( ...,
> > > > > > > -	const T
> > > > > > > +	T
> > > > > > > 	I
> > > > > > > 	,...
> > > > > > >  )
> > > > > > >  { ... }
> > > > > > >
> > > > > > > That seems to work fine on function declarations, but does not cover
> > > > > > > function prototypes. Extending the patch to function prototypes I ran
> > > > > > > into some trouble. As complete separate patch, I evolved the above to
> > > > > > > (just replacing the function body with a ';'):
> > > > > > > /* Remove const from arguments passed by value */
> > > > > > > @ rule1 @
> > > > > > > identifier fi, I;
> > > > > > > type T;
> > > > > > > @@
> > > > > > >  fi ( ...,
> > > > > > > -	const T
> > > > > > > +	T
> > > > > > > 	I
> > > > > > > 	,...
> > > > > > >  ) ;
> > > > > > >
> > > > > > > But that is not parsed by spatch, causing this error:
> > > > > > >   init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> > > > > > >   minus: parse error:
> > > > > > >     File "const_proto.cocci", line 9, column 1, charpos = 115
> > > > > > >     around = 'I',
> > > > > > >     whole content = '     I'
> > > > > > >
> > > > > > > Also, using the first version of a patch on a c-file with matching function
> > > > > > > declarations that also includes function prototypes, results in both,
> > > > > > > declarations and prototypes, while using it on a header with prototypes
> > > > > > > only doesn't result in any replacements.
> > > > > > >
> > > > > > > I attach the patches and some test files for reference.
> > > > > > > const.cocci apparently correctly removes all const keywords from the
> > > > > > > test.c file, but does not change the .h at all (which is correct, just
> > > > > > > weird that it changes the prototype in the .c).
> > > > > > >
> > > > > > > The const_proto.cocci file causes the above-mentioned error.
> > > > > > >
> > > > > > > Does anybody know what I'm missing here?
> > > > > >
> > > > > > By default, when you change a function header, Coccinell changes the
> > > > > > prototype accordingly.  But to be able to do that it needs to have access
> > > > > > to the prototype.  So you may need to give an argument like --all-includes
> > > > > > (include all the .h files mentioned in the .c file) or --recursive-incudes
> > > > > > (include all the .h files recurively included by any .h file mentioned in
> > > > > > the .c file).  --recursive-includes at least may be slow, but things will
> > > > > > be done in a consistent manner.
> > > > >
> > > > > Interesting. I tried different variations of the include switches on the
> > > > > real code base, but none resulted in the headers to be updated. Though,
> > > > > with those switches I get 'different modification result for foo.h' warnings.
> > > > >
> > > > > For the test files, including the header in the c file and running
> > > > > spatch with --local-includes works as expected.
> > > > >
> > > > > Guess I'll dig into the headers and check if there are any unusual
> > > > > constructs causing the problem. Any hints would be appreciated :)
> > > > >
> > > > > > I will check on why the expliciti prototype rule doesn't parse.
> > > > >
> > > > > Thanks for looking into it.
> > > >
> > > > Could you send an example of a .c and .h file where the result is not as
> > > > expected?
> > >
> > > It works now... It seems something is handled significantly different
> > > depending on how the target files are specified on the command line.
> > >
> > > Initially I ran:
> > >   spatch --sp-file const.cocci --local-includes --in-place --dir .
> > > and now with your hints, I changed to
> > >   spatch --sp-file const.cocci --local-includes --in-place *.c
> > >
> > > That way headers are updated as expected and the warnings are gone.
> > >
> > > I can try to condense a small test case tomorrow, if you're interested/necessary.
> >
> > --dir is the proper solution.  Since it is not working, I would be
> > interested in having a test case.  I guess it should be a directory with
> > multiple .c files.
> >
> > When you put multiple files on the command line, you are treating them all
> > at once, ie they are all parsed and in memory at the same time.  This is
> > good if there are references between .c files that have to be taken into
> > account.  But obviously it doesn't scale.  On the other hand, .h files
> > should be treated at the same time as the .c files that reference them, so
> > it should work.
>
> I think I have a test case that is minimal.
> I attach the following files:
>  - module_a.c  -- c source
>  - module_a.h  -- c header providing prototypes for functions in module_a.c
>  - module_b.c  -- c source including module_a.h
>  - const.cocci  -- semantic patch using fundecl syntax
>  - const_proto.cocci  -- semantic patch using funproto syntax
>
> The patch with funproto syntax fails as described above and is also
> redundant when headers are included in the transformation the other
> patch is doing.
>
> When I run:
>   spatch --sp-file const.cocci --local-includes --in-place *.c
> things work fine and both, module_a[ch], get updated as intended.
>
> When I run
>   spatch --sp-file const.cocci --local-includes --in-place --dir .
> then only the c files are updated (in-place). But, the patch output
> includes the correct header transformation (I was expecting headers to
> be updated in-place too, just as in the first case).
> Also, removing module_b.c from the test, makes spatch update both
> module.[ch] in place.

Thanks for the example files.  When you run with --dir ., the output
gives:

different modification result for ./module_a.h

It detects that the resulting files are different when processed via the
different includes, so it doesn't want to overwrite the file with one
thing or the other.

julia

[Cocci] Deletion of indication for "optional" syntax elements?

[SF Markus Elfring]

>> Looks to me then that the grammar documentation should not indicate
>> the fn_ctype as optional for function prototypes
>> (http://coccinelle.lip6.fr/docs/main_grammar006.html)?
> Indeed.  I'll fix that.

Would you like to delete the specification "\opt" at a few places?
https://github.com/coccinelle/coccinelle/blob/df648d123ac4dad16a855f0c5f3adf42ce602b29/docs/manual/cocci_syntax.tex#L1173

Regards,
Markus
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160523/29deafa2/attachment.html>

[Cocci] Deletion of indication for "optional" syntax elements?

[Julia Lawall]

On Mon, 23 May 2016, SF Markus Elfring wrote:

>
> Looks to me then that the grammar documentation should not indicate
> the fn_ctype as optional for function prototypes
> (http://coccinelle.lip6.fr/docs/main_grammar006.html)?
>
> Indeed.  I'll fix that.
>
>
> Would you like to delete the specification "\opt" at a few places?
> https://github.com/coccinelle/coccinelle/blob/df648d123ac4dad16a855f0c5f3adf42ce602b29/docs/manual/cocci_syntax.tex#L1173

At what place do you have in mind?  The return type of a functin
definition really is optional.  Other than that, I don't kow what is being
referred to, and don't have time to check every instance.

julia

[Cocci] Deletion of indication for "optional" syntax elements?

[SF Markus Elfring]

>> Would you like to delete the specification "\opt" at a few places?
>> https://github.com/coccinelle/coccinelle/blob/df648d123ac4dad16a855f0c5f3adf42ce602b29/docs/manual/cocci_syntax.tex#L1173
> At what place do you have in mind?  The return type of a functin
> definition really is optional.

Will it be sufficient to make the function return type a mandatory
property for the syntax item "funproto"?

Regards,
Markus

[Cocci] Deletion of indication for "optional" syntax elements?

[SF Markus Elfring]

> At what place do you have in mind?

Was an update for one place enough?

more correct about requirements on function prototypes
https://github.com/coccinelle/coccinelle/commit/51a2709600bc7e0051cabbebe0005ee61e235600

Regards,
Markus