Remove folly's ColdClass, which was of dubious utility

Summary: `folly::cold_detail::ColdClass` was marking things (like `folly::Unexpected`) cold, but at the cost of inhibiting the inliner from doing its job. This is leading to bad codegen, which offsets any small wins we mind get for the `cold` attribute.

Reviewed By: yfeldblum

Differential Revision: D19324159

fbshipit-source-id: 7ed431b6c9d6e963c3bf438c707fa6cf6a38bf9d

CMakeLists.txt

@@ -745,7 +745,6 @@ if (BUILD_TESTS)
       TEST bits_test SOURCES BitsTest.cpp
       TEST c_string_test SOURCES CStringTest.cpp
       TEST cast_test SOURCES CastTest.cpp
-      TEST cold_class_test SOURCES ColdClassTest.cpp
       TEST safe_assert_test SOURCES SafeAssertTest.cpp
 
     DIRECTORY memory/test/

folly/Expected.h

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

folly/lang/ColdClass.cpp

-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * 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>
-
-folly::cold_detail::ColdClass::ColdClass() noexcept = default;

folly/lang/ColdClass.h

-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * 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_detail
-
-/* using override */ using cold_detail::ColdClass;
-} // namespace folly

folly/lang/test/ColdClassTest.cpp

-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * 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;
-
-template <class TestClass>
-static void validateInheritedClass() {
-  // 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.
-  EXPECT_TRUE(std::is_nothrow_default_constructible<TestClass>::value);
-#if !defined(__GLIBCXX__) || __GNUC__ >= 5
-  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);
-#endif
-  EXPECT_TRUE(std::is_nothrow_copy_constructible<TestClass>::value);
-  EXPECT_TRUE(std::is_nothrow_move_constructible<TestClass>::value);
-  EXPECT_TRUE(std::is_nothrow_copy_assignable<TestClass>::value);
-  EXPECT_TRUE(std::is_nothrow_move_assignable<TestClass>::value);
-  EXPECT_TRUE(std::is_trivially_destructible<TestClass>::value);
-}
-
-TEST(ColdClassTest, publicInheritance) {
-  struct TestPublic : ColdClass {};
-  validateInheritedClass<TestPublic>();
-}
-
-TEST(ColdClassTest, protectedInheritance) {
-  // Same again, but protected inheritance. Should make no difference.
-  class TestProtected : protected ColdClass {};
-  validateInheritedClass<TestProtected>();
-}
-
-TEST(ColdClassTest, privateInheritance) {
-  // Same again, but private inheritance. Should make no difference.
-  class TestPrivate : ColdClass {};
-  validateInheritedClass<TestPrivate>();
-}