[Support] Clean up Align

[kazutakahirata]

This patch cleans up the constructors and operator= of Align.

• "constexpr Align(LogValue CA)" is a really strange "public:"
  constructor. It can only be constructed with a private struct
  LogValue, which wraps the log2 of an alignment. Since nobody uses
  it outside the class anyway, this patch moves the constructor to
  "private:" while switching to a tag-based parameter list. In turn,
  this patch removes LogValue.

• The block of comment being deleted is no longer applicable, so this
  patch marks operator= constexpr. To test operator= in a unit test,
  this patch makes value() constexpr also. Refer to the unit test to
  see how operator= and value() are put together.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Kazu Hirata (kazutakahirata)

Changes

This patch cleans up the constructors and operator= of Align.

• "constexpr Align(LogValue CA)" is a really strange "public:"
  constructor. It can only be constructed with a private struct
  LogValue, which wraps the log2 of an alignment. Since nobody uses
  it outside the class anyway, this patch moves the constructor to
  "private:" while switching to a tag-based parameter list. In turn,
  this patch removes LogValue.

• The block of comment being deleted is no longer applicable, so this
  patch marks operator= constexpr. To test operator= in a unit test,
  this patch makes value() constexpr also. Refer to the unit test to
  see how operator= and value() are put together.

---

Full diff: https://github.com/llvm/llvm-project/pull/160644.diff

2 Files Affected:

• (modified) llvm/include/llvm/Support/Alignment.h (+6-17)
• (modified) llvm/unittests/Support/AlignmentTest.cpp (+10)

diff --git a/llvm/include/llvm/Support/Alignment.h b/llvm/include/llvm/Support/Alignment.h
index 84773f1d9c37b..a4ca54e26f18d 100644
--- a/llvm/include/llvm/Support/Alignment.h
+++ b/llvm/include/llvm/Support/Alignment.h
@@ -52,16 +52,8 @@ struct Align {
   friend unsigned encode(struct MaybeAlign A);
   friend struct MaybeAlign decodeMaybeAlign(unsigned Value);
 
-  /// A trivial type to allow construction of constexpr Align.
-  /// This is currently needed to workaround a bug in GCC 5.3 which prevents
-  /// definition of constexpr assign operators.
-  /// https://stackoverflow.com/questions/46756288/explicitly-defaulted-function-cannot-be-declared-as-constexpr-because-the-implic
-  /// FIXME: Remove this, make all assign operators constexpr and introduce user
-  /// defined literals when we don't have to support GCC 5.3 anymore.
-  /// https://llvm.org/docs/GettingStarted.html#getting-a-modern-host-c-toolchain
-  struct LogValue {
-    uint8_t Log;
-  };
+  struct FromShiftValue {};
+  constexpr Align(FromShiftValue, uint8_t Shift) : ShiftValue(Shift) {}
 
 public:
   /// Default is byte-aligned.
@@ -70,8 +62,8 @@ struct Align {
   /// checks have been performed when building `Other`.
   constexpr Align(const Align &Other) = default;
   constexpr Align(Align &&Other) = default;
-  Align &operator=(const Align &Other) = default;
-  Align &operator=(Align &&Other) = default;
+  constexpr Align &operator=(const Align &Other) = default;
+  constexpr Align &operator=(Align &&Other) = default;
 
   explicit Align(uint64_t Value) {
     assert(Value > 0 && "Value must not be 0");
@@ -82,7 +74,7 @@ struct Align {
 
   /// This is a hole in the type system and should not be abused.
   /// Needed to interact with C for instance.
-  uint64_t value() const { return uint64_t(1) << ShiftValue; }
+  constexpr uint64_t value() const { return uint64_t(1) << ShiftValue; }
 
   // Returns the previous alignment.
   Align previous() const {
@@ -94,7 +86,7 @@ struct Align {
 
   /// Allow constructions of constexpr Align.
   template <size_t kValue> constexpr static Align Constant() {
-    return LogValue{static_cast<uint8_t>(ConstantLog2<kValue>())};
+    return Align(FromShiftValue{}, ConstantLog2<kValue>());
   }
 
   /// Allow constructions of constexpr Align from types.
@@ -102,9 +94,6 @@ struct Align {
   template <typename T> constexpr static Align Of() {
     return Constant<std::alignment_of_v<T>>();
   }
-
-  /// Constexpr constructor from LogValue type.
-  constexpr Align(LogValue CA) : ShiftValue(CA.Log) {}
 };
 
 /// Treats the value 0 as a 1, so Align is always at least 1.
diff --git a/llvm/unittests/Support/AlignmentTest.cpp b/llvm/unittests/Support/AlignmentTest.cpp
index 7b771977027b4..3a4416128d0e5 100644
--- a/llvm/unittests/Support/AlignmentTest.cpp
+++ b/llvm/unittests/Support/AlignmentTest.cpp
@@ -44,6 +44,16 @@ TEST(AlignmentTest, AlignConstexprConstant) {
   EXPECT_EQ(Align(alignof(uint64_t)), kConstantAlign);
 }
 
+TEST(AlignmentTest, ConstexprAssign) {
+  constexpr auto assignAndGet = []() constexpr {
+    Align A = Align::Constant<8>();
+    Align B = Align::Constant<16>();
+    A = B;
+    return A.value();
+  };
+  static_assert(assignAndGet() == 16);
+}
+
 std::vector<uint64_t> getValidAlignments() {
   std::vector<uint64_t> Out;
   for (size_t Shift = 0; Shift < 64; ++Shift)