Skip to content
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

Attempt to fix isfinite() checks to work with all C++11 compilers #849

Merged
merged 5 commits into from
Dec 20, 2016

Conversation

ojwb
Copy link
Member

@ojwb ojwb commented Dec 16, 2016

Fixes #615 and
#788

@wsfulton
Copy link
Member

i like it, very neat solution. There is one potential problem I can see though. If the std namespace is not defined (which it may not be with just the math.h header include), then the using namespace std; will give a compiler error. This can be solved by adding in namespace std {} into the fragment, but I suspect that SWIG unfortunately, includes enough STL headers that std is defined somewhere, so may not be a problem in practice, even with with a minimal example.

Also, I think you should add SWIGRUNTIME to SWIG_isfinite_func to avoid linking problems when using multiple modules and %import.

@ojwb
Copy link
Member Author

ojwb commented Dec 16, 2016

Good point about namespace std { }. For the second point, might be better to just make it inline, since it's just a forwarding wrapper so definitely wants to be inlined. Will address later this weekend (or feel free to push a commit to the branch).

@ojwb
Copy link
Member Author

ojwb commented Dec 19, 2016

If the std namespace is not defined (which it may not be with just the math.h header include), then the using namespace std; will give a compiler error.

FWIW, this does not actually seem to result in a compiler error (tested with GCC and clang), though clang gives a warning which we want to avoid:

$ echo 'using namespace std;'|clang++-4.0 -Wall -W -x c++ -c -o /dev/null -
<stdin>:1:17: warning: using directive refers to implicitly-defined namespace
      'std'
using namespace std;
                ^
1 warning generated.

@wsfulton
Copy link
Member

Let's go with this then. Indentation is conventionally 2 not 4 spaces though.

@ojwb ojwb merged commit 76d1aac into swig:master Dec 20, 2016
@ojwb ojwb deleted the isfinite-c++11 branch December 20, 2016 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants