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

Source code compatibility with SeqAn3 3.2 and C++20 #13

Closed
jmarshall opened this issue Sep 29, 2022 · 3 comments · Fixed by #14
Closed

Source code compatibility with SeqAn3 3.2 and C++20 #13

jmarshall opened this issue Sep 29, 2022 · 3 comments · Fixed by #14

Comments

@jmarshall
Copy link

jmarshall commented Sep 29, 2022

SeqAn3 3.2.0 was released in June and has dropped support for C++17. Attempting to build unitig-caller against current SeqAn3 (in a Bioconda context; see also bioconda/bioconda-recipes#36884) produced complaints about not using ‑std=c++20.

This was easily fixed by adjusting unitig-caller's CMakeLists.txt, line 3:

set(CMAKE_CXX_STANDARD 20)

However even after fixing this to use C++20, there is a cascade of C++ build errors:

[ 25%] Building CXX object CMakeFiles/unitig_query.dir/src/map_strings.cpp.o
…/x86_64-conda-linux-gnu-c++ -DSEQAN3_HAS_BZIP2=1 -DSEQAN3_HAS_ZLIB=1 -Dunitig_query_EXPORTS … -fvisibility-inlines-hidden -std=c++17 -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe … -DVERSION_INFO=\"1.2.1\" -fconcepts -O3 -DNDEBUG -fPIC -fvisibility=hidden -flto -fno-fat-lto-objects -std=c++20 -pthread -std=gnu++2a -MD -MT CMakeFiles/unitig_query.dir/src/map_strings.cpp.o -MF CMakeFiles/unitig_query.dir/src/map_strings.cpp.o.d -o CMakeFiles/unitig_query.dir/src/map_strings.cpp.o -c …/conda-bld/unitig-caller_1664446063704/work/src/map_strings.cpp
…/conda-bld/unitig-caller_1664446063704/work/src/map_strings.cpp: In function 'void call_strings(const std::vector<std::__cxx11::basic_string<char> >&, const std::vector<std::__cxx11::basic_string<char> >&, const std::vector<std::__cxx11::basic_string<char> >&, const string&, bool, size_t)':
…/conda-bld/unitig-caller_1664446063704/work/src/map_strings.cpp:73:85: error: no match for 'operator|' (operand types are 'std::ranges::transform_view<std::ranges::ref_view<const std::__cxx11::basic_string<char> >, <lambda(auto:56&&)> >' and '<unresolved overloaded function type>')
   73 |       seqan3::dna5_vector query = *unitig_it | seqan3::views::char_to<seqan3::dna5> | seqan3::views::to<std::vector>;
      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                              |                                                       |
      |                                              |                                                       <unresolved overloaded function type>
      |                                              std::ranges::transform_view<std::ranges::ref_view<const std::__cxx11::basic_string<char> >, <lambda(auto:56&&)> >

[…etc…]

The SeqAn3 3.2.0 release notes mention that seqan3::views::to has been substantially changed.

Do you plan to support SeqAn3 3.2.x? I haven't looked at whether the source code changes required would remain workable with earlier versions of SeqAn3…

@johnlees
Copy link
Member

Thanks for this detailed report, I'll try and take a look next week, but I think I might replace seqan with sdsl-lite-3 as we're just using an FM-index, and the latter has a more stable API so would hopefully prevent similar problems in future.

@johnlees
Copy link
Member

johnlees commented Oct 3, 2022

@jmarshall I have made a PR and release which swaps out seqan for sdsl-lite, so this should sort this out. I presume a bioconda PR for the new release will automatically be made shortly -- I can edit and merge when I see it

@jmarshall
Copy link
Author

Thanks; that seems like a fairly sensible approach.

I don't know why the bot didn't notice the new release, but I see you've created a bioconda-recipes PR now anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants