Jonathan Gilbert wrote:
>
> I've figured out how to put it into svn_private_config.h.
Good. Note that there are two places: "configure.in" for autoconf systems and
build/generator/ for MS Windows/MSVC.
> I have a proposed change to the definition of the macro. Basically, what's
> happening as I add it into all the places that need it is stuff like this:
>
> svn_prop_kind_t kind = svn_property_kind (NULL, name);
>
> SVN_UNUSED_PARAMETER (pool);
>
> if (kind != svn_prop_regular_kind)
> return svn_error_createf (...);
Yes, that's rather ugly.
> It has to go *after* the local variable declarations because it is a
> statement, which often has the effect of splitting up code. What I was
> thinking would be to allow:
>
> SVN_UNUSED_PARAMETER (pool);
>
> svn_prop_kind_t kind = svn_property_kind (NULL, name);
> if (kind != svn_prop_regular_kind)
> return svn_error_createf
>
> ..by turning the macro into something like:
>
> #define SVN_UNUSED_PARAMETER(x) void *svn_unused_parameter__##x = &x
Yes, that's much nicer, except...
> The compiler would then see a declaration. Now, the compiler might also be
> emitting warnings for unused local variables. However, at least when GCC is
> being used, there is an attribute which can be applied to it which will
> stop it from producing that warning: __attribute__((__unused__)). In
... that's a problem. On other compilers, we're now replacing an unused
parameter with an unused local variable, which doesn't seem like any
improvement to me. On GCC, we could have solved the problem a better way, by
marking the parameter itself "unused".
> addition, if the __deprecated__ attribute is applied to the parameter in
> the function's signature, then the compiler will emit a warning whenever
> code actually does use the parameter. (Deprecation isn't a perfect match to
> the meaning we want, but it's the only attribute I saw which would result
> in a warning.) In order to do this, another macro would also be needed,
> perhaps:
>
> #define SVN_UNUSED_PARAMETER_NAME(x) x __attribute__((__deprecated__))
No, that's silly. Mark the parameter "unused" instead, and make the other
macro evaluate to nothing.
> it occurs to me that the __unused__ attribute could probably also be
> applied directly here, but of course only GCC will support this, so
> SVN_UNUSED_PARAMETER would still be needed.
>
> Another thing that SVN_UNUSED_PARAMETER_NAME could do is simply leave the
> name off for C++ code:
>
> #ifdef __cplusplus
> # define SVN_UNUSED_PARAMETER_NAME(x)
> #elif defined __GCC__
> # define SVN_UNUSED_PARAMETER_NAME(x) x __attribute__((__deprecated__))
> #else
> # define SVN_UNUSED_PARAMETER_NAME(x) x
> #endif
Yes, certainly, if our code is compilable under C++.
> It would mean even more expansion in the size of the code, but it would
I don't know what you mean. What's "it", and what size?
> also mean that we would be able to detect instances where parameters that
> were marked as unused were in fact being used. I'm not sure how important
> this is.
Not very. The whole thing isn't very important. If there's a neat solution,
then it's worth doing something, but it's not worth implementing a complex or
ugly solution.
> Usage would look like this:
>
> static svn_error_t *
> validate_prop (const char *name,
> apr_pool_t * SVN_UNUSED_PARAMETER_NAME (pool))
> {
> SVN_UNUSED_PARAMETER (pool);
And that - having to mark the parameter twice - is ugly. I don't want to see that.
I think we have a choice among three options:
1) Mark the parameter where it appears in the parameter list. This is the best
way for compilers that support it. We know GCC supports it; can anyone tell us
about others?
2) Mark the parameter in the function body. Compatible with all compilers, but
tends to just replace one warning with another: "statement with no effect" for
the version that goes after declarations; "unused local variable" for the
version that goes before, except on compilers where (1) is possible.
3) Disable "unused parameter" warnings. (GCC: disabled by default;
"-Wno-unused-parameter" if required. MSVC: maybe "#pragma
warning(disable:NNNN)" ?.) This is what we (at least I) do now. The
disadvantage is that we can't get the benefit of such warnings in functions
where we did intend to use the parameters. However, in practice I don't think
we actually gain very much, as we use other methods to ensure our functions
work correctly (review and testing).
Having seen your latest analysis above and written this response, it now looks
to me like (1) and (3) are both better than (2).
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 27 12:29:34 2005