From 45dc480b7514ca7760d3ed25f9e86d50e47a43e2 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Wed, 12 Nov 2014 02:06:08 +0000 Subject: [PATCH] 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) llvm-svn: 221753 --- llvm/include/llvm/ADT/STLExtras.h | 7 +++++-- llvm/unittests/ADT/CMakeLists.txt | 1 + llvm/unittests/ADT/FunctionRefTest.cpp | 28 ++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 llvm/unittests/ADT/FunctionRefTest.cpp diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h index f69a15b..16f850c 100644 --- a/llvm/include/llvm/ADT/STLExtras.h +++ b/llvm/include/llvm/ADT/STLExtras.h @@ -77,8 +77,11 @@ class function_ref { } public: - template - function_ref(Callable &&callable) + template + function_ref(Callable &&callable, + typename std::enable_if< + !std::is_same::type, + function_ref>::value>::type * = nullptr) : callback(callback_fn::type>), callable(reinterpret_cast(&callable)) {} Ret operator()(Params ...params) const { diff --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt index 0f214f3..845e805 100644 --- a/llvm/unittests/ADT/CMakeLists.txt +++ b/llvm/unittests/ADT/CMakeLists.txt @@ -13,6 +13,7 @@ set(ADTSources DenseMapTest.cpp DenseSetTest.cpp FoldingSet.cpp + FunctionRefTest.cpp HashingTest.cpp ilistTest.cpp ImmutableMapTest.cpp diff --git a/llvm/unittests/ADT/FunctionRefTest.cpp b/llvm/unittests/ADT/FunctionRefTest.cpp new file mode 100644 index 0000000..075d9a0 --- /dev/null +++ b/llvm/unittests/ADT/FunctionRefTest.cpp @@ -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 X = A; + function_ref Y = X; + X = B; + EXPECT_EQ(1, Y()); +} + +} -- 2.7.4