Will we get rid of the 32 bit flag variables?

[Pete Mack]

various people write:

** tests then become (bitmap[FLAG/32] & (1<<(FLAG%32))), which a competent optimizing compiler should handle as efficiently as the current code.

I don't understand what you want to do with this. It (or something vaguely like it) will work to store flag values, but it wont let you do ordinary bit-vector math.

In particular, what happens if you want to match more than one flag variable at a time? You may want to do things logically like:

flags & (RCONF | RCHAOS) /* Example from NPP */

You can't do this if the high order bits and low order bits are dependent the way they would be in your example.

You just can't OR together array indexes to get valid array indexes. They are ordinal numbers, not orthogonal bits.

[Bandobras]

The original example wasn't mine, but I couldn't ignore that one:

  flags & (RCONF | RCHAOS)   /* Example from NPP */

This is one of the many bad things induced by bit arithmetic. Such coding is very error prone, especally when you extend your variant. If the 'flags' bitfields is not the one variable (of a few) that holds the RCONF and RCHAOS bits, you have an error that people may not notice for years. If the RCONF and RCHAOS bits land at opposite sides of a bitfiled boundary you can get another error and you cannot write the code in this way at all, so you get irregular code.

This should be written

object_has_flag(o_ptr, F_CONF) && object_has_flag(o_ptr, F_CHAOS)

and you are safe against all errors I mentioned and then some.

Edit: corrected my code

Edit2: the F_CONF, etc. flags can be the same constant for object flags, monster flags, character flags, etc. because there is plenty of space in a bit vector, so no problem with unused bits. The macros can catch inappropriate flags, though, to point out errors, or they can translate the flags to save space in the bit vector, when really needed.

Edit3: actually, bit vectors are not necessary for the high-level approach I proposed above, bietfields would do, with much messier macros, but even the following mid-level code is quite safe:

check_flag(vecor, F_CONF) && check_flag(vector, F_CHAOS)

[zaimoni]

I don't understand what you want to do with this. It (or something vaguely like it) will work to store flag values, but it wont let you do ordinary bit-vector math.

I didn't say this was a good idea, merely that it was possible. The experiment of constructing the patch is worthwhile, even though I cannot do a great job because I'm prejudiced against it.

In particular, what happens if you want to match more than one flag variable at a time? You may want to do things logically like:

flags & (RCONF | RCHAOS) /* Example from NPP */

You can't do this if the high order bits and low order bits are dependent the way they would be in your example.

In C, you're hung with a high-level interface. Thus, use macro-wrappers so you can do this.

If I was making the final decision for V (I'm not, I merely am willing to provide patching as a code experiment), based on what I'm seeing in prototype I'd stop with:
* sanity-checking the flags for low-level dependencies with assert() [V probably needs this regardless]
* make a policy decision on whether converting to bitvector arrays is worth future-proofing, and C_COPY/C_WIPE applicability.
* making a policy decision on whether the trivial syntax change for newbie bit test/set/update is worth supporting two macro sets. If it is, make a policy decision on whether to use enum or #define for the flag index version. C dictates going from flag index to bitmap. See angband-dev for what the #define changes will look like for object flags in the enum case.
* Design requirement: leave in the bitvector math capability on array elements, as it's moronic to ditch that.

In C++ with Boost libraries, "just" use a template function with two parameters and a conditional type definition to detect whether or not the two flags are in the same bucket. You lose all C++ compilers too archaic for Substitution Failure Is Not An Error (SFINAE). [While not relevant to this construction, a number of Boost libraries do imply not using MSVC++'s ISO-breaking Safe STL.]

[zaimoni]

This is one of the many bad things induced by bit arithmetic. Such coding is very error prone, especally when you extend your variant. If the 'flags' bitfields is not the one variable (of a few) that holds the RCONF and RCHAOS bits, you have an error that people may not notice for years.

This is what automatic unit tests are for -- in any language, not just C. C just makes it easier by standardizing the NDEBUG flag for omitting them in release builds.

If the RCONF and RCHAOS bits land at opposite sides of a bitfiled boundary you can get another error and you cannot write the code in this way at all

Wrong. Pete's example works perfectly fine as long as the two flags really are in the same bitfield bucket. (EDIT: Assumed "another error" was accurate. As a bitfield boundary is well-defined only within a bitvector, this is the same error as your first example.)

The macros can catch inappropriate flags, though, to point out errors, or they can translate the flags to save space in the bit vector, when really needed.

Almost wrong. You need Boost Preprocessor to do this cleanly at all (the *one* Boost library that can live in C), and the standard release version wipes out at 256 (hard-limiting you to a 256-bit bitvector).