Skip to content
This repository was archived by the owner on Jan 7, 2023. It is now read-only.

Conversation

xenoscopic
Copy link
Contributor

Two header files (Column.h and BetterChain.h) contained 'using namespace
std;' directives. Not only is this bad practice for C++ header files,
but it also causes compatibility issues with Cython-generated C++ code
when compiling against libc++ (e.g. on Mac OS X 10.9). The issue arises
due to Cython utility functions using non-namespaced C stdlib functions
(e.g. isspace). When these two root_numpy headers were included, they
caused an amiguity error by exposing 's std::isspace method.
This seems to not have been an issue with GNU libstdc++, but libc++'s
more strict adherence to standards did not like this.

The solution here is simply to prefix all std::-namespace symbols in
these two headers and remove the 'using namespace std;' directives.

One might argue that Cython should std::-prefix their function use, but
the reality is that their routines still need to support C, and their
routines do work if users don't pull in the entire std:: C++ namespace,
so I don't think this is worth commenting about upstream.

Two header files (Column.h and BetterChain.h) contained 'using namespace
std;' directives.  Not only is this bad practice for C++ *header* files,
but it also causes compatibility issues with Cython-generated C++ code
when compiling against libc++ (e.g. on Mac OS X 10.9).  The issue arises
due to Cython utility functions using non-namespaced C stdlib functions
(e.g. isspace).  When these two root_numpy headers were included, they
caused an amiguity error by exposing <cctype>'s std::isspace method.
This seems to not have been an issue with GNU libstdc++, but libc++'s
more strict adherence to standards did not like this.

The solution here is simply to prefix all std::-namespace symbols in
these two headers and remove the 'using namespace std;' directives.

One might argue that Cython should std::-prefix their function use, but
the reality is that their routines still need to support C, and their
routines do work if users don't pull in the entire std:: C++ namespace,
so I don't think this is worth commenting about upstream.
@ndawe
Copy link
Member

ndawe commented Nov 6, 2013

Thanks a lot for this fix! isspace was definitely the error in the thread on the email list. I'm merging this in now.

ndawe added a commit that referenced this pull request Nov 6, 2013
Fix compatibility with root_numpy and libc++.
@ndawe ndawe merged commit 2d7ebe0 into scikit-hep:master Nov 6, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants