From 202b6050ad6bfeb90eea4246a08123701a43b4e0 Mon Sep 17 00:00:00 2001
From: Henry Schreiner <henryschreineriii@gmail.com>
Date: Tue, 26 Oct 2021 12:56:11 -0400
Subject: [PATCH] refactor: address review

---
 include/pybind11/detail/type_caster_base.h | 13 ++++++-------
 include/pybind11/pybind11.h                | 11 ++++++-----
 tests/CMakeLists.txt                       | 21 ++++++++++++---------
 3 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h
index 2424fb00e1b..00ce1a7a1ec 100644
--- a/include/pybind11/detail/type_caster_base.h
+++ b/include/pybind11/detail/type_caster_base.h
@@ -469,19 +469,18 @@ PYBIND11_NOINLINE std::string error_string() {
         errorString += "\n\nAt:\n";
         while (frame) {
 #if PY_VERSION_HEX >= 0x03090000
-            PyCodeObject *code = PyFrame_GetCode(frame);
+            PyCodeObject *f_code = PyFrame_GetCode(frame);
 #else
-            PyCodeObject *code = frame->f_code;
+            PyCodeObject *f_code = frame->f_code;
+            Py_INCREF(f_code);
 #endif
             int lineno = PyFrame_GetLineNumber(frame);
             errorString +=
-                "  " + handle(code->co_filename).cast<std::string>() +
+                "  " + handle(f_code->co_filename).cast<std::string>() +
                 "(" + std::to_string(lineno) + "): " +
-                handle(code->co_name).cast<std::string>() + "\n";
+                handle(f_code->co_name).cast<std::string>() + "\n";
             frame = frame->f_back;
-#if PY_VERSION_HEX >= 0x03090000
-            Py_DECREF(code);
-#endif
+            Py_DECREF(f_code);
         }
     }
 #endif
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 32b1bd6afa1..bfc1c368c00 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -2339,21 +2339,22 @@ inline function get_type_override(const void *this_ptr, const type_info *this_ty
 #if PY_VERSION_HEX >= 0x03090000
     PyFrameObject *frame = PyThreadState_GetFrame(PyThreadState_Get());
     if (frame != nullptr) {
-        PyCodeObject *code = PyFrame_GetCode(frame);
-        if ((std::string) str(code->co_name) == name && code->co_argcount > 0) {
+        PyCodeObject *f_code = PyFrame_GetCode(frame);
+        // f_code is guaranteed to not be NULL
+        if ((std::string) str(f_code->co_name) == name && f_code->co_argcount > 0) {
             PyObject* locals = PyEval_GetLocals();
             if (locals != nullptr) {
                 PyObject *self_caller = dict_getitem(
-                    locals, PyTuple_GET_ITEM(code->co_varnames, 0)
+                    locals, PyTuple_GET_ITEM(f_code->co_varnames, 0)
                 );
                 if (self_caller == self.ptr()) {
-                    Py_DECREF(code);
+                    Py_DECREF(f_code);
                     Py_DECREF(frame);
                     return function();
                 }
             }
         }
-        Py_DECREF(code);
+        Py_DECREF(f_code);
         Py_DECREF(frame);
     }
 #else
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index b0ba0775228..d182b91a6c6 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -175,13 +175,13 @@ set(PYBIND11_EIGEN_REPO
     "https://gitlab.com/libeigen/eigen.git"
     CACHE STRING "Eigen repository to use for tests")
 # Always use a hash for reconfigure speed and security reasons
-set(PYBIND11_EIGEN_VERSION
-    "929bc0e191d0927b1735b9a1ddc0e8b77e3a25ec"
-    CACHE STRING "Eigen version to use for tests")
-# Pretty print the version (keep in sync)
-set(PYBIND11_EIGEN_VERSION_STRING
-    "3.4.0"
-    CACHE STRING "Eigen version string to use for tests")
+# Include the version number for pretty printing (keep in sync)
+set(PYBIND11_EIGEN_VERSION_AND_HASH
+    "3.4.0;929bc0e191d0927b1735b9a1ddc0e8b77e3a25ec"
+    CACHE STRING "Eigen version to use for tests, format: VERSION;HASH")
+
+list(GET PYBIND11_EIGEN_VERSION_AND_HASH 0 PYBIND11_EIGEN_VERSION_STRING)
+list(GET PYBIND11_EIGEN_VERSION_AND_HASH 1 PYBIND11_EIGEN_VERSION_HASH)
 
 # Check if Eigen is available; if not, remove from PYBIND11_TEST_FILES (but
 # keep it in PYBIND11_PYTEST_FILES, so that we get the "eigen is not installed"
@@ -200,11 +200,14 @@ if(PYBIND11_TEST_FILES_EIGEN_I GREATER -1)
     FetchContent_Declare(
       eigen
       GIT_REPOSITORY "${PYBIND11_EIGEN_REPO}"
-      GIT_TAG "${PYBIND11_EIGEN_VERSION}")
+      GIT_TAG "${PYBIND11_EIGEN_VERSION_AND_HASH}")
 
     FetchContent_GetProperties(eigen)
     if(NOT eigen_POPULATED)
-      message(STATUS "Downloading Eigen")
+      message(
+        STATUS
+          "Downloading Eigen ${PYBIND11_EIGEN_VERSION_STRING} (${PYBIND11_EIGEN_VERSION_HASH}) from ${PYBIND11_EIGEN_REPO}"
+      )
       FetchContent_Populate(eigen)
     endif()