On Sun, 26 Sep 2010, Greg Stein wrote:
First off, if you could somehow extract the patch for fixing the
comments' @deprecated statements, that would be awesome. Fixing those
is a no-brainer and introduces no extra complexity.
Done, I sent that out in a separate email.
Wrapping declarations with #if SVN_TARGET_API is a non-starter, I
believe. That sounds like it would obfuscate the headers a bit too
much.
In some senses, I'd say yes-- it does make the headers more verbose,
certainly. But I also feel there are cases where it actually clarifies
things a bit, like when structs get extended; IMHO seeing
struct svn_some_type {
int x;
int y;
#if SVN_TARGET_API >= 2
int x2;
int y2;
#endif
#if SVN_TARGET_API >= 5
void *baton;
#endif
}
makes it very clear which parts of the struct were introduced in which
versions. Yes, the information is duplicated in the Doxygen comments,
but I feel this draws the eye more easily.
That being said, it is a half-megabyte patch, and even given the
overhead of unified diff format, that's a whole bunch of extra lines of
code.
In your original email, you mentioned something about "gcc
poison" which made it sound like you could flag certain declarations
to gcc as "bad to use". I just looked up that poison stuff, and I
don't see how we can magically work that into the SVN_DEPRECATED
macros that occur in each declaration (since you have to list the
function name in there). The closest that I could see would be
something like:
SVN_DEPRECATED_5
svn_error_t *
svn_wc_some_function(arguments,
more argments);
SVN_MAYBE_POISON_5(svn_wc_some_function);
Where the latter macro expands to: _Pragma("GCC poison
svn_wc_some_function"); (I think that is the right syntax)
But again, that will add even more cruft around the declarations which
I'm not sure will be acceptable (gotta see what people think).
I thought about that, and that's pretty much what SVN_POISON does for
GCC; however, I believe that if you poison an identifier, it's best not
to have it appear anywhere in the included source. Also, the way I
implemented the SVN_POISON() macro uses a different semantic for non-GCC
compilers; it generates a function prototype that looks like this:
svn_api_mismatch *svn_wc_some_function(svn_api_mismatch *);
Where "svn_api_mismatch" is an opaque struct defined at the same time as
SVN_POISON. That makes it so that any calls to the function will give
argument type warnings and make some amount of sense.
It is also possible to simply deprecate the "future" APIs rather than
declare them poisoned, although it means adding an SVN_DEPRECATED_? to
the true API. Or maybe we could add SVN_CURRENT (or somesuch) to the
current APIs, which become deprecated if you target an older API.
I think this may in fact end up being the best idea; I'd actually
suggest a slightly different format for the SVN_DEPRECATED_? macros than
I originally used. Rather than noting the API version in which a
function was deprecated, instead note the versions in which it was
active, like the following:
SVN_API_SINCE_5
svn_error_t *
svn_client_update3([...]);
SVN_API_SINCE_2
SVN_API_UNTIL_4
svn_error_t *
svn_client_update2([...]);
SVN_API_UNTIL_1
svn_error_t *
svn_client_update([...]);
This has the benefit of being directly in line with the @since and
@deprecated tags in the Doxygen comments.
Anyway, that's probably more than enough for this email, I think. Is it
any surprise that I don't have any issue with verbose header files? :)
-Dani Church
