[ADT] Deprecate Optional::getValueOr (NFC)

[kazu]

Summary

This patch deprecates getValueOr in favor of value_or.

[dblaikie]

I was originally planning to deprecate this method, but I cannot put [[deprecated]] on C++ templates.

Hmm, I'm not sure I understand what you mean. It looks like you could put [[deprecated]] on a member function template: https://godbolt.org/z/jG1KxMT1f

[kazu]

In D130140#3677054, @dblaikie wrote:

I was originally planning to deprecate this method, but I cannot put [[deprecated]] on C++ templates.

Hmm, I'm not sure I understand what you mean. It looks like you could put [[deprecated]] on a member function template: https://godbolt.org/z/jG1KxMT1f

Oops. I was putting [[deprecated]] before template. It works now with [[deprecated]] immediately before the return type. Let me revise the patch.

[kazu]

Use [[deprecate]] instead of removing getValueOr.

[kazu]

Please take look. Thanks!

[dblaikie]

Looks good to me, if it's sufficiently portable to supported compilers, etc.

llvm/include/llvm/ADT/Optional.h

322

Do we have prior art (other instances of use of the raw [[deprecated]] attribute without any macro conditionals/feature checking/etc) that this is compatible with all LLVM supported compilers, etc?

llvm/include/llvm/ADT/Optional.h

public:
   constexpr const T *operator->() const { return getPointer(); }
   T *operator->() { return getPointer(); }
   constexpr const T &operator*() const & { return value(); }
   T &operator*() & { return value(); }
 
   template <typename U> constexpr T value_or(U &&alt) const & {
     return has_value() ? value() : std::forward<U>(alt);
   }
-  template <typename U> constexpr T getValueOr(U &&alt) const & {
+  template <typename U>
+  [[deprecated("Use value_or instead.")]] constexpr T
+  getValueOr(U &&alt) const & {
     return has_value() ? value() : std::forward<U>(alt);
   }
 
   /// Apply a function to the value if present; otherwise return None.
   template <class Function>
   auto map(const Function &F) const & -> Optional<decltype(F(value()))> {
     if (*this)
       return F(value());
     return None;
   }
 
   T &&value() && { return std::move(Storage.value()); }
   T &&getValue() && { return std::move(Storage.value()); }
   T &&operator*() && { return std::move(Storage.value()); }
 
   template <typename U> T value_or(U &&alt) && {
     return has_value() ? std::move(value()) : std::forward<U>(alt);
   }
-  template <typename U> T getValueOr(U &&alt) && {
+  template <typename U>
+  [[deprecated("Use value_or instead.")]] T getValueOr(U &&alt) && {
     return has_value() ? std::move(value()) : std::forward<U>(alt);
   }
 
   /// Apply a function to the value if present; otherwise return None.
   template <class Function>
   auto map(const Function &F)
       && -> Optional<decltype(F(std::move(*this).value()))> {
     if (*this)

llvm/unittests/ADT/OptionalTest.cpp

TEST(OptionalTest, InPlaceConstructionNonDefaultConstructibleTest) {
   EXPECT_EQ(0u, NonDefaultConstructible::CopyConstructions);
   EXPECT_EQ(0u, NonDefaultConstructible::CopyAssignments);
   EXPECT_EQ(1u, NonDefaultConstructible::Destructions);
 }
 
 TEST(OptionalTest, GetValueOr) {
   Optional<int> A;
   EXPECT_EQ(42, A.value_or(42));
-  EXPECT_EQ(42, A.getValueOr(42));
 
   A = 5;
   EXPECT_EQ(5, A.value_or(42));
-  EXPECT_EQ(5, A.getValueOr(42));
 }
 
 struct MultiArgConstructor {
   int x, y;
   MultiArgConstructor(int x, int y) : x(x), y(y) {}
   explicit MultiArgConstructor(int x, bool positive)
     : x(x), y(positive ? x : -x) {}
 
TEST(OptionalTest, MoveValueOr) {
   A = MoveOnly(5);
   MoveOnly::ResetCounts();
   EXPECT_EQ(5, std::move(A).value_or(MoveOnly(42)).val);
   EXPECT_EQ(1u, MoveOnly::MoveConstructions);
   EXPECT_EQ(0u, MoveOnly::MoveAssignments);
   EXPECT_EQ(2u, MoveOnly::Destructions);
 }
 
-TEST(OptionalTest, MoveGetValueOr) {
-  Optional<MoveOnly> A;
-
-  MoveOnly::ResetCounts();
-  EXPECT_EQ(42, std::move(A).getValueOr(MoveOnly(42)).val);
-  EXPECT_EQ(1u, MoveOnly::MoveConstructions);
-  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
-  EXPECT_EQ(2u, MoveOnly::Destructions);
-
-  A = MoveOnly(5);
-  MoveOnly::ResetCounts();
-  EXPECT_EQ(5, std::move(A).getValueOr(MoveOnly(42)).val);
-  EXPECT_EQ(1u, MoveOnly::MoveConstructions);
-  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
-  EXPECT_EQ(2u, MoveOnly::Destructions);
-}
-
 struct EqualTo {
   template <typename T, typename U> static bool apply(const T &X, const U &Y) {
     return X == Y;
   }
 };
 
 struct NotEqualTo {
   template <typename T, typename U> static bool apply(const T &X, const U &Y) {