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

fix compile problem on g++ 4.8.2 on os x #54

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@smw

smw commented May 12, 2014

I had to apply this patch to optional.hpp to get it to compile under Gnu g++ 4.8.2 on OS X. The source file seems to be from Boost, so I figure it's possible I'm just doing something wrong?

Here's the error I was getting:

cd /Users/smw/dev/mag/magnum/build-fail/src/Magnum && /usr/local/bin/g++-4.8   -DMagnum_EXPORTS -std=c++0x -fno-strict-aliasing -Wall -Wextra -Wold-style-cast -Winit-self -Werror=return-type -Wmissing-declarations -pedantic -fvisibility=hidden -Wzero-as-null-pointer-constant -Wdouble-promotion -fPIC -I/Users/smw/dev/mag/magnum/src -I/Users/smw/dev/mag/magnum/build-fail/src -I/usr/local/magnum/include -I/Corrade -I/Users/smw/dev/mag/magnum/src/MagnumExternal/OpenGL    -DGLLoadGen_EXPORTS -fPIC -o CMakeFiles/Magnum.dir/Trade/AbstractImporter.cpp.o -c /Users/smw/dev/mag/magnum/src/Magnum/Trade/AbstractImporter.cpp
In file included from /Users/smw/dev/mag/magnum/src/Magnum/Trade/AbstractImporter.h:39:0,
                 from /Users/smw/dev/mag/magnum/src/Magnum/Trade/AbstractImporter.cpp:26:
/Users/smw/dev/mag/magnum/src/MagnumExternal/Optional/optional.hpp: In function 'void std::fail(const char*, const char*, unsigned int)':
/Users/smw/dev/mag/magnum/src/MagnumExternal/Optional/optional.hpp:157:29: error: '_assert' was not declared in this scope
     _assert(expr, file, line);
                             ^
make[2]: *** [src/Magnum/CMakeFiles/Magnum.dir/Trade/AbstractImporter.cpp.o] Error 1
make[1]: *** [src/Magnum/CMakeFiles/Magnum.dir/all] Error 2
make: *** [all] Error 2

CMake like this:

2.0.0 foo:~/dev/mag/magnum/build-fail master $ export CC=/usr/local/bin/gcc-4.8
2.0.0 foo:~/dev/mag/magnum/build-fail master $ export CXX+=/usr/local/bin/g++-4.8
2.0.0 foo:~/dev/mag/magnum/build-fail master $ cmake -DCMAKE_INSTALL_PREFIX=/usr/local/magnum ..
@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra May 13, 2014

Owner

The file is an proposal for std::optional object, planned for C++1y, originally from https://github.com/akrzemi1/Optional, my fork at https://github.com/mosra/optional. It looks like every platform on Earth defined its own assertion function for fun and I didn't cover all possible cases yet :) On OSX it surely works with Clang and apparently nobody tried it with GCC yet.

However, I'd rather not change anything that's already there to avoid breaking stuff I have no control over. Can you try the following patch, if it works for you?

diff --git a/src/MagnumExternal/Optional/optional.hpp b/src/MagnumExternal/Optional/optional.hpp
index 07db190..e837628 100644
--- a/src/MagnumExternal/Optional/optional.hpp
+++ b/src/MagnumExternal/Optional/optional.hpp
@@ -151,7 +151,7 @@ template <class T> inline constexpr typename std::remove_reference<T>::type&& co
     __assert(expr, line, file); // WHY.
   # elif defined __ANDROID__
     __assert(file, line, expr);
-  # elif defined __clang__ || defined __GNU_LIBRARY__
+  # elif defined __clang__ || defined __GNU_LIBRARY__ || (defined __GNUC__ && defined __APPLE__)
     __assert(expr, file, line);
   # elif defined __GNUC__
     _assert(expr, file, line);
Owner

mosra commented May 13, 2014

The file is an proposal for std::optional object, planned for C++1y, originally from https://github.com/akrzemi1/Optional, my fork at https://github.com/mosra/optional. It looks like every platform on Earth defined its own assertion function for fun and I didn't cover all possible cases yet :) On OSX it surely works with Clang and apparently nobody tried it with GCC yet.

However, I'd rather not change anything that's already there to avoid breaking stuff I have no control over. Can you try the following patch, if it works for you?

diff --git a/src/MagnumExternal/Optional/optional.hpp b/src/MagnumExternal/Optional/optional.hpp
index 07db190..e837628 100644
--- a/src/MagnumExternal/Optional/optional.hpp
+++ b/src/MagnumExternal/Optional/optional.hpp
@@ -151,7 +151,7 @@ template <class T> inline constexpr typename std::remove_reference<T>::type&& co
     __assert(expr, line, file); // WHY.
   # elif defined __ANDROID__
     __assert(file, line, expr);
-  # elif defined __clang__ || defined __GNU_LIBRARY__
+  # elif defined __clang__ || defined __GNU_LIBRARY__ || (defined __GNUC__ && defined __APPLE__)
     __assert(expr, file, line);
   # elif defined __GNUC__
     _assert(expr, file, line);
@smw

This comment has been minimized.

Show comment
Hide comment
@smw

smw May 13, 2014

Patch works just fine.

smw commented May 13, 2014

Patch works just fine.

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra May 13, 2014

Owner

Fixed in 257426d. Thanks! :)

Owner

mosra commented May 13, 2014

Fixed in 257426d. Thanks! :)

@mosra mosra closed this May 13, 2014

@mosra mosra added bug labels Jul 22, 2014

@mosra mosra added this to the 2014.06 milestone Feb 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment