pisa
pisa copied to clipboard
Replace boost::variant with std::variant
https://github.com/pisa-engine/pisa/blob/3bfbdec0da399dcb278d61d496ae348b999644fe/include/sequence/strict_sequence.hpp
https://github.com/pisa-engine/pisa/blob/3bfbdec0da399dcb278d61d496ae348b999644fe/include/sequence/indexed_sequence.hpp
Done in https://github.com/pisa-engine/pisa/pull/71 but currently blocked
I saw that "std::variant" is already in use in the project, e.g., in the warcpp code. Maybe it is ok to replace it in other parts of the code for harmonization ?
By the way, when building the code source of PISA today, I had a problem because of "std::variant". After investigation, it turns out that "std::variant" is in g++ since 7.1 and I had only g++ 5.5 (see https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.201z) so maybe it should be clearly written that we need g++7.1 to compile, whereas it is written, when we build the library:
-- GCC version must be at least 4.7!
@ybarsami We also had a problem with appleclang, which didn't implement certain functions for std::variant, hence blocking the issue. Now, for the readme, would you mind updating and creating a pull request for that?
Here are, in your project, where the message "GCC version must be at least 4.7!" appears. It is not in any README, but in the cmakelists. I don't know how you want to change that (one is a fatal error, others are only status), so change it as you like (and, if I'm pretty sure you need g++ 7.1 for warcpp, I don't know for other modules so I don't know how to update those messages anyway):
external/trecpp/CMakeLists.txt: message(STATUS "GCC version must be at least 4.7!") external/Porter2/CMakeLists.txt: message(STATUS "GCC version must be at least 4.7!") external/FastPFor/CMakeLists.txt: message(STATUS "GCC version must be at least 4.7!") external/warcpp/CMakeLists.txt: message(STATUS "GCC version must be at least 4.7!") external/QMX/CMakeLists.txt: message(STATUS "GCC version must be at least 4.7!") external/wapopp/cmake/verify_version.cmake: message(FATAL_ERROR "GCC version must be at least 4.7!") external/KrovetzStemmer/CMakeLists.txt: message(STATUS "GCC version must be at least 4.7!")
By the way you're right, it would be better if it was written in the global readme.md file, just write it wherever you think is more appropriate.
These messages are old and I'm actually removing them one of the ongoing PRs. Documentation talks about compilers https://pisa.readthedocs.io/en/latest/getting_started.html#building-the-code but isn't very clear, so I opened #259
Do you feel like it's unclear how to find this information? Maybe we should give it a separate Requirements section so it's easier to spot...
Oh, I totally saw this message on the documentations. But, from a strict logical point of view, the fact that your code compiles with the GNU compiler 7.4 does not imply that this is required (and, I could compile starting from g++ 7.1, as I said). I saw "gcc must be >= 4.7" when typing make, so I was surprised to see that there was a message, but that this message was not accurate.
Here is what I usually do in my projects, but I know it's a long, painful, process:
- whenever I use a fancy stuff from a specific compiler /language version but I can either do without it (and, e.g., lose some performance), I put the optimization under a #ifdef. e.g. for the OpenMP pragma simd, here is what I do :
#if defined(__ICC) || defined(__INTEL_COMPILER) // Intel ICC/ICPC
if (__INTEL_COMPILER_BUILD_DATE >= 20140726)
define WE_HAVE_OPENMP_4_0
endif
#elif defined(GNUC) // GCC
if ((GNUC > 4) || (GNUC == 4 && (GNUC_MINOR >= 9)))
define WE_HAVE_OPENMP_4_0
endif
#else // Feel free to add your preferred compiler tests here, I take as default that we have OpenMP 4.0 implemented
define WE_HAVE_OPENMP_4_0
#endif
- when I really need a specific stuff from a specific compiler version, I try nevertheless to find alternatives. e.g. in the C11 standard we can declare alignment with _Alignas(alignment) but it was already in built-in GNU function attribute((aligned(alignment))), so I make it available anyway if the user has this compiler. Of course it happens that we have no choice but to use a more modern compiler, but in this case, yes, I feel that it should clearly be documented. Because the compiler cannot know that he is the one responsible, and the error message he throws is not at all understandable by the user who tries to compile :)