On Thu, 2010-11-04 at 11:51 +0000, Julian Foad wrote: > On Thu, 2010-11-04, Stefan Fuhrmann wrote: > > Hi there, > > > > after stumbling twice over this issue, I ran grep > > and found that the current usage of svn_tristate_t > > does not depend on the actual numerical values > > used for its states. > > > > Therefore, I propose to define svn_tristate_false > > equal to FALSE and svn_tristate_true equal to > > TRUE. That way, we can compare booleans with > > tristates and assign booleans to tristates. Without > > that, we need to write code like > > > > if ((get_some_bool() ? svn_tristate_true : svn_tristate_false) == > > get_a_tristate()) > > ... > > > > Also, the following will compile without warning but > > requires the patch to do what was intended: > > > > // FALSE becomes "unknown", TRUE becomes "false" > > svn_tristate_t my_tristate = get_some_bool(); > > > > -- Stefan^2. > > > > [[[ > > Make svn_tristate_t mainly compatible to svn_boolean_t > > by making its numerical values match the ones used for > > svn_boolean_t. > > > > * subversion/include/svn_types.h > > (svn_tristate_t): define *_true and *_false in terms of > > the svn_boolean_t values TRUE and FALSE, respectively. > > ]]] > > It seems sensible to have the set of values either match where they > overlap (like you're suggesting), or be totally incompatible (e.g. the > set of tristate values being 2/3/4, or the tristate data type being > incompatible with "int"). > > If the values match, then there is no need for the enumerators > "svn_tristate_false" and "svn_tristate_true", since "FALSE" and "TRUE" > can be used instead. If the idea is that these values are compatible, > then we *should* delete the "svn_tristate_true/false" names and use the > existing names for them, and not have the ambiguity of which name to > use. > > As Bert said, it's not always safe to assign or compare what we call a > "Boolean" value directly with an svn_tristate_t value, because we can't > always guarantee that "true" is represented by "1". But often it would > be safe. :-/
Having said all that, +1 on removing the gratuitous inconsistency by applying this patch. Committed r1030909. Thanks. - Julian
