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

David Blaikie dblaikie at gmail.com
Tue Nov 11 18:06:08 PST 2014


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());
+}
+
+}





More information about the llvm-commits mailing list