-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Python bindings don't compile with Clang #143
Comments
Cherry pick
pybind/pybind11@dc7bb5d
onto upstream pybind11.
Reason for fork is this is not included in upstream (for obvious reasons)
plus several bug fixes
…On Fri, Sep 8, 2023, 10:46 AM Robert Adam ***@***.***> wrote:
When using clang to compile SeQuant, the Python bindings (enabled via the
SEQUANT_PYTHON option) fail to compile with the error
In file included from /Data/home/user/Documents/Git/SeQuant/python/src/sequant/_sequant.cc:12:
In file included from /Data/home/user/Documents/Git/SeQuant/python/src/sequant/mbpt.h:11:
In file included from /Data/home/user/Documents/Git/SeQuant/python/src/sequant/python.h:4:
In file included from /Data/home/user/Documents/Git/SeQuant/build_clang/_deps/pybind11-src/include/pybind11/boost/container/small_vector.h:3:
In file included from /Data/home/user/Documents/Git/SeQuant/build_clang/_deps/pybind11-src/include/pybind11/boost/container/../../stl.h:12:
/Data/home/user/Documents/Git/SeQuant/build_clang/_deps/pybind11-src/include/pybind11/boost/container/../../pybind11.h:1010:9: error: no matching function for call to 'operator delete'
::operator delete(p, s);
There is an upstream bug report
<pybind/pybind11#1604> about this, that
indicates that this has been fixed in pybind version >= 2.4.4.
However, you are using a custom fork and within that you seem to be on
version 2.4.3, which still contains this error.
I tried the quick fix of pointing to version 2.5.0 in your fork, but then
an include to something like pybind11/boost/container/small_vector fails
to be resolved (from SeQuant's files). Thus, I am not entirely sure how you
would want to proceed with this. I was also wondering why you were using a
fork to begin with. It doesn't seem like you have made any kind of patches
to pybind11 🤔
—
Reply to this email directly, view it on GitHub
<#143>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABH62KGBRPCFK7LWX6Q5AH3XZMOUDANCNFSM6AAAAAA4QNJX3E>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Thanks! I just saw that you have the master branch unpatched, but have your patches located in https://github.com/ValeevGroup/pybind11/tree/ValeevGroup/v2.4.3. That's why I thought your fork was unchanged. Anyway, I don't yet need Python bindings - it's just something I stumbled upon and thought I'd let you know. It seems a bit more complicated for me to first update your fork via PR and then submit a PR to this repo once the fork as been updated, than if someone of you (with write access to the repos) patched this. |
I ll fix it myself, next week. busy could days
…On Fri, Sep 8, 2023, 11:04 AM Robert Adam ***@***.***> wrote:
Thanks! I just saw that you have the master branch unpatched, but have
your patches located in
https://github.com/ValeevGroup/pybind11/tree/ValeevGroup/v2.4.3. That's
why I thought your fork was unchanged.
Anyway, I don't yet need Python bindings - it's just something I stumbled
upon and thought I'd let you know. It seems a bit more complicated for me
to first update your fork via PR and then submit a PR to this repo once the
fork as been updated, than if someone of you (with write access to the
repos) patched this.
If you want me to create those PRs instead though, just let me know.
—
Reply to this email directly, view it on GitHub
<#143 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABH62KBXDJBUN6Q4DF7QOITXZMQW5ANCNFSM6AAAAAA4QNJX3E>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@asadchev would it make sense to add pybind/pybind11@dc7bb5d to SeQuant directly? |
Let me look next week. Patches were necessary but should be easy to port
and one maybe already in pybind tree
…On Wed, Nov 8, 2023, 11:52 AM Eduard Valeyev ***@***.***> wrote:
@asadchev <https://github.com/asadchev> would it make sense to add
***@***.***
<pybind/pybind11@dc7bb5d>
to SeQuant directly?
—
Reply to this email directly, view it on GitHub
<#143 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABH62KFVKCQ7R6C5JQRMTITYDPPIPAVCNFSM6AAAAAA4QNJX3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBSGU2TEOJUG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@Krzmbrzl switching to https://github.com/ValeevGroup/pybind11/tree/ValeevGroup/v2.11 should help |
When using
clang
to compile SeQuant, the Python bindings (enabled via theSEQUANT_PYTHON
option) fail to compile with the errorThere is an upstream bug report about this, that indicates that this has been fixed in pybind version >= 2.4.4.
However, you are using a custom fork and within that you seem to be on version 2.4.3, which still contains this error.
I tried the quick fix of pointing to version 2.5.0 in your fork, but then an include to something like
pybind11/boost/container/small_vector
fails to be resolved (from SeQuant's files). Thus, I am not entirely sure how you would want to proceed with this. I was also wondering why you were using a fork to begin with. It doesn't seem like you have made any kind of patches to pybind11 🤔The text was updated successfully, but these errors were encountered: