CommandLine -- ResetAllOptionOccurrences with cl::bits -- Bug or by design?

I have some unit tests where I want to parse different command lines where some options use cl::bits with an enum type. I want to reset the option occurrences in each unit test and reinvoke the command line parser with a new command line. While ResetAllOptionOccurrences resets every other kind of option, it doesn’t do so for cl::bits. Is this by design?

I see setDefaultValue of cl::bits is an empty function. Why doesn’t it set the internal bit storage to 0 when ResetAllOptionOccurrences is called? I thought I could instead use an external storage and set that to 0. But that doesn’t work either. It results in an error:

unsigned MyOptStorage;

static llvm::cl::bits<MyOpt, unsigned> MyOpts(

….

llvm::cl::location(MyOptStorage),

…);

include/llvm/Support/CommandLine.h:1723:23: error: reinterpret_cast from 'MyOpt’ to ‘unsigned int’ is not allowed

unsigned BitPos = reinterpret_cast(V);

where ‘MyOpt’ is an enum type.

So how does one specify cl::bits with an external storage location if DataType is an enum? There is not much information in Command Line Reference Manual. Note that I used unsigned above just like the excerpt from the manual says:

The cl::bits class is the class used to represent a list of command line options in the form of a bit vector. It is also a templated class which can take up to three arguments:

namespace cl {

template <class DataType, class Storage = bool,

class ParserClass = parser >

class bits;

}

This class works the exact same as the cl::list class, except that the second argument must be of type unsigned if external storage is used.

I didn’t see a response. But I fixed this in our downstream code and it is working now as expected. It fixes two issues (1) clearing cl::bits and (2) allowing an optional storage location for cl::bits (the reinterpret_cast is wrong for an enum to unsigned cast; it should be a static_cast). If there is interest in fixing this upstream, I can send the following patch for review (note, the following is for our downstream branch but I can put together a patch for master along with some unit tests that expose the issue).

diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h

index 05374e3…712d6d3 100644

— a/llvm/include/llvm/Support/CommandLine.h

+++ b/llvm/include/llvm/Support/CommandLine.h

@@ -1720,7 +1720,7 @@ template <class DataType, class StorageClass> class bits_storage {

unsigned *Location = nullptr; // Where to store the bits…

template static unsigned Bit(const T &V) {

  • unsigned BitPos = reinterpret_cast(V);
  • unsigned BitPos = static_cast(V);

assert(BitPos < sizeof(unsigned) * CHAR_BIT &&

“enum exceeds width of bit vector!”);

return 1 << BitPos;

@@ -1744,6 +1744,11 @@ public:

unsigned getBits() { return *Location; }

  • void clear() {

  • if (Location)

  • *Location = 0;

  • }

Hello Riyaz,

I did indeed answer here: https://lists.llvm.org/pipermail/llvm-dev/2021-February/148299.html

If you have a fix, would you mind sending it on Phabricator please?

Thanks,

Alex.

Sorry. I didn’t see your answer to my email back in Feb. The email I recent last week describes a second issue with reinterpret_cast and didn’t see a response. Thank you. I will send a patch on Phabricator.

/Riyaz

recent → resent.