Introduce llvm/ADT/OptionSet.h utility class

[akyrtzi]

Summary

Introduces ‘OptionSet’ in llvm/ADT headers, which is a utility class that makes it convenient to work with enumerators representing bit options.

We have found it useful in the swift repo where it originates. Apple owns the copyright on the entire file as posted for review and can thus relicense under the LLVM license.

For example of its usefulness, if accepted I intend to use OptionSet in clang code, for 'SymbolRoleSet' here: http://llvm.org/svn/llvm-project/cfe/trunk/include/clang/Index/IndexSymbol.h.
This will improve type safety and make the code more readable, avoiding all the 'unsigned' casts such as this:

Roles |= (unsigned)SymbolRole::Implicit;

MSVC 2013 had some trouble with the code and there are parts ifdef’ed out for MSVC 2013.
Aaron was kind enough to offer feedback on the MSVC issues.
Per Jordan, the static assert that gives trouble for MSVC is just an enforced style check.

The specific MSVC issues were:

Compiler crash:
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9718
Compiler error in unit test:
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9720
Unit test failure:
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9721

[reames]

LGTM w/minor comments.

include/llvm/ADT/OptionSet.h

51

How does this differ from the one above it?

55

Please add an assertion that Storage is a power of two inside this constructor. This is the only place we get to validate that Flag is actually a disjoint bitset.

74

Should this be a static_cast<T>? Otherwise, why have the type param?

99

minor: expression & in terms of &= or vice versa would make it more clear that both are correct (or equally wrong), but increasing test coverage. Its very, very likely that any decent compiler will elide the extra copies.

unittests/ADT/OptionSetTest.cpp

89

Seems like this could be handled just like the early constructable return.

[chandlerc]

include/llvm/ADT/OptionSet.h

74

It causes 'sizeof(T)' to be dependent for the purpose of SFINAE.

At the point you've done this though, you might as well also tuck the enable_if into a template argument. You're already using the feature with an optional template argument here...

[• ham99922]

This comment was removed by asl.

[• ham99922]

This comment was removed by asl.

[• ham999]

ConversionFunction

include/llvm/ADT/OptionSet.h

+//===--- OptionSet.h - Sets of boolean options ------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines the OptionSet class template.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ADT_OPTIONSET_H
+#define LLVM_ADT_OPTIONSET_H
+
+#include "llvm/ADT/None.h"
+#include "llvm/Support/Compiler.h"
+
+#include <type_traits>
+#include <cstdint>
+
+namespace llvm {
+
+/// The class template \c OptionSet captures a set of options stored as the
+/// bits in an unsigned integral value.
+///
+/// Each option corresponds to a particular flag value in the provided
+/// enumeration type (\c Flags). The option set provides ways to add options,
+/// remove options, intersect sets, etc., providing a thin type-safe layer
+/// over the underlying unsigned value.
+///
+/// \tparam Flags An enumeration type that provides the individual flags
+/// for options. Each enumerator should have a power-of-two value, indicating
+/// which bit it is associated with.
+///
+/// \tparam StorageType The unsigned integral type to use to store the flags
+/// enabled within this option set. This defaults to the unsigned form of the
+/// underlying type of the enumeration.
+template<typename Flags,
+         typename StorageType = typename std::make_unsigned<
+                                  typename std::underlying_type<Flags>::type
+                                 >::type>
+class OptionSet {
+  StorageType Storage;
+
+public:
+  /// Create an empty option set.
+  OptionSet() : Storage() { }
+
+  /// Create an empty option set.
+  OptionSet(llvm::NoneType) : Storage() { }
+
+  /// Create an option set with only the given option set.
+  OptionSet(Flags flag) : Storage(static_cast<StorageType>(flag)) { }
+
+  /// Create an option set from raw storage.
+  explicit OptionSet(StorageType storage) : Storage(storage) { }
+
+  /// Check whether an option set is non-empty.
+  explicit operator bool() const { return Storage != 0; }
+
+  /// Explicitly convert an option set to its underlying storage.
+  explicit operator StorageType() const { return Storage; }
+
+  /// Explicitly convert an option set to intptr_t, for use in
+  /// llvm::PointerIntPair.
+  ///
+  /// This member is not present if the underlying type is bigger than
+  /// a pointer.
+  template <typename T = std::intptr_t>
+  explicit operator typename std::enable_if<sizeof(StorageType) <= sizeof(T),
+      std::intptr_t>::type () const {
+    return static_cast<intptr_t>(Storage);
+  }
+
+  /// Retrieve the "raw" representation of this option set.
+  StorageType toRaw() const { return Storage; }
+  
+  /// Determine whether this option set contains all of the options in the
+  /// given set.
+  bool contains(OptionSet set) const {
+    return !static_cast<bool>(set - *this);
+  }
+
+  /// Produce the union of two option sets.
+  friend OptionSet operator|(OptionSet lhs, OptionSet rhs) {
+    return OptionSet(lhs.Storage | rhs.Storage);
+  }
+
+  /// Produce the union of two option sets.
+  friend OptionSet &operator|=(OptionSet &lhs, OptionSet rhs) {
+    lhs.Storage |= rhs.Storage;
+    return lhs;
+ }
+
+  /// Produce the intersection of two option sets.
+  friend OptionSet operator&(OptionSet lhs, OptionSet rhs) {
+    return OptionSet(lhs.Storage & rhs.Storage);
+  }
+
+  /// Produce the intersection of two option sets.
+  friend OptionSet &operator&=(OptionSet &lhs, OptionSet rhs) {
+    lhs.Storage &= rhs.Storage;
+    return lhs;
+  }
+
+  /// Produce the difference of two option sets.
+  friend OptionSet operator-(OptionSet lhs, OptionSet rhs) {
+    return OptionSet(lhs.Storage & ~rhs.Storage);
+  }
+
+  /// Produce the intersection of two option sets.
+  friend OptionSet &operator-=(OptionSet &lhs, OptionSet rhs) {
+    lhs.Storage &= ~rhs.Storage;
+    return lhs;
+  }
+
+private:
+#if !defined(_MSC_VER) || LLVM_MSC_PREREQ(1900)
+  // MSVC 2013 has an ICE due to the following.
+  template <typename T>
+  static auto _checkResultTypeOperatorOr(T t) -> decltype(t | t) { return T(); }
+
+  static void _checkResultTypeOperatorOr(...) {}
+
+  static_assert(!std::is_same<decltype(_checkResultTypeOperatorOr(Flags())),
+                              Flags>::value,
+                "operator| should produce an OptionSet");
+#endif
+};
+
+}
+
+#endif

unittests/ADT/CMakeLists.txt

   MakeUniqueTest.cpp
   MapVectorTest.cpp
   OptionalTest.cpp
+  OptionSetTest.cpp
   PackedVectorTest.cpp
   PointerEmbeddedIntTest.cpp
   PointerIntPairTest.cpp

unittests/ADT/OptionSetTest.cpp

+//===- llvm/unittests/ADT/OptionSetTest.cpp -------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/OptionSet.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+TEST(OptionSet, contains) {
+  enum class Flags {
+    A = 1 << 0,
+    B = 1 << 1,
+    C = 1 << 2
+  };
+
+  OptionSet<Flags> emptySet;
+  OptionSet<Flags> aSet = Flags::A;
+  OptionSet<Flags> abSet = aSet | Flags::B;
+  OptionSet<Flags> abcSet = abSet | Flags::C;
+  OptionSet<Flags> bcSet = abcSet - Flags::A;
+  OptionSet<Flags> cSet = bcSet - Flags::B;
+
+  EXPECT_TRUE(emptySet.contains(emptySet));
+  EXPECT_FALSE(emptySet.contains(aSet));
+  EXPECT_FALSE(emptySet.contains(abSet));
+  EXPECT_FALSE(emptySet.contains(abcSet));
+  EXPECT_FALSE(emptySet.contains(bcSet));
+  EXPECT_FALSE(emptySet.contains(cSet));
+
+  EXPECT_TRUE(aSet.contains(emptySet));
+  EXPECT_TRUE(aSet.contains(aSet));
+  EXPECT_FALSE(aSet.contains(abSet));
+  EXPECT_FALSE(aSet.contains(abcSet));
+  EXPECT_FALSE(aSet.contains(bcSet));
+  EXPECT_FALSE(aSet.contains(cSet));
+
+  EXPECT_TRUE(abSet.contains(emptySet));
+  EXPECT_TRUE(abSet.contains(aSet));
+  EXPECT_TRUE(abSet.contains(abSet));
+  EXPECT_FALSE(abSet.contains(abcSet));
+  EXPECT_FALSE(abSet.contains(bcSet));
+  EXPECT_FALSE(abSet.contains(cSet));
+
+  EXPECT_TRUE(abcSet.contains(emptySet));
+  EXPECT_TRUE(abcSet.contains(aSet));
+  EXPECT_TRUE(abcSet.contains(abSet));
+  EXPECT_TRUE(abcSet.contains(abcSet));
+  EXPECT_TRUE(abcSet.contains(bcSet));
+  EXPECT_TRUE(abcSet.contains(cSet));
+}
+
+
+TEST(OptionSet, intptr_t) {
+  enum class Small : int8_t {
+    A = 1 << 0
+  };
+
+  OptionSet<Small> small = Small::A;
+  EXPECT_EQ(static_cast<intptr_t>(Small::A), static_cast<intptr_t>(small));
+
+
+#if !defined(_MSC_VER) || LLVM_MSC_PREREQ(1900)
+  // Fails to compile in MSVC 2013 because it lacks proper constexpr support
+  // for std::numeric_limits.
+  enum class UPtr : uintptr_t {
+    A = std::numeric_limits<uintptr_t>::max()
+  };
+
+  OptionSet<UPtr> uptr = UPtr::A;
+  EXPECT_EQ(static_cast<intptr_t>(UPtr::A), static_cast<intptr_t>(uptr));
+
+
+  enum class Ptr : intptr_t {
+    A = std::numeric_limits<intptr_t>::min()
+  };
+
+  OptionSet<Ptr> ptr = Ptr::A;
+  EXPECT_EQ(static_cast<intptr_t>(Ptr::A), static_cast<intptr_t>(ptr));
+#endif
+}
+
+#if !defined(_MSC_VER) || LLVM_MSC_PREREQ(1900)
+// This fails on MSVC 2013; is_constructible returns true despite
+// sizeof(intptr_t) == 4 and sizeof(long long) == 8.
+TEST(OptionSet, intptr_t_isConstructible) {
+  // First check that std::is_constructible counts explicit conversion
+  // operators.
+  class AlwaysConvertible {
+  public:
+    explicit operator intptr_t () const { return 0; }
+  };
+
+  if (!std::is_constructible<intptr_t, AlwaysConvertible>::value) {
+    // std::is_constructible doesn't test what we want it to. Just exit early.
+    return;
+  }
+
+  enum class LongLong : unsigned long long {
+    A = 1
+  };
+  bool isConvertible =
+      std::is_constructible<intptr_t, OptionSet<LongLong>>::value;
+
+  if (sizeof(intptr_t) < sizeof(long long)) {
+    EXPECT_FALSE(isConvertible);
+  } else {
+    EXPECT_TRUE(isConvertible);
+  }
+}
+#endif
+