You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> on 2008/08/25 02:09:04 UTC
[PATCH] Mark deprecated functions with SVN_DEPRECATED
The attached patch marks deprecated functions with SVN_DEPRECATED.
When such functions are used, compiler will print appropriate warnings.
This idea has been recently discussed here:
http://svn.haxx.se/dev/archive-2008-08/0508.shtml
[[[
Mark deprecated functions with SVN_DEPRECATED.
* subversion/include/svn_types.h: Define SVN_DEPRECATED.
* subversion/include/svn_auth.h:
* subversion/include/svn_base64.h:
* subversion/include/svn_client.h:
* subversion/include/svn_cmdline.h:
* subversion/include/svn_config.h:
* subversion/include/svn_delta.h:
* subversion/include/svn_diff.h:
* subversion/include/svn_dso.h:
* subversion/include/svn_error.h:
* subversion/include/svn_fs.h:
* subversion/include/svn_hash.h:
* subversion/include/svn_io.h:
* subversion/include/svn_opt.h:
* subversion/include/svn_ra.h:
* subversion/include/svn_ra_svn.h:
* subversion/include/svn_repos.h:
* subversion/include/svn_subst.h:
* subversion/include/svn_utf.h:
* subversion/include/svn_wc.h: Mark deprecated functions with SVN_DEPRECATED.
]]]
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-08-26 15:59:34 Julian Foad napisał(a):
> On Tue, 2008-08-26 at 10:36 +0100, Julian Foad wrote:
> > Arfrever Frehtes Taifersar Arahesis wrote:
> > > [[[
> > > Mark deprecated functions with SVN_DEPRECATED.
> > >
> > > * subversion/include/svn_types.h: Define SVN_DEPRECATED.
> > >
> > > * subversion/bindings/swig/include/svn_global.swg: Define __declspec(x).
> >
> > Can you comment on why this is necessary? It doesn't look like the
> > "__declspec" would be seen by SWIG, given that in "svn_types.h" you have
> > "#if !defined(SWIG..." which tries to hide it from SWIG.
>
> Arfrever's comments from IRC:
>
> <Arfrever> julianf: Re defining __declspec(x) in
> subversion/bindings/swig/include/svn_global.swg: svn_types.h only checks
> 'SWIGPERL', 'SWIGPYTHON' and 'SWIGRUBY', but SWIG parses headers with
> only 'SWIG' defined.
>
> <julianf> OK, I think I understand what you mean. Thanks. That's OK
> then.
>
> <Arfrever> SWIG places SWIGPERL, SWIGPYTHON or SWIGRUBY definitions
> in .c files generated by SWIG.
>
> <Arfrever> 'SWIG' alone isn't defined in such files.
>
> <Arfrever> svn_types.h checks for SWIGPERL, SWIGPYTHON and SWIGRUBY to
> avoid compiler warnings during compilation of SWIG-based bindings which
> wrap some deprecated functions.
I said truth, but it's irrelevant here.
The change in subversion/bindings/swig/include/svn_global.swg is unnecessary
(but harmless) because SVN_DEPRECATED is defined as __declspec(deprecated)
only when _MSC_VER is defined and >= 1300, but SWIG doesn't define it during
parsing headers.
I will revert this change.
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Julian Foad <ju...@btopenworld.com>.
On Tue, 2008-08-26 at 10:36 +0100, Julian Foad wrote:
> Arfrever Frehtes Taifersar Arahesis wrote:
> > [[[
> > Mark deprecated functions with SVN_DEPRECATED.
> >
> > * subversion/include/svn_types.h: Define SVN_DEPRECATED.
> >
> > * subversion/bindings/swig/include/svn_global.swg: Define __declspec(x).
>
> Can you comment on why this is necessary? It doesn't look like the
> "__declspec" would be seen by SWIG, given that in "svn_types.h" you have
> "#if !defined(SWIG..." which tries to hide it from SWIG.
Arfrever's comments from IRC:
<Arfrever> julianf: Re defining __declspec(x) in
subversion/bindings/swig/include/svn_global.swg: svn_types.h only checks
'SWIGPERL', 'SWIGPYTHON' and 'SWIGRUBY', but SWIG parses headers with
only 'SWIG' defined.
<julianf> OK, I think I understand what you mean. Thanks. That's OK
then.
<Arfrever> SWIG places SWIGPERL, SWIGPYTHON or SWIGRUBY definitions
in .c files generated by SWIG.
<Arfrever> 'SWIG' alone isn't defined in such files.
<Arfrever> svn_types.h checks for SWIGPERL, SWIGPYTHON and SWIGRUBY to
avoid compiler warnings during compilation of SWIG-based bindings which
wrap some deprecated functions.
> > * subversion/include/svn_auth.h:
> > * subversion/include/svn_base64.h:
> > * subversion/include/svn_client.h:
> > * subversion/include/svn_cmdline.h:
> > * subversion/include/svn_config.h:
> > * subversion/include/svn_delta.h:
> > * subversion/include/svn_diff.h:
> > * subversion/include/svn_dso.h:
> > * subversion/include/svn_error.h:
> > * subversion/include/svn_fs.h:
> > * subversion/include/svn_hash.h:
> > * subversion/include/svn_io.h:
> > * subversion/include/svn_opt.h:
> > * subversion/include/svn_ra.h:
> > * subversion/include/svn_ra_svn.h:
> > * subversion/include/svn_repos.h:
> > * subversion/include/svn_subst.h:
> > * subversion/include/svn_utf.h:
> > * subversion/include/svn_wc.h: Mark deprecated functions with SVN_DEPRECATED.
>
> It's probably not such a good idea to combine adding "SVN_DEPRECATED"
> with moving the return type to a separate line in the same change. That
> will make it harder to merge changes between branches. Can you separate
> these two changes please?
>
> > * www/hacking.html
> > (deprecation): Update example.
> > ]]]
>
> Other than that, it looks good. I tried it (with GCC 4.1.2) and confirm
> it does what it should.
>
> Unfortunately, GCC 4.1.2 emits a warning for using a deprecated function
> even where this use is in the body of another deprecated function, which
> is not what I want, but that's another matter.
>
> - Julian
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Julian Foad <ju...@btopenworld.com>.
Arfrever Frehtes Taifersar Arahesis wrote:
> [[[
> Mark deprecated functions with SVN_DEPRECATED.
>
> * subversion/include/svn_types.h: Define SVN_DEPRECATED.
>
> * subversion/bindings/swig/include/svn_global.swg: Define __declspec(x).
Can you comment on why this is necessary? It doesn't look like the
"__declspec" would be seen by SWIG, given that in "svn_types.h" you have
"#if !defined(SWIG..." which tries to hide it from SWIG.
> * subversion/include/svn_auth.h:
> * subversion/include/svn_base64.h:
> * subversion/include/svn_client.h:
> * subversion/include/svn_cmdline.h:
> * subversion/include/svn_config.h:
> * subversion/include/svn_delta.h:
> * subversion/include/svn_diff.h:
> * subversion/include/svn_dso.h:
> * subversion/include/svn_error.h:
> * subversion/include/svn_fs.h:
> * subversion/include/svn_hash.h:
> * subversion/include/svn_io.h:
> * subversion/include/svn_opt.h:
> * subversion/include/svn_ra.h:
> * subversion/include/svn_ra_svn.h:
> * subversion/include/svn_repos.h:
> * subversion/include/svn_subst.h:
> * subversion/include/svn_utf.h:
> * subversion/include/svn_wc.h: Mark deprecated functions with SVN_DEPRECATED.
It's probably not such a good idea to combine adding "SVN_DEPRECATED"
with moving the return type to a separate line in the same change. That
will make it harder to merge changes between branches. Can you separate
these two changes please?
> * www/hacking.html
> (deprecation): Update example.
> ]]]
Other than that, it looks good. I tried it (with GCC 4.1.2) and confirm
it does what it should.
Unfortunately, GCC 4.1.2 emits a warning for using a deprecated function
even where this use is in the body of another deprecated function, which
is not what I want, but that's another matter.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
Here's the updated patch.
sed -i -e '/.\+SVN_DEPRECATED$/iSVN_DEPRECATED' -e '/.\+SVN_DEPRECATED$/s:[[:space:]]*SVN_DEPRECATED::' was very helpful :).
[[[
Mark deprecated functions with SVN_DEPRECATED.
* subversion/include/svn_types.h: Define SVN_DEPRECATED.
* subversion/bindings/swig/include/svn_global.swg: Define __declspec(x).
* subversion/include/svn_auth.h:
* subversion/include/svn_base64.h:
* subversion/include/svn_client.h:
* subversion/include/svn_cmdline.h:
* subversion/include/svn_config.h:
* subversion/include/svn_delta.h:
* subversion/include/svn_diff.h:
* subversion/include/svn_dso.h:
* subversion/include/svn_error.h:
* subversion/include/svn_fs.h:
* subversion/include/svn_hash.h:
* subversion/include/svn_io.h:
* subversion/include/svn_opt.h:
* subversion/include/svn_ra.h:
* subversion/include/svn_ra_svn.h:
* subversion/include/svn_repos.h:
* subversion/include/svn_subst.h:
* subversion/include/svn_utf.h:
* subversion/include/svn_wc.h: Mark deprecated functions with SVN_DEPRECATED.
* www/hacking.html
(deprecation): Update example.
]]]
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
Committed in r32772.
Some very old versions of GCC probably don't support __attribute__((deprecated)),
so I added the check for GCC version:
...
#if defined(__GNUC__) && (__GNUC__ >= 4 || (__GNUC__==3 && __GNUC_MINOR__>=1))
...
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Julian Foad <ju...@btopenworld.com>.
Arfrever Frehtes Taifersar Arahesis wrote:
> I'm attaching the updated patch which should address all issues.
> Code formatting in some headers was fixed in r32726.
> If nobody objects, I will commit this patch tomorrow.
+1.
- Julian
> [[[
> Mark deprecated functions with SVN_DEPRECATED.
>
> * subversion/include/svn_types.h: Define SVN_DEPRECATED.
>
> * subversion/bindings/swig/include/svn_global.swg: Define __declspec(x).
>
> * subversion/include/svn_auth.h:
> * subversion/include/svn_base64.h:
> * subversion/include/svn_client.h:
> * subversion/include/svn_cmdline.h:
> * subversion/include/svn_config.h:
> * subversion/include/svn_delta.h:
> * subversion/include/svn_diff.h:
> * subversion/include/svn_dso.h:
> * subversion/include/svn_error.h:
> * subversion/include/svn_fs.h:
> * subversion/include/svn_hash.h:
> * subversion/include/svn_io.h:
> * subversion/include/svn_opt.h:
> * subversion/include/svn_ra.h:
> * subversion/include/svn_ra_svn.h:
> * subversion/include/svn_repos.h:
> * subversion/include/svn_subst.h:
> * subversion/include/svn_utf.h:
> * subversion/include/svn_wc.h: Mark deprecated functions with SVN_DEPRECATED.
>
> * www/hacking.html
> (deprecation): Update example.
>
> Review by: blair
> julianfoad
> ]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
I'm attaching the updated patch which should address all issues.
Code formatting in some headers was fixed in r32726.
If nobody objects, I will commit this patch tomorrow.
[[[
Mark deprecated functions with SVN_DEPRECATED.
* subversion/include/svn_types.h: Define SVN_DEPRECATED.
* subversion/bindings/swig/include/svn_global.swg: Define __declspec(x).
* subversion/include/svn_auth.h:
* subversion/include/svn_base64.h:
* subversion/include/svn_client.h:
* subversion/include/svn_cmdline.h:
* subversion/include/svn_config.h:
* subversion/include/svn_delta.h:
* subversion/include/svn_diff.h:
* subversion/include/svn_dso.h:
* subversion/include/svn_error.h:
* subversion/include/svn_fs.h:
* subversion/include/svn_hash.h:
* subversion/include/svn_io.h:
* subversion/include/svn_opt.h:
* subversion/include/svn_ra.h:
* subversion/include/svn_ra_svn.h:
* subversion/include/svn_repos.h:
* subversion/include/svn_subst.h:
* subversion/include/svn_utf.h:
* subversion/include/svn_wc.h: Mark deprecated functions with SVN_DEPRECATED.
* www/hacking.html
(deprecation): Update example.
Review by: blair
julianfoad
]]]
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Blair Zajac wrote:
> Arfrever Frehtes Taifersar Arahesis wrote:
>> 2008-08-25 08:35:33 Blair Zajac napisał(a):
>>> Arfrever Frehtes Taifersar Arahesis wrote:
>>>> The attached patch marks deprecated functions with SVN_DEPRECATED.
>>>> When such functions are used, compiler will print appropriate warnings.
>>> Hi Arfrever,
>>>
>>> Thanks for putting this patch together.
>>>
>>> However, the location of SVN_DEPRECATED is odd, as though it's part
>>> of the pointer return type:
>>>
>>> svn_error_t * SVN_DEPRECATED
>>> svn_opt_print_help2(apr_getopt_t *os,
>>> const char *pgm_name,
>>> svn_boolean_t print_version,
>>>
>>> Can we move it before the return type of the function?
>>
>> Do you want SVN_DEPRECATED on separate lines?
>> I.e. do you prefer:
>>
>> SVN_DEPRECATED
>> svn_error_t *
>> svn_function(args);
>>
>> ... or ...
>>
>> SVN_DEPRECATED svn_error_t *
>> svn_function(args);
>
> I like the first one a little better, since we always list the return
> type at the start of the line.
Agreed. It would also lessen the code churn in instances where we don't yet
have the return type and symbol name on different lines (which is actually quite
common, lamentably).
-Hyrum
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Blair Zajac <bl...@orcaware.com>.
Arfrever Frehtes Taifersar Arahesis wrote:
> 2008-08-25 08:35:33 Blair Zajac napisał(a):
>> Arfrever Frehtes Taifersar Arahesis wrote:
>>> The attached patch marks deprecated functions with SVN_DEPRECATED.
>>> When such functions are used, compiler will print appropriate warnings.
>> Hi Arfrever,
>>
>> Thanks for putting this patch together.
>>
>> However, the location of SVN_DEPRECATED is odd, as though it's part of the
>> pointer return type:
>>
>> svn_error_t * SVN_DEPRECATED
>> svn_opt_print_help2(apr_getopt_t *os,
>> const char *pgm_name,
>> svn_boolean_t print_version,
>>
>> Can we move it before the return type of the function?
>
> Do you want SVN_DEPRECATED on separate lines?
> I.e. do you prefer:
>
> SVN_DEPRECATED
> svn_error_t *
> svn_function(args);
>
> ... or ...
>
> SVN_DEPRECATED svn_error_t *
> svn_function(args);
I like the first one a little better, since we always list the return type at
the start of the line.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-08-25 08:35:33 Blair Zajac napisał(a):
> Arfrever Frehtes Taifersar Arahesis wrote:
> > The attached patch marks deprecated functions with SVN_DEPRECATED.
> > When such functions are used, compiler will print appropriate warnings.
>
> Hi Arfrever,
>
> Thanks for putting this patch together.
>
> However, the location of SVN_DEPRECATED is odd, as though it's part of the
> pointer return type:
>
> svn_error_t * SVN_DEPRECATED
> svn_opt_print_help2(apr_getopt_t *os,
> const char *pgm_name,
> svn_boolean_t print_version,
>
> Can we move it before the return type of the function?
Do you want SVN_DEPRECATED on separate lines?
I.e. do you prefer:
SVN_DEPRECATED
svn_error_t *
svn_function(args);
... or ...
SVN_DEPRECATED svn_error_t *
svn_function(args);
?
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Mark deprecated functions with SVN_DEPRECATED
Posted by Blair Zajac <bl...@orcaware.com>.
Arfrever Frehtes Taifersar Arahesis wrote:
> The attached patch marks deprecated functions with SVN_DEPRECATED.
> When such functions are used, compiler will print appropriate warnings.
Hi Arfrever,
Thanks for putting this patch together.
However, the location of SVN_DEPRECATED is odd, as though it's part of the
pointer return type:
svn_error_t * SVN_DEPRECATED
svn_opt_print_help2(apr_getopt_t *os,
const char *pgm_name,
svn_boolean_t print_version,
Can we move it before the return type of the function?
Thanks,
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org