Actually mark Unexpected as cold

Summary:
Testing indicates that GCC ignores the cold attribute when the function
is available for inlining. Because Unexpected is a template class we
can't make the constructors non-inline, but we can make it derive from a
class with a cold constructor, which has the effect of making all the
Unexpected constructors implicitly cold.

Reviewed By: yfeldblum

Differential Revision: D6261013

fbshipit-source-id: 482e49253d5b104742018133c53fb60279dd9f9b

folly/Expected.h

@@ -39,6 +39,7 @@
 #include <folly/Traits.h>
 #include <folly/Unit.h>
 #include <folly/Utility.h>
+#include <folly/lang/ColdClass.h>
 
 #define FOLLY_EXPECTED_ID(X) FB_CONCATENATE(FB_CONCATENATE(Folly, X), __LINE__)
 
@@ -675,7 +676,7 @@ namespace expected_detail {
  * Expected objects in the error state.
  */
 template <class Error>
-class Unexpected final {
+class Unexpected final : ColdClass {
   template <class E>
   friend class Unexpected;
   template <class V, class E>
@@ -712,10 +713,8 @@ class Unexpected final {
   Unexpected(Unexpected&&) = default;
   Unexpected& operator=(const Unexpected&) = default;
   Unexpected& operator=(Unexpected&&) = default;
-  FOLLY_COLD constexpr /* implicit */ Unexpected(const Error& err)
-      : error_(err) {}
-  FOLLY_COLD constexpr /* implicit */ Unexpected(Error&& err)
-      : error_(std::move(err)) {}
+  constexpr /* implicit */ Unexpected(const Error& err) : error_(err) {}
+  constexpr /* implicit */ Unexpected(Error&& err) : error_(std::move(err)) {}
 
   template <class Other FOLLY_REQUIRES_TRAILING(
       std::is_constructible<Error, Other&&>::value)>

folly/Makefile.am

@@ -313,6 +313,7 @@ nobase_follyinclude_HEADERS = \
 	io/async/test/Util.h \
 	json.h \
 	lang/Assume.h \
+	lang/ColdClass.h \
 	lang/Launder.h \
 	lang/RValueReferenceWrapper.h \
 	lang/SafeAssert.h \
@@ -566,6 +567,7 @@ libfolly_la_SOURCES = \
 	io/async/ssl/SSLErrors.cpp \
 	json.cpp \
 	lang/Assume.cpp \
+	lang/ColdClass.cpp \
 	lang/SafeAssert.cpp \
 	detail/MemoryIdler.cpp \
 	detail/SocketFastOpen.cpp \

folly/lang/ColdClass.cpp

+/*
+ * Copyright 2004-present Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "ColdClass.h"
+
+folly::cold_detail::ColdClass::ColdClass() noexcept {}

folly/lang/ColdClass.h

+/*
+ * Copyright 2004-present Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* Tag any class as `cold` by inheriting from folly::cold::ColdClass
+ *
+ * Intended use: things like folly::Unexpected which are designed to only be
+ * instantiated on error paths.
+ */
+#pragma once
+
+#include <folly/CppAttributes.h>
+
+namespace folly {
+// ColdClass should be in its own namespace: inheriting from any class adds its
+// innermost namespace to the namespaces inspected during
+// argument-dependent-lookoup. We want people to be able to derive from this
+// without implicitly picking up the folly namespace for ADL on their classes.
+namespace cold_detail {
+struct ColdClass {
+  FOLLY_COLD ColdClass() noexcept;
+};
+} // namespace cold
+
+/* using override */ using cold_detail::ColdClass;
+} // namespace folly

folly/lang/test/ColdClassTest.cpp

+/*
+ * Copyright 2004-present Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <folly/lang/ColdClass.h>
+
+#include <folly/portability/GTest.h>
+#include <type_traits>
+
+using folly::ColdClass;
+
+TEST(ColdClass, inheritance) {
+  // The only verifiable property of ColdClass is that it must not disrupt the
+  // default constructor/destructor, default copy/move constructors and default
+  // copy/move assignment operators when a class derives from it.
+  struct TestStruct : ColdClass {};
+  EXPECT_TRUE(std::is_nothrow_default_constructible<TestStruct>::value);
+  EXPECT_TRUE(std::is_trivially_copy_constructible<TestStruct>::value);
+  EXPECT_TRUE(std::is_trivially_move_constructible<TestStruct>::value);
+  EXPECT_TRUE(std::is_trivially_copy_assignable<TestStruct>::value);
+  EXPECT_TRUE(std::is_trivially_move_assignable<TestStruct>::value);
+  EXPECT_TRUE(std::is_trivially_destructible<TestStruct>::value);
+  // Same again, but private inheritance. Should make no difference.
+  class TestClass : ColdClass {};
+  EXPECT_TRUE(std::is_nothrow_default_constructible<TestClass>::value);
+  EXPECT_TRUE(std::is_trivially_copy_constructible<TestClass>::value);
+  EXPECT_TRUE(std::is_trivially_move_constructible<TestClass>::value);
+  EXPECT_TRUE(std::is_trivially_copy_assignable<TestClass>::value);
+  EXPECT_TRUE(std::is_trivially_move_assignable<TestClass>::value);
+  EXPECT_TRUE(std::is_trivially_destructible<TestClass>::value);
+}