Unconditionally enable lvalue function designators; NFC

[aaron.ballman]

Summary

We previously had to guard against older MSVC and GCC versions which had rvalue references but not support for marking functions with ref qualifiers. However, having bumped our minimum required version to MSVC 2017 and GCC 5.1 mean we can unconditionally enable this feature. Rather than keeping the macro around, this replaces use of the macro with the actual ref qualifier.

I tested with my copy of MSVC 2017, but I am not on the base release of 2017, so additional testing would be appreciated to verify the patch.

[thakis]

Realistically, the the only way to find out if this works is to land and see.

[aaron.ballman]

I've commit in 968561bcdc34c7d74482fe3bb69a045abf08d2c1 (clang) and 968561bcdc34c7d74482fe3bb69a045abf08d2c1 (llvm) and will keep an eye on the bots to revert if needed. Thanks!

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h

 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/Support/Allocator.h"
-#include "llvm/Support/Compiler.h"
 #include <cassert>
 #include <cstdint>
 #include <memory>
 #include <utility>
 #include <vector>
 
 namespace clang {
 
public:
   template <typename T>
   T &getAnalysis() const {
     return *getLocationContext()->getAnalysis<T>();
   }
 
   const ProgramStateRef &getState() const { return State; }
 
   template <typename T>
-  Optional<T> getLocationAs() const LLVM_LVALUE_FUNCTION {
+  Optional<T> getLocationAs() const & {
     return Location.getAs<T>();
   }
 
   /// Get the value of an arbitrary expression at this node.
   SVal getSVal(const Stmt *S) const {
     return getState()->getSVal(S, getLocationContext());
   }
 

llvm/include/llvm/ADT/Optional.h

 //  a value can be optional.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_OPTIONAL_H
 #define LLVM_ADT_OPTIONAL_H
 
 #include "llvm/ADT/None.h"
-#include "llvm/Support/Compiler.h"
 #include "llvm/Support/type_traits.h"
 #include <cassert>
 #include <memory>
 #include <new>
 #include <utility>
 
 namespace llvm {
 
void reset() noexcept {
     if (hasVal) {
       value.~T();
       hasVal = false;
     }
   }
 
   bool hasValue() const noexcept { return hasVal; }
 
-  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+  T &getValue() & noexcept {
     assert(hasVal);
     return value;
   }
-  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+  T const &getValue() const & noexcept {
     assert(hasVal);
     return value;
   }
-#if LLVM_HAS_RVALUE_REFERENCE_THIS
   T &&getValue() && noexcept {
     assert(hasVal);
     return std::move(value);
   }
-#endif
 
   template <class... Args> void emplace(Args &&... args) {
     reset();
     ::new ((void *)std::addressof(value)) T(std::forward<Args>(args)...);
     hasVal = true;
   }
 
   OptionalStorage &operator=(T const &y) {
void reset() noexcept {
     if (hasVal) {
       value.~T();
       hasVal = false;
     }
   }
 
   bool hasValue() const noexcept { return hasVal; }
 
-  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+  T &getValue() & noexcept {
     assert(hasVal);
     return value;
   }
-  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+  T const &getValue() const & noexcept {
     assert(hasVal);
     return value;
   }
-#if LLVM_HAS_RVALUE_REFERENCE_THIS
   T &&getValue() && noexcept {
     assert(hasVal);
     return std::move(value);
   }
-#endif
 
   template <class... Args> void emplace(Args &&... args) {
     reset();
     ::new ((void *)std::addressof(value)) T(std::forward<Args>(args)...);
     hasVal = true;
   }
 
   OptionalStorage &operator=(T const &y) {
Optional &operator=(const T &y) {
     return *this;
   }
   Optional &operator=(const Optional &O) = default;
 
   void reset() { Storage.reset(); }
 
   const T *getPointer() const { return &Storage.getValue(); }
   T *getPointer() { return &Storage.getValue(); }
-  const T &getValue() const LLVM_LVALUE_FUNCTION { return Storage.getValue(); }
-  T &getValue() LLVM_LVALUE_FUNCTION { return Storage.getValue(); }
+  const T &getValue() const & { return Storage.getValue(); }
+  T &getValue() & { return Storage.getValue(); }
 
   explicit operator bool() const { return hasValue(); }
   bool hasValue() const { return Storage.hasValue(); }
   const T *operator->() const { return getPointer(); }
   T *operator->() { return getPointer(); }
-  const T &operator*() const LLVM_LVALUE_FUNCTION { return getValue(); }
-  T &operator*() LLVM_LVALUE_FUNCTION { return getValue(); }
+  const T &operator*() const & { return getValue(); }
+  T &operator*() & { return getValue(); }
 
   template <typename U>
-  constexpr T getValueOr(U &&value) const LLVM_LVALUE_FUNCTION {
+  constexpr T getValueOr(U &&value) const & {
     return hasValue() ? getValue() : std::forward<U>(value);
   }
 
   /// Apply a function to the value if present; otherwise return None.
   template <class Function>
-  auto map(const Function &F) const
+  auto map(const Function &F) const &
       -> Optional<decltype(F(getValue()))> {
     if (*this) return F(getValue());
     return None;
   }
 
-#if LLVM_HAS_RVALUE_REFERENCE_THIS
   T &&getValue() && { return std::move(Storage.getValue()); }
   T &&operator*() && { return std::move(Storage.getValue()); }
 
   template <typename U>
   T getValueOr(U &&value) && {
     return hasValue() ? std::move(getValue()) : std::forward<U>(value);
   }
 
   /// 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).getValue()))> {
     if (*this) return F(std::move(*this).getValue());
     return None;
   }
-#endif
 };
 
 template <typename T, typename U>
 bool operator==(const Optional<T> &X, const Optional<U> &Y) {
   if (X && Y)
     return *X == *Y;
   return X.hasValue() == Y.hasValue();
 }

llvm/include/llvm/ADT/PointerIntPair.h

 //===- llvm/ADT/PointerIntPair.h - Pair for pointer and int -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the PointerIntPair class.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_POINTERINTPAIR_H
 #define LLVM_ADT_POINTERINTPAIR_H
 
-#include "llvm/Support/Compiler.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
 #include "llvm/Support/type_traits.h"
 #include <cassert>
 #include <cstdint>
 #include <limits>
 
 namespace llvm {
 
public:
   }
 
   explicit PointerIntPair(PointerTy PtrVal) { initWithPointer(PtrVal); }
 
   PointerTy getPointer() const { return Info::getPointer(Value); }
 
   IntType getInt() const { return (IntType)Info::getInt(Value); }
 
-  void setPointer(PointerTy PtrVal) LLVM_LVALUE_FUNCTION {
+  void setPointer(PointerTy PtrVal) & {
     Value = Info::updatePointer(Value, PtrVal);
   }
 
-  void setInt(IntType IntVal) LLVM_LVALUE_FUNCTION {
+  void setInt(IntType IntVal) & {
     Value = Info::updateInt(Value, static_cast<intptr_t>(IntVal));
   }
 
-  void initWithPointer(PointerTy PtrVal) LLVM_LVALUE_FUNCTION {
+  void initWithPointer(PointerTy PtrVal) & {
     Value = Info::updatePointer(0, PtrVal);
   }
 
-  void setPointerAndInt(PointerTy PtrVal, IntType IntVal) LLVM_LVALUE_FUNCTION {
+  void setPointerAndInt(PointerTy PtrVal, IntType IntVal) & {
     Value = Info::updateInt(Info::updatePointer(0, PtrVal),
                             static_cast<intptr_t>(IntVal));
   }
 
   PointerTy const *getAddrOfPointer() const {
     return const_cast<PointerIntPair *>(this)->getAddrOfPointer();
   }
 
   PointerTy *getAddrOfPointer() {
     assert(Value == reinterpret_cast<intptr_t>(getPointer()) &&
            "Can only return the address if IntBits is cleared and "
            "PtrTraits doesn't change the pointer");
     return reinterpret_cast<PointerTy *>(&Value);
   }
 
   void *getOpaqueValue() const { return reinterpret_cast<void *>(Value); }
 
-  void setFromOpaqueValue(void *Val) LLVM_LVALUE_FUNCTION {
+  void setFromOpaqueValue(void *Val) & {
     Value = reinterpret_cast<intptr_t>(Val);
   }
 
   static PointerIntPair getFromOpaqueValue(void *V) {
     PointerIntPair P;
     P.setFromOpaqueValue(V);
     return P;
   }

llvm/include/llvm/Support/Compiler.h

 #if !LLVM_MSC_PREREQ(1910)
 #error LLVM requires at least MSVC 2017.
 #endif
 
 #else
 #define LLVM_MSC_PREREQ(version) 0
 #endif
 
-/// Does the compiler support ref-qualifiers for *this?
-///
-/// Sadly, this is separate from just rvalue reference support because GCC
-/// and MSVC implemented this later than everything else.
-#if __has_feature(cxx_rvalue_references) || LLVM_GNUC_PREREQ(4, 8, 1)
-#define LLVM_HAS_RVALUE_REFERENCE_THIS 1
-#else
-#define LLVM_HAS_RVALUE_REFERENCE_THIS 0
-#endif
-
-/// Expands to '&' if ref-qualifiers for *this are supported.
-///
-/// This can be used to provide lvalue/rvalue overrides of member functions.
-/// The rvalue override should be guarded by LLVM_HAS_RVALUE_REFERENCE_THIS
-#if LLVM_HAS_RVALUE_REFERENCE_THIS
-#define LLVM_LVALUE_FUNCTION &
-#else
-#define LLVM_LVALUE_FUNCTION
-#endif
-
 /// LLVM_LIBRARY_VISIBILITY - If a class marked with this attribute is linked
 /// into a shared library, then the class should be private to the library and
 /// not accessible from outside it.  Can also be used to mark variables and
 /// functions, making them private to any shared library they are linked into.
 /// On PE/COFF targets, library visibility is the default, so this isn't needed.
 ///
 /// LLVM_EXTERNAL_VISIBILITY - classes, functions, and variables marked with
 /// this attribute will be made public and visible outside of any shared library

llvm/unittests/ADT/OptionalTest.cpp

   Immovable::ResetCounts();
   A.emplace(4);
   EXPECT_TRUE((bool)A);
   EXPECT_EQ(4, A->val);
   EXPECT_EQ(1u, Immovable::Constructions);
   EXPECT_EQ(0u, Immovable::Destructions);
 }
 
-#if LLVM_HAS_RVALUE_REFERENCE_THIS
-
 TEST_F(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);
 }
 
-#endif // LLVM_HAS_RVALUE_REFERENCE_THIS
-
 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) {