eggs-cpp / variant

Eggs.Variant is a C++11/14/17 generic, type-safe, discriminated union.
http://eggs-cpp.github.io/variant/
Boost Software License 1.0
137 stars 27 forks source link

CMake build fails with unsupported language features. #17

Closed jbcoe closed 8 years ago

jbcoe commented 8 years ago

f9bcaf334bd4e0ddb7bac26fbaf3cd054be975a5 fails with unsupported language features errors like:

../include/eggs/variant/detail/pack.hpp:22:9: error: unknown type name 'constexpr'
        EGGS_CXX11_CONSTEXPR bool operator==(empty) const { return true; }

requiring C++14 fixes this but I'm not sure if that's the best way to go. (PR coming).

K-ballo commented 8 years ago

This is a header only library, there is no build step. In case you are referring to using CMake to run the test suite, that requires you to specify a standard flavor equal or greater to C++11. You may check the travis-ci configuration file to see how we run tests on a combination of compilers, compiler versions and standard versions.

You said C++11 does not sufice, I'm guessing that means you tried it and hit an error. That's likely a bug, please report it with enough information for it to be reproduced.

jbcoe commented 8 years ago

From master f9bcaf334bd4e0ddb7bac26fbaf3cd054be975a5

mkdir build
cd build
cmake ..
make

Gives a large number of C++11 related errors starting with:

../include/eggs/variant/detail/pack.hpp:22:9: error: unknown type name 'constexpr'
        EGGS_CXX11_CONSTEXPR bool operator==(empty) const { return true; }

If I add -std=c++11 to the cmake c++ compiler options using ccmake I get different compiler errors starting with:

/Users/jon/DEV/eggs.variant/test/rel.order.cpp:38:53: error: constexpr variable 'v1' must be initialized by a constant expression
            constexpr eggs::variant<int, Constexpr> v1(Constexpr(42));
                                                    ^~~~~~~~~~~~~~~~~
/Users/jon/DEV/eggs.variant/include/eggs/variant/variant.hpp:318:46: note: non-constexpr function 'forward<Constexpr>' cannot be used in a constant expression
          : _storage{detail::index<I + 1>{}, std::forward<U>(v)}

This is not blocking me in any way and I am more than happy to help with any investigation.

K-ballo commented 8 years ago

The first error is expected if your compiler's default flavor is less than C++11. Eggs.Variant supports all recent standard revisions, it's up to you or your project to pick which one you would like to use. I'll see about adding an #error message when used in C++03 mode to make this clear.

The second error sounds like a bug but I am not able to reproduce it, and it doesn't show up in travis-ci either. Please let me know which compiler and compiler version are you testing with.

jbcoe commented 8 years ago

I'm using AppleClang from Xcode 7.2.1.

K-ballo commented 8 years ago

I see, unfortunately I do not have access to that platform, but I will see if I can set up an automated tester for it.

The error is correct, this is a technically a genuine bug. In C++11, due to oversight, std::forward and std::move were not marked as constexpr. Sadly this means I will have to reimplement them as constexpr functions, or replace their uses by static_cast, but I'll bite the bullet...

K-ballo commented 8 years ago

Working on a preliminar solution at branch constexpr-forward-move. Feel fry to try it out while I figure out how to set up an AppleClang tester.

jbcoe commented 8 years ago

constexpr-forward-move branch tests build and pass.

I need to add --std=c++11 to CMAKE_CXX_FLAGS as AppleClang appears to use C++03 by default.

If you were to require an higher CMake minimum version (3.1) you could add

set (CMAKE_CXX_STANDARD 11)

to CMakeLists to avoid changes to compiler flags being needed.

K-ballo commented 8 years ago

I have pushed the fix to master, and added AppleClang builds to travis-ci to catch these kind of things in the future. Thank you for your report!

Bumping the CMake requirement would be problematic, forcing people to upgrade or install newer versions locally. I could still do it manually but that's not how it's supposed to work, the test suit shall be run with the exact flags that the user specified. After all, it's possible for the library to work under one configuration but fail under another, like in the case you have encountered. I have added an #error message to try and make the cause of the failures clearer.