Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

[Julian Foad <ju...@btopenworld.com>]

Garrett Rooney wrote:
> Philip Martin wrote:
>> We already have functions, e.g. svn_path_remove_components, where it
>> doesn't make make sense to pass a -ve value (they don't document it
>> either) so perhaps just say "-ve values are invalid".
> 
> Why only document it when we can force the issue by declaring the 
> argument unsigned?  It seems awfully close to saying "we're never going 
> to modify this argument" instead of just making it const.

There's a big difference: 'const' is enforced whereas 'unsigned' is not.

There is a strong case for avoiding 'unsigned' integer types altogether in C programming (except for low-level bit manipulations etc.).  This has mainly to do with the fact that they hardly protect you at all from such errors because of automatic (silent) conversions from signed to unsigned, and especially because unsigned has priority when a binary operator takes one of each.

void blah(unsigned Revision);
...
int Offset = -2;
blah(Offset);  /* Generally no error or warning. (you can make some compilers warn) */
...
int Offset = ...;  /* number of revisions after (+ve) or before (-ve) Head */
unsigned Revision = HeadRevision();
if (Revision + Offset < 0) error();
/* Oops: will never error, because type of (Revision+Offset) is unsigned. */

The Java language takes this to a logical conclusion and has no unsigned types.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Use of 'unsigned' in interfaces [was: Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data]

[Julian Foad <ju...@btopenworld.com>]

OK, I don't want to force the issue, given that we already have 'unsigned' in other APIs, but to continue the thread...

Greg Hudson wrote:
> On Fri, 2004-10-01 at 13:23, Julian Foad wrote:
> 
>>There's a big difference: 'const' is enforced whereas 'unsigned' is not.
> 
> gcc warns about the second of your examples with -W, and I think MSVC
> warns about the first (uncasted signed -> unsigned conversion), and
> modern gcc may have a warning option for that as well.

Yes, indeed they do or can.  In my first example, I mentioned in a comment that they can; I didn't mention it for the second example, which I suppose was a bit misleading.

It's not clear whether you are just picking me up on my sloppiness or whether you are implying that I'm wrong in asserting that 'unsigned' is far less useful than 'const'.  I maintain that the existence of such warnings (only when enabled or when using certain compilers) provides a distinctly lower class of safety than the errors caused by attempting to violate 'const'.  (There is another big difference in that 'const' errors apply only to the user of the 'const' pointer, whereas 'unsigned' mismatch warnings apply to both the user and the provider of the value, but I don't think that is very relevant to the discussion.)

The book "Large-Scale C++ Software Design" by John Lakos is _very_ good on both large-scale design issues and small-scale issues, many of which apply to C as well as C++, and he gives level-headed advice on appropriate use of 'unsigned' and 'short' especially in interfaces but also in implementation code.  His points are, briefly:

  "Guideline: Avoid using 'unsigned' in the interface; use 'int' instead."
  "Principle: Occasionally comments work better than trying to express an interface decision directly in the code (e.g., 'unsigned')."
  Use of 'unsigned' in the interface:
  + Prevents negative numbers from being passed?  "No. C++ allows the bit pattern to be reinterpreted silently."  (This is true for C as well.  We know that we might get a warning.)
  + Allows for the possibility for checking for negative values?  Only if you cast back to 'int'.
  + Runtime efficiency?  No difference.
  + Increase the range of positive values?  Yes, by 1 bit - rarely useful.  Risk of loss by conversion back to 'int'.
  + Likelihood of user error?  Increased, by accidental mixing with negative signed ints.
  + Encapsulation?  He says it "effectively limits the values that any implementation will accommodate, thereby reducing encapsulation".  I don't follow that.
  + (Two C++ points about it interfering with function overloading and template instantiation.)

Not trying to argue for the sake of it, just trying to advocate goodness.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

[Greg Hudson <gh...@MIT.EDU>]

On Fri, 2004-10-01 at 13:23, Julian Foad wrote:
> There's a big difference: 'const' is enforced whereas 'unsigned' is not.

gcc warns about the second of your examples with -W, and I think MSVC
warns about the first (uncasted signed -> unsigned conversion), and
modern gcc may have a warning option for that as well.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org