PYBIND11_NOINLINE-related cleanup. (#3179)

* Removing pragma for GCC -Wattributes, fixing forward declarations.

* Introducing PYBIND11_NOINLINE_FWD to deal with CUDA, GCC7, GCC8.

* Updating PYBIND11_NOINLINE_DCL in Doxyfile.

* Trying noinline, noinline for {CUDA, GCC7, GCC8}

* Trying noinline, inline for {CUDA, GCC7, GCC8}

* Adding GCC -Wattributes `pragma` in 3 header files.

* Introducing PYBIND11_NOINLINE_GCC_PRAGMA_ATTRIBUTES_NEEDED, used in 9 header files.

* Removing ICC pragma 2196, to see if it is still needed.

* Trying noinline, noinline for ICC

* Trying noinline, inline for ICC

* Restoring ICC pragma 2196, introducing PYBIND11_NOINLINE_FORCED, defined for testing.

* Removing code accidentally left in (was for experimentation only).

* Removing one-time-test define.

* Removing PYBIND11_NOINLINE_FWD macro (after learning that it makes no sense).

* Testing with PYBIND11_NOINLINE_DISABLED. Minor non-functional enhancements.

* Removing #define PYBIND11_NOINLINE_DISABLED (test was successful).

* Removing PYBIND11_NOINLINE_FORCED and enhancing comments for PYBIND11_NOINLINE.

* WIP stripping back

* Making -Wattributes pragma in pybind11 specific to GCC7, GCC8, CUDA.

Author: rwgk

include/pybind11/attr.h

@@ -124,7 +124,7 @@ enum op_id : int;
 enum op_type : int;
 struct undefined_t;
 template <op_id id, op_type ot, typename L = undefined_t, typename R = undefined_t> struct op_;
-inline void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret);
+void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret);
 
 /// Internal data structure which holds metadata about a keyword argument
 struct argument_record {

include/pybind11/detail/common.h

@@ -99,10 +99,15 @@
 #  endif
 #endif
 
-#if defined(_MSC_VER)
-#  define PYBIND11_NOINLINE __declspec(noinline)
+// The PYBIND11_NOINLINE macro is for function DEFINITIONS.
+// In contrast, FORWARD DECLARATIONS should never use this macro:
+// https://stackoverflow.com/questions/9317473/forward-declaration-of-inline-functions
+#if defined(PYBIND11_NOINLINE_DISABLED) // Option for maximum portability and experimentation.
+#  define PYBIND11_NOINLINE inline
+#elif defined(_MSC_VER)
+#  define PYBIND11_NOINLINE __declspec(noinline) inline
 #else
-#  define PYBIND11_NOINLINE __attribute__ ((noinline))
+#  define PYBIND11_NOINLINE __attribute__ ((noinline)) inline
 #endif
 
 #if defined(__MINGW32__)
@@ -778,8 +783,8 @@ PYBIND11_RUNTIME_EXCEPTION(import_error, PyExc_ImportError)
 PYBIND11_RUNTIME_EXCEPTION(cast_error, PyExc_RuntimeError) /// Thrown when pybind11::cast or handle::call fail due to a type casting error
 PYBIND11_RUNTIME_EXCEPTION(reference_cast_error, PyExc_RuntimeError) /// Used internally
 
-[[noreturn]] PYBIND11_NOINLINE inline void pybind11_fail(const char *reason) { throw std::runtime_error(reason); }
-[[noreturn]] PYBIND11_NOINLINE inline void pybind11_fail(const std::string &reason) { throw std::runtime_error(reason); }
+[[noreturn]] PYBIND11_NOINLINE void pybind11_fail(const char *reason) { throw std::runtime_error(reason); }
+[[noreturn]] PYBIND11_NOINLINE void pybind11_fail(const std::string &reason) { throw std::runtime_error(reason); }
 
 template <typename T, typename SFINAE = void> struct format_descriptor { };
 

include/pybind11/detail/internals.h

@@ -257,7 +257,7 @@ inline void translate_local_exception(std::exception_ptr p) {
 #endif
 
 /// Return a reference to the current `internals` data
-PYBIND11_NOINLINE inline internals &get_internals() {
+PYBIND11_NOINLINE internals &get_internals() {
     auto **&internals_pp = get_internals_pp();
     if (internals_pp && *internals_pp)
         return **internals_pp;
@@ -352,14 +352,14 @@ PYBIND11_NAMESPACE_END(detail)
 /// Returns a named pointer that is shared among all extension modules (using the same
 /// pybind11 version) running in the current interpreter. Names starting with underscores
 /// are reserved for internal usage. Returns `nullptr` if no matching entry was found.
-inline PYBIND11_NOINLINE void *get_shared_data(const std::string &name) {
+PYBIND11_NOINLINE void *get_shared_data(const std::string &name) {
     auto &internals = detail::get_internals();
     auto it = internals.shared_data.find(name);
     return it != internals.shared_data.end() ? it->second : nullptr;
 }
 
 /// Set the shared data that can be later recovered by `get_shared_data()`.
-inline PYBIND11_NOINLINE void *set_shared_data(const std::string &name, void *data) {
+PYBIND11_NOINLINE void *set_shared_data(const std::string &name, void *data) {
     detail::get_internals().shared_data[name] = data;
     return data;
 }

include/pybind11/detail/type_caster_base.h

@@ -81,7 +81,7 @@ class loader_life_support {
 inline std::pair<decltype(internals::registered_types_py)::iterator, bool> all_type_info_get_cache(PyTypeObject *type);
 
 // Populates a just-created cache entry.
-PYBIND11_NOINLINE inline void all_type_info_populate(PyTypeObject *t, std::vector<type_info *> &bases) {
+PYBIND11_NOINLINE void all_type_info_populate(PyTypeObject *t, std::vector<type_info *> &bases) {
     std::vector<PyTypeObject *> check;
     for (handle parent : reinterpret_borrow<tuple>(t->tp_bases))
         check.push_back((PyTypeObject *) parent.ptr());
@@ -150,7 +150,7 @@ inline const std::vector<detail::type_info *> &all_type_info(PyTypeObject *type)
  * ancestors are pybind11-registered.  Throws an exception if there are multiple bases--use
  * `all_type_info` instead if you want to support multiple bases.
  */
-PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type) {
+PYBIND11_NOINLINE detail::type_info* get_type_info(PyTypeObject *type) {
     auto &bases = all_type_info(type);
     if (bases.empty())
         return nullptr;
@@ -176,7 +176,7 @@ inline detail::type_info *get_global_type_info(const std::type_index &tp) {
 }
 
 /// Return the type info for a given C++ type; on lookup failure can either throw or return nullptr.
-PYBIND11_NOINLINE inline detail::type_info *get_type_info(const std::type_index &tp,
+PYBIND11_NOINLINE detail::type_info *get_type_info(const std::type_index &tp,
                                                           bool throw_if_missing = false) {
     if (auto ltype = get_local_type_info(tp))
         return ltype;
@@ -191,13 +191,13 @@ PYBIND11_NOINLINE inline detail::type_info *get_type_info(const std::type_index
     return nullptr;
 }
 
-PYBIND11_NOINLINE inline handle get_type_handle(const std::type_info &tp, bool throw_if_missing) {
+PYBIND11_NOINLINE handle get_type_handle(const std::type_info &tp, bool throw_if_missing) {
     detail::type_info *type_info = get_type_info(tp, throw_if_missing);
     return handle(type_info ? ((PyObject *) type_info->type) : nullptr);
 }
 
 // Searches the inheritance graph for a registered Python instance, using all_type_info().
-PYBIND11_NOINLINE inline handle find_registered_python_instance(void *src,
+PYBIND11_NOINLINE handle find_registered_python_instance(void *src,
                                                                 const detail::type_info *tinfo) {
     auto it_instances = get_internals().registered_instances.equal_range(src);
     for (auto it_i = it_instances.first; it_i != it_instances.second; ++it_i) {
@@ -325,7 +325,7 @@ struct values_and_holders {
  * The returned object should be short-lived: in particular, it must not outlive the called-upon
  * instance.
  */
-PYBIND11_NOINLINE inline value_and_holder instance::get_value_and_holder(const type_info *find_type /*= nullptr default in common.h*/, bool throw_if_missing /*= true in common.h*/) {
+PYBIND11_NOINLINE value_and_holder instance::get_value_and_holder(const type_info *find_type /*= nullptr default in common.h*/, bool throw_if_missing /*= true in common.h*/) {
     // Optimize common case:
     if (!find_type || Py_TYPE(this) == find_type->type)
         return value_and_holder(this, find_type, 0, 0);
@@ -349,7 +349,7 @@ PYBIND11_NOINLINE inline value_and_holder instance::get_value_and_holder(const t
 #endif
 }
 
-PYBIND11_NOINLINE inline void instance::allocate_layout() {
+PYBIND11_NOINLINE void instance::allocate_layout() {
     auto &tinfo = all_type_info(Py_TYPE(this));
 
     const size_t n_types = tinfo.size();
@@ -397,19 +397,19 @@ PYBIND11_NOINLINE inline void instance::allocate_layout() {
     owned = true;
 }
 
-PYBIND11_NOINLINE inline void instance::deallocate_layout() const {
+PYBIND11_NOINLINE void instance::deallocate_layout() const {
     if (!simple_layout)
         PyMem_Free(nonsimple.values_and_holders);
 }
 
-PYBIND11_NOINLINE inline bool isinstance_generic(handle obj, const std::type_info &tp) {
+PYBIND11_NOINLINE bool isinstance_generic(handle obj, const std::type_info &tp) {
     handle type = detail::get_type_handle(tp, false);
     if (!type)
         return false;
     return isinstance(obj, type);
 }
 
-PYBIND11_NOINLINE inline std::string error_string() {
+PYBIND11_NOINLINE std::string error_string() {
     if (!PyErr_Occurred()) {
         PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred");
         return "Unknown internal error occurred";
@@ -456,7 +456,7 @@ PYBIND11_NOINLINE inline std::string error_string() {
     return errorString;
 }
 
-PYBIND11_NOINLINE inline handle get_object_handle(const void *ptr, const detail::type_info *type ) {
+PYBIND11_NOINLINE handle get_object_handle(const void *ptr, const detail::type_info *type ) {
     auto &instances = get_internals().registered_instances;
     auto range = instances.equal_range(ptr);
     for (auto it = range.first; it != range.second; ++it) {
@@ -483,7 +483,7 @@ inline PyThreadState *get_thread_state_unchecked() {
 }
 
 // Forward declarations
-inline void keep_alive_impl(handle nurse, handle patient);
+void keep_alive_impl(handle nurse, handle patient);
 inline PyObject *make_new_instance(PyTypeObject *type);
 
 class type_caster_generic {

include/pybind11/detail/typeid.h

@@ -29,7 +29,7 @@ inline void erase_all(std::string &string, const std::string &search) {
     }
 }
 
-PYBIND11_NOINLINE inline void clean_type_id(std::string &name) {
+PYBIND11_NOINLINE void clean_type_id(std::string &name) {
 #if defined(__GNUG__)
     int status = 0;
     std::unique_ptr<char, void (*)(void *)> res {

include/pybind11/numpy.h

@@ -104,7 +104,7 @@ struct numpy_internals {
     }
 };
 
-inline PYBIND11_NOINLINE void load_numpy_internals(numpy_internals* &ptr) {
+PYBIND11_NOINLINE void load_numpy_internals(numpy_internals* &ptr) {
     ptr = &get_or_create_shared_data<numpy_internals>("_numpy_internals");
 }
 
@@ -1110,7 +1110,7 @@ struct field_descriptor {
     dtype descr;
 };
 
-inline PYBIND11_NOINLINE void register_structured_dtype(
+PYBIND11_NOINLINE void register_structured_dtype(
     any_container<field_descriptor> fields,
     const std::type_info& tinfo, ssize_t itemsize,
     bool (*direct_converter)(PyObject *, void *&)) {

include/pybind11/pybind11.h

@@ -10,7 +10,7 @@
 
 #pragma once
 
-#if defined(__GNUG__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
+#if defined(__CUDACC__) || (defined(__GNUC__) && (__GNUC__ == 7 || __GNUC__ == 8))
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wattributes"
 #endif
@@ -1875,7 +1875,7 @@ template <typename Type> class enum_ : public class_<Type> {
 PYBIND11_NAMESPACE_BEGIN(detail)
 
 
-inline void keep_alive_impl(handle nurse, handle patient) {
+PYBIND11_NOINLINE void keep_alive_impl(handle nurse, handle patient) {
     if (!nurse || !patient)
         pybind11_fail("Could not activate keep_alive!");
 
@@ -1902,7 +1902,7 @@ inline void keep_alive_impl(handle nurse, handle patient) {
     }
 }
 
-PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret) {
+PYBIND11_NOINLINE void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret) {
     auto get_arg = [&](size_t n) {
         if (n == 0)
             return ret;
@@ -2152,7 +2152,7 @@ exception<CppException> &register_local_exception(handle scope,
 }
 
 PYBIND11_NAMESPACE_BEGIN(detail)
-PYBIND11_NOINLINE inline void print(const tuple &args, const dict &kwargs) {
+PYBIND11_NOINLINE void print(const tuple &args, const dict &kwargs) {
     auto strings = tuple(args.size());
     for (size_t i = 0; i < args.size(); ++i) {
         strings[i] = str(args[i]);
@@ -2384,6 +2384,6 @@ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
 #    pragma GCC diagnostic pop // -Wnoexcept-type
 #endif
 
-#if defined(__GNUG__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
+#if defined(__CUDACC__) || (defined(__GNUC__) && (__GNUC__ == 7 || __GNUC__ == 8))
 #  pragma GCC diagnostic pop
 #endif

include/pybind11/pytypes.h

@@ -24,7 +24,7 @@ struct arg; struct arg_v;
 
 PYBIND11_NAMESPACE_BEGIN(detail)
 class args_proxy;
-inline bool isinstance_generic(handle obj, const std::type_info &tp);
+bool isinstance_generic(handle obj, const std::type_info &tp);
 
 // Accessor forward declarations
 template <typename Policy> class accessor;
@@ -316,7 +316,7 @@ template <typename T> T reinterpret_borrow(handle h) { return {h, object::borrow
 template <typename T> T reinterpret_steal(handle h) { return {h, object::stolen_t{}}; }
 
 PYBIND11_NAMESPACE_BEGIN(detail)
-inline std::string error_string();
+std::string error_string();
 PYBIND11_NAMESPACE_END(detail)
 
 #if defined(_MSC_VER)