[llvm] r221753 - Ensure function_refs are copyable even from non-const references

[Rafael Espíndola]

Thanks!

On 11 November 2014 21:06, David Blaikie <dblaikie at gmail.com> wrote:
> Author: dblaikie
> Date: Tue Nov 11 20:06:08 2014
> New Revision: 221753
>
> URL: http://llvm.org/viewvc/llvm-project?rev=221753&view=rev
> Log:
> Ensure function_refs are copyable even from non-const references
>
> A subtle bug was found where attempting to copy a non-const function_ref
> lvalue would actually invoke the generic forwarding constructor (as it
> was a closer match - being T& rather than the const T& of the implicit
> copy constructor). In the particular case this lead to a dangling
> function_ref member (since it had referenced the function_ref passed by
> value to its ctor, rather than the outer function_ref that was still
> alive)
>
> SFINAE the converting constructor to not be considered if the copy
> constructor is available and demonstrate that this causes the copy to
> refer to the original functor, not to the function_ref it was copied
> from. (without the code change, the test would fail as Y would be
> referencing X and Y() would see the result of the mutation to X, ie: 2)
>
> Added:
>     llvm/trunk/unittests/ADT/FunctionRefTest.cpp
> Modified:
>     llvm/trunk/include/llvm/ADT/STLExtras.h
>     llvm/trunk/unittests/ADT/CMakeLists.txt
>
> Modified: llvm/trunk/include/llvm/ADT/STLExtras.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/STLExtras.h?rev=221753&r1=221752&r2=221753&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/STLExtras.h (original)
> +++ llvm/trunk/include/llvm/ADT/STLExtras.h Tue Nov 11 20:06:08 2014
> @@ -77,8 +77,11 @@ class function_ref<Ret(Params...)> {
>    }
>
>  public:
> -  template<typename Callable>
> -  function_ref(Callable &&callable)
> +  template <typename Callable>
> +  function_ref(Callable &&callable,
> +               typename std::enable_if<
> +                   !std::is_same<typename std::remove_reference<Callable>::type,
> +                                 function_ref>::value>::type * = nullptr)
>        : callback(callback_fn<typename std::remove_reference<Callable>::type>),
>          callable(reinterpret_cast<intptr_t>(&callable)) {}
>    Ret operator()(Params ...params) const {
>
> Modified: llvm/trunk/unittests/ADT/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/CMakeLists.txt?rev=221753&r1=221752&r2=221753&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/CMakeLists.txt (original)
> +++ llvm/trunk/unittests/ADT/CMakeLists.txt Tue Nov 11 20:06:08 2014
> @@ -13,6 +13,7 @@ set(ADTSources
>    DenseMapTest.cpp
>    DenseSetTest.cpp
>    FoldingSet.cpp
> +  FunctionRefTest.cpp
>    HashingTest.cpp
>    ilistTest.cpp
>    ImmutableMapTest.cpp
>
> Added: llvm/trunk/unittests/ADT/FunctionRefTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/FunctionRefTest.cpp?rev=221753&view=auto
> ==============================================================================
> --- llvm/trunk/unittests/ADT/FunctionRefTest.cpp (added)
> +++ llvm/trunk/unittests/ADT/FunctionRefTest.cpp Tue Nov 11 20:06:08 2014
> @@ -0,0 +1,28 @@
> +//===- llvm/unittest/ADT/MakeUniqueTest.cpp - make_unique unit tests ------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "llvm/ADT/STLExtras.h"
> +#include "gtest/gtest.h"
> +
> +using namespace llvm;
> +
> +namespace {
> +
> +// Ensure that copies of a function_ref copy the underlying state rather than
> +// causing one function_ref to chain to the next.
> +TEST(FunctionRefTest, Copy) {
> +  auto A = [] { return 1; };
> +  auto B = [] { return 2; };
> +  function_ref<int()> X = A;
> +  function_ref<int()> Y = X;
> +  X = B;
> +  EXPECT_EQ(1, Y());
> +}
> +
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits