[llvm] r368939 - Remove LVALUE / RVALUE workarounds

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 15 09:18:48 PDT 2019


Thanks for doing this, I’ll take a look.

> On Aug 15, 2019, at 3:13 AM, Russell Gallop <russell.gallop at gmail.com> wrote:
> 
> Hi,
> 
> I've reverted this in r368985 to get the Windows bots green again.
> 
> Regards
> Russ
> 
> On Thu, 15 Aug 2019 at 07:54, via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Hi,
> 
> The test that was added in this change with the removal of the LLVM_HAS_RVALUE_REFERENCE_THIS seems to be failing with run on Windows:
> 
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/2727 <http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/2727>
> FAIL: LLVM-Unit :: ADT/./ADTTests.exe/OptionalTest.MoveGetValueOr (535 of 3846)
> ******************** TEST 'LLVM-Unit :: ADT/./ADTTests.exe/OptionalTest.MoveGetValueOr' FAILED ********************
> Note: Google Test filter = OptionalTest.MoveGetValueOr
> 
> [==========] Running 1 test from 1 test case.
> 
> [----------] Global test environment set-up.
> 
> [----------] 1 test from OptionalTest
> 
> [ RUN      ] OptionalTest.MoveGetValueOr
> 
> C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm\unittests\ADT\OptionalTest.cpp(390): error:       Expected: 1u
> 
>       Which is: 1
> 
> To be equal to: MoveOnly::MoveConstructions
> 
>       Which is: 2
> 
> C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm\unittests\ADT\OptionalTest.cpp(392): error:       Expected: 2u
> 
>       Which is: 2
> 
> To be equal to: MoveOnly::Destructions
> 
>       Which is: 3
> 
> C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm\unittests\ADT\OptionalTest.cpp(397): error:       Expected: 1u
> 
>       Which is: 1
> 
> To be equal to: MoveOnly::MoveConstructions
> 
>       Which is: 2
> 
> C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm\unittests\ADT\OptionalTest.cpp(399): error:       Expected: 2u
> 
>       Which is: 2
> 
> To be equal to: MoveOnly::Destructions
> 
>       Which is: 3
> 
> [  FAILED  ] OptionalTest.MoveGetValueOr (0 ms)
> 
> [----------] 1 test from OptionalTest (0 ms total)
> 
> 
> 
> [----------] Global test environment tear-down
> 
> [==========] 1 test from 1 test case ran. (0 ms total)
> 
> [  PASSED  ] 0 tests.
> 
> [  FAILED  ] 1 test, listed below:
> 
> [  FAILED  ] OptionalTest.MoveGetValueOr
> 
> 
> 
>  1 FAILED TEST
> 
> Can you take a look?
> 
> Douglas Yung
> 
> -----Original Message-----
> From: llvm-commits <llvm-commits-bounces at lists.llvm.org <mailto:llvm-commits-bounces at lists.llvm.org>> On Behalf Of JF Bastien via llvm-commits
> Sent: Wednesday, August 14, 2019 15:48
> To: llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> Subject: [llvm] r368939 - Remove LVALUE / RVALUE workarounds
> 
> Author: jfb
> Date: Wed Aug 14 15:48:12 2019
> New Revision: 368939
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=368939&view=rev <http://llvm.org/viewvc/llvm-project?rev=368939&view=rev>
> Log:
> Remove LVALUE / RVALUE workarounds
> 
> Summary: LLVM_HAS_RVALUE_REFERENCE_THIS and LLVM_LVALUE_FUNCTION shouldn't be needed anymore because the minimum compiler versions support them.
> 
> Subscribers: jkorous, dexonsmith, cfe-commits, llvm-commits, hans, thakis, chandlerc, rnk
> 
> Tags: #clang, #llvm
> 
> Differential Revision: https://reviews.llvm.org/D66240 <https://reviews.llvm.org/D66240>
> 
> Modified:
>     llvm/trunk/include/llvm/ADT/Optional.h
>     llvm/trunk/include/llvm/Support/Compiler.h
>     llvm/trunk/unittests/ADT/OptionalTest.cpp
> 
> Modified: llvm/trunk/include/llvm/ADT/Optional.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=368939&r1=368938&r2=368939&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=368939&r1=368938&r2=368939&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/Optional.h (original)
> +++ llvm/trunk/include/llvm/ADT/Optional.h Wed Aug 14 15:48:12 2019
> @@ -69,20 +69,18 @@ public:
> 
>    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();
> @@ -169,20 +167,19 @@ public:
> 
>    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();
> @@ -252,22 +249,21 @@ public:
> 
>    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);
>    }
> 
> -#if LLVM_HAS_RVALUE_REFERENCE_THIS
>    T &&getValue() && { return std::move(Storage.getValue()); }
>    T &&operator*() && { return std::move(Storage.getValue()); }
> 
> @@ -275,7 +271,6 @@ public:
>    T getValueOr(U &&value) && {
>      return hasValue() ? std::move(getValue()) : std::forward<U>(value);
>    }
> -#endif
>  };
> 
>  template <typename T, typename U>
> 
> Modified: llvm/trunk/include/llvm/Support/Compiler.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=368939&r1=368938&r2=368939&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=368939&r1=368938&r2=368939&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/Compiler.h (original)
> +++ llvm/trunk/include/llvm/Support/Compiler.h Wed Aug 14 15:48:12 2019
> @@ -83,26 +83,6 @@
>  #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
> 
> Modified: llvm/trunk/unittests/ADT/OptionalTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=368939&r1=368938&r2=368939&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=368939&r1=368938&r2=368939&view=diff>
> ==============================================================================
> --- llvm/trunk/unittests/ADT/OptionalTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/OptionalTest.cpp Wed Aug 14 15:48:12 2019
> @@ -382,8 +382,6 @@ TEST_F(OptionalTest, ImmovableEmplace) {
>    EXPECT_EQ(0u, Immovable::Destructions);  }
> 
> -#if LLVM_HAS_RVALUE_REFERENCE_THIS
> -
>  TEST_F(OptionalTest, MoveGetValueOr) {
>    Optional<MoveOnly> A;
> 
> @@ -401,8 +399,6 @@ TEST_F(OptionalTest, MoveGetValueOr) {
>    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;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190815/e026dcce/attachment.html>


More information about the llvm-commits mailing list