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
Received on 2010-09-27 00:27:47 CEST