From patchwork Mon Feb  8 21:13:23 2016
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [RFC] RFC: allow optional C++ 11 extensions with pybind11 for
 performance code
From: Laurent Charignon <lcharignon@fb.com>
X-Patchwork-Id: 13055
Message-Id: <3c1ae9c7e93e8556fa74.1454966003@lcharignon-mbp.local>
To: <mercurial-devel@mercurial-scm.org>
Date: Mon, 8 Feb 2016 13:13:23 -0800

# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1454965979 28800
#      Mon Feb 08 13:12:59 2016 -0800
# Branch stable
# Node ID 3c1ae9c7e93e8556fa7470919108a02bd989d040
# Parent  61f4d59e9a0be4e25c1aa016db1a80a540a9d337
RFC: allow optional C++ 11 extensions with pybind11 for performance code

This is a proposal to allow us to write C++ 11 extensions (in addition to C89),
for optional performance code.

According to Augie, lazymanifest was a large undertaking and it would have been
much easier to implement it in C++.
And, as I plan to write a native version of tree manifest, I would like to
introduce C++ extensions beforehand and write the native tree manifest in C++.

Like our current C modules, C++ modules would be optional and have Python
fallbacks for performance purposes.

I propose to use pybind11 to do that.
pybind11's website (https://pybind11.readthedocs.org/en/latest/index.html)
describes it as follows:
"pybind11 is a lightweight header-only library that exposes C++ types in
Python and vice versa, mainly to create Python bindings of existing C++ code"

This patch contains a change to setup.py to optional compile C++ modules (I just
put a test module there). The only change needed to our codebase is in util.h
as we cannot redefine the "bool" type if we are using C++.

As you can see in the example, not having to convert the arguments and return
values between python and native code would save us a lot of lines of code and
complexity, making native code easier to review.

The following example works with the patch that I sent:

    >>> from mercurial import pybindtest
    >>> print pybindtest.add(40, 2)
        42

diff --git a/mercurial/pybindtest.cpp b/mercurial/pybindtest.cpp
new file mode 100644
--- /dev/null
+++ b/mercurial/pybindtest.cpp
@@ -0,0 +1,15 @@
+#include <pybind11/pybind11.h>
+#include "util.h"
+
+int add(int i, int j) {
+    return i + j;
+}
+
+namespace py = pybind11;
+
+PYBIND11_PLUGIN(pybindtest) {
+    py::module m("pybindtest", "pybind11 example plugin");
+    m.def("add", &add, "A function which adds two numbers");
+
+    return m.ptr();
+}
diff --git a/mercurial/util.h b/mercurial/util.h
--- a/mercurial/util.h
+++ b/mercurial/util.h
@@ -157,6 +157,8 @@ enum normcase_spec {
 };
 
 #define MIN(a, b) (((a)<(b))?(a):(b))
+/* C++ defines bool, we don't want to redefine it */
+#ifndef __cplusplus
 /* VC9 doesn't include bool and lacks stdbool.h based on my searching */
 #if defined(_MSC_VER) || __STDC_VERSION__ < 199901L
 #define true 1
@@ -165,5 +167,6 @@ typedef unsigned char bool;
 #else
 #include <stdbool.h>
 #endif
+#endif /* __cplusplus */
 
 #endif /* _HG_UTIL_H_ */
diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -610,6 +610,16 @@ datafiles = []
 setupversion = version
 extra = {}
 
+pybindpath = os.environ.get("HGPYBIND11INCLUDEPATH", None)
+if pybindpath:
+    cppext = [
+              Extension('mercurial.pybindtest', ['mercurial/pybindtest.cpp',],
+              language="c++",
+              include_dirs = [pybindpath],
+              extra_compile_args=['-std=c++11'])
+    ]
+    extmodules.extend(cppext)
+
 if py2exeloaded:
     extra['console'] = [
         {'script':'hg',