From 2b4477eb65d9e852da9cad1d163dde277d5b3c2e Mon Sep 17 00:00:00 2001
From: Dean Moldovan <dean0x7d@gmail.com>
Date: Sat, 9 Sep 2017 20:21:34 +0200
Subject: [PATCH] Make TypeErrors more informative when an optional header is
 missing

E.g. trying to convert a `list` to a `std::vector<int>` without
including <pybind11/stl.h> will now raise an error with a note that
suggests checking the headers.

The note is only appended if `std::` is found in the function
signature. This should only be the case when a header is missing.
E.g. when stl.h is included, the signature would contain `List[int]`
instead of `std::vector<int>` while using stl_bind.h would produce
something like `MyVector`. Similarly for `std::map`/`Dict`, `complex`,
`std::function`/`Callable`, etc.

There's a possibility for false positives, but it's pretty low.
---
 docs/changelog.rst                    |  4 ++++
 include/pybind11/pybind11.h           | 12 ++++++++++++
 tests/CMakeLists.txt                  |  1 +
 tests/pybind11_cross_module_tests.cpp |  6 ++++++
 tests/test_stl.py                     | 19 +++++++++++++++++++
 5 files changed, 42 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index eea15d548..1015f0c71 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -35,6 +35,10 @@ v2.2.1 (Not yet released)
   that's only registered in an external module.
   `#1058 <https://github.com/pybind/pybind11/pull/1058>`_.
 
+* Conversion errors now try to be more informative when it's likely that
+  a missing header is the cause (e.g. forgetting ``<pybind11/stl.h>``).
+  `#1077 <https://github.com/pybind/pybind11/pull/1077>`_.
+
 v2.2.0 (August 31, 2017)
 -----------------------------------------------------
 
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 80102c5e8..613135a7a 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -694,6 +694,16 @@ protected:
             return nullptr;
         }
 
+        auto append_note_if_missing_header_is_suspected = [](std::string &msg) {
+            if (msg.find("std::") != std::string::npos) {
+                msg += "\n\n"
+                       "Did you forget to `#include <pybind11/stl.h>`? Or <pybind11/complex.h>,\n"
+                       "<pybind11/functional.h>, <pybind11/chrono.h>, etc. Some automatic\n"
+                       "conversions are optional and require extra headers to be included\n"
+                       "when compiling your pybind11 module.";
+            }
+        };
+
         if (result.ptr() == PYBIND11_TRY_NEXT_OVERLOAD) {
             if (overloads->is_operator)
                 return handle(Py_NotImplemented).inc_ref().ptr();
@@ -751,12 +761,14 @@ protected:
                 }
             }
 
+            append_note_if_missing_header_is_suspected(msg);
             PyErr_SetString(PyExc_TypeError, msg.c_str());
             return nullptr;
         } else if (!result) {
             std::string msg = "Unable to convert function return value to a "
                               "Python type! The signature was\n\t";
             msg += it->signature;
+            append_note_if_missing_header_is_suspected(msg);
             PyErr_SetString(PyExc_TypeError, msg.c_str());
             return nullptr;
         } else {
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index d8c53c21f..25e06662c 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -76,6 +76,7 @@ string(REPLACE ".cpp" ".py" PYBIND11_PYTEST_FILES "${PYBIND11_TEST_FILES}")
 set(PYBIND11_CROSS_MODULE_TESTS
   test_exceptions.py
   test_local_bindings.py
+  test_stl.py
   test_stl_binders.py
 )
 
diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp
index 928b25586..f705e3106 100644
--- a/tests/pybind11_cross_module_tests.cpp
+++ b/tests/pybind11_cross_module_tests.cpp
@@ -114,4 +114,10 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
     // the same module (it would be an ODR violation). Therefore `bind_vector` of `bool`
     // is defined here and tested in `test_stl_binders.py`.
     py::bind_vector<std::vector<bool>>(m, "VectorBool");
+
+    // test_missing_header_message
+    // The main module already includes stl.h, but we need to test the error message
+    // which appears when this header is missing.
+    m.def("missing_header_arg", [](std::vector<float>) { });
+    m.def("missing_header_return", []() { return std::vector<float>(); });
 }
diff --git a/tests/test_stl.py b/tests/test_stl.py
index f04eaeb86..db8515e7a 100644
--- a/tests/test_stl.py
+++ b/tests/test_stl.py
@@ -179,3 +179,22 @@ def test_stl_pass_by_pointer(msg):
     """  # noqa: E501 line too long
 
     assert m.stl_pass_by_pointer([1, 2, 3]) == [1, 2, 3]
+
+
+def test_missing_header_message():
+    """Trying convert `list` to a `std::vector`, or vice versa, without including
+    <pybind11/stl.h> should result in a helpful suggestion in the error message"""
+    import pybind11_cross_module_tests as cm
+
+    expected_message = ("Did you forget to `#include <pybind11/stl.h>`? Or <pybind11/complex.h>,\n"
+                        "<pybind11/functional.h>, <pybind11/chrono.h>, etc. Some automatic\n"
+                        "conversions are optional and require extra headers to be included\n"
+                        "when compiling your pybind11 module.")
+
+    with pytest.raises(TypeError) as excinfo:
+        cm.missing_header_arg([1.0, 2.0, 3.0])
+    assert expected_message in str(excinfo.value)
+
+    with pytest.raises(TypeError) as excinfo:
+        cm.missing_header_return()
+    assert expected_message in str(excinfo.value)