On Monday 26 April 2010 05:11:25 pm Stefan Fuhrmann wrote:
> > I saw in a later part of the patch that this is so you can make the
> > function 'decode_instruction()' rely on those values. As you know,
> > adding the explicit "=0", "=1", "=2" on the enum definition does not
> > make any difference to the compiler, it just provides a hint to the
> > human developers that these values might be significant.
> >
> > We need a stronger hint than this. Please add code comments in both
> > the enum definition and the function 'decode_instruction()', saying
> > "The enum values must match the instruction encoding values."
>
> Oh! I wasn't aware of that - being a C++ guy.
> But as long as comments can "fix" that, so be it ... ;)
For such things, I find it useful to have some compile-time checks. Since
the preprocessor cannot compare enumerated values or some other values
known only at the compilation time, I use something like:
#define compile_assert(x) extern int __hack__compile_assert[(x) ? 1 : -1]
Then, you could add such checks (with comments explaining why this values
are important) to one of the source files:
compile_assert(svn_txdelta_source == 0);
compile_assert(svn_txdelta_target == 1);
compile_assert(svn_txdelta_new == 2);
Disclaimer: I am not a Subversion developer, so I am not sure if
Subversion community is willing to have such assertions in the code.
Regards,
Alexey.
Received on 2010-04-27 02:49:39 CEST