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

use wstring as input to stoull to make GCC 10.1.0 happy #429

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ekg
Copy link
Contributor

@ekg ekg commented May 25, 2020

I ran into a change in GCC behavior in 10.1.0.

Apparently, it's not possible to apply std::stoull to a regular std::string, and now std::wstring is required.

For instance

#include <iostream>
#include <string>                                                                                                                                                                               

uint64_t _parse_number(std::string::const_iterator& c, const std::string::const_iterator& end) {
    std::string::const_iterator s = c;
    while (c != end and isdigit(*c)) ++c;
    if (c > s) {
        return std::stoull(std::string(s,c));
    } else {
        return 0;                                                                                                                                                                                  
    }
}

int main(int argc, char** argv) {
    std::string s(argv[1]);
    std::string::const_iterator b = s.begin();
    std::string::const_iterator e = s.end();
    std::cout << _parse_number(b, e) << std::endl;
    return 0;
}

Compiling with GCC 10.1.0 gives this error:

-> % g++ test-stoull.cpp -o test-stoull
test-stoull.cpp: In function ‘uint64_t _parse_number(std::__cxx11::basic_string<char>::const_iterator&, const const_iterator&)’:                                                               test-stoull.cpp:13:33: error: invalid initialization of reference of type ‘const wstring&’ {aka ‘const std::__cxx11::basic_string<wchar_t>&’} from expression of type ‘std::string’ {aka ‘std::__cxx11::basic_string<char>’}
   13 |         return std::stoull(std::string(s,c));
      |                                 ^~~~~~~~~~~
In file included from /home/erikg/.guix-profile/include/c++/string:55,
                 from /home/erikg/.guix-profile/include/c++/bits/locale_classes.h:40,
                 from /home/erikg/.guix-profile/include/c++/bits/ios_base.h:41,
                 from /home/erikg/.guix-profile/include/c++/ios:42,
                 from /home/erikg/.guix-profile/include/c++/ostream:38,
                 from /home/erikg/.guix-profile/include/c++/iostream:39,
                 from test-stoull.cpp:1:
/home/erikg/.guix-profile/include/c++/bits/basic_string.h:6697:25: note: in passing argument 1 of ‘long long unsigned int std::__cxx11::stoull(const wstring&, std::size_t*, int)’
 6697 |   stoull(const wstring& __str, size_t* __idx = 0, int __base = 10)
      |          ~~~~~~~~~~~~~~~^~~~~

I get the same error when trying to compile sdsl-lite in this environment.

Changing the _parse_number code to use std::wstring resolves the issue. It might be less efficient, but I would expect things to work as before without issue. This patch does that within the io code.

I would be happy to hear that someone else can reproduce this. I didn't find much about wstring in the GCC release notes, and my environment is kind of funny (guix install).

@ekg
Copy link
Contributor Author

ekg commented May 27, 2020

Any thoughts on this?

@eloj
Copy link

eloj commented May 27, 2020

This is most likely an issue with your environment/installation. The function should be overloaded so both std::string and std::wstring should work, and in fact, your example code compiles without error on my linux machine.

$ g++ test-stoull.cpp -o test-stoull
$ ./test-stoull hello
0
$ g++ -dumpversion
10.1.0

@ekg
Copy link
Contributor Author

ekg commented May 27, 2020 via email

@pjotrp
Copy link

pjotrp commented May 31, 2020

For future reference: adding -D_GLIBCXX_USE_C99_STDLIB fixes the build:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 657949f..fcedbee 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -44,7 +44,7 @@ else()
 endif()

 if( CMAKE_COMPILER_IS_GNUCXX )
-    append_cxx_compiler_flags("-std=c++11 -Wall -Wextra -DNDEBUG" "GCC" CMAKE_CXX_FLAGS)
+    append_cxx_compiler_flags("-std=c++11 -D_GLIBCXX_USE_C99_STDLIB -Wall -Wextra -DNDEBUG" "GCC" CMAKE_CXX_FLAGS)
     append_cxx_compiler_flags("-O3 -ffast-math -funroll-loops" "GCC" CMAKE_CXX_OPT_FLAGS)
     if ( CODE_COVERAGE )
         append_cxx_compiler_flags("-g -fprofile-arcs -ftest-coverage -lgcov" "GCC" CMAKE_CXX_FLAGS)

related to gnu c++ default behaviour discussed in, for example, http://freebsd.1045724.x6.nabble.com/base-gcc-and-GLIBCXX-USE-C99-td5781697.html.

sdsl-lite built successfuly in a GNU Guix container this way:

penguin2:~/tmp/sdsl-lite$ ~/opt/guix-guile3/bin/guix environment -C guix --ad-hoc cmake libdivsufsort gcc-toolchain@10.1.0
cmake .
make
(...)
[100%] Linking CXX static library libsdsl_static.a
make[2]: Leaving directory '/home/wrk/tmp/sdsl-lite'
[100%] Built target sdsl_static

Not sure why the gcc-toolchain defaults to this older behaviour on GNU Guix.

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 this pull request may close these issues.

3 participants