Repository navigation
Description
I just attempted to build the library with cpp0x support enabled and I encountered build errors. I have tracked down the issues and have outlined them below.
First, in src/httpserver/webserver.hpp, in the #include section, <set> is conditionally included if CPP0X support is not enabled (https://github.com/etr/libhttpserver/blob/master/src/httpserver/webserver.hpp#L36-40). This causes errors because std::set is used elsewhere throughout the code. This can be fixed by changing:
#ifdef USE_CPP_ZEROX
#include <unordered_set>
#else
#include <set>
#endif
to
#ifdef USE_CPP_ZEROX
#include <unordered_set>
#endif
#include <set>
Next, when declaring the bans and allowances variables on https://github.com/etr/libhttpserver/blob/master/src/httpserver/webserver.hpp#L238-244, std::unordered_set is used if CPP0X support is enabled, but the type that it stores (http::ip_representation) does not have std::hash defined for it. In my opinion, unordered set may not be required here as O(lg n) lookup is probably sufficient for the purposes that it serves, and the time gain that you would get with O(1) lookup with unordered set likely doesn't outweigh the space overhead.
Next, in src/webserver.cpp, there are quite a few instances where std::make_pair is called but the template arguments are provided; they should always be omitted with make_pair, and because C++0x is stricter with type conversions, they fail to compile. The offending lines are https://github.com/etr/libhttpserver/blob/master/src/webserver.cpp#L252, https://github.com/etr/libhttpserver/blob/master/src/webserver.cpp#L1417, and https://github.com/etr/libhttpserver/blob/master/src/webserver.cpp#L1428.
Lastly, for some reason, when running configure with CPP0X enabled, the --std=c++0x flag is missing from somewhere in the generated Makefile. To fix this I had to run the configure command as follows:
CXXFLAGS=--std=c++11 ../libhttpserver/configure --prefix=/usr --enable-cpp0x
I have no clue how the configure script works so I don't know what would have to be modified to make this work out of the box.
Activity
etr commentedon Mar 6, 2014
I partially imagined this since that option was an old abandoned experiment. I will do a cleanup to make it work.
I second you opinion about unordered_set and I add to it that apart the space waste it actually also is not positive from the time point of view - I expect both of the sets to be fairly small and so the cost of hashing could be potentially higher than the effective cost of accessing the set.
m-renaud commentedon Mar 6, 2014
Ahh, if it's abandoned maybe just remove it all together. If there is work on a C++11 version in the future it could just be a separate branch of the project, then the code won't get mixed in together and muddied up with
#ifdefs everywhere.etr commentedon Mar 8, 2014
I am not prioritizing it right now, but I think the best approach would be to isolate the "#ifdef inferno" in a place where it is controllable but to continue to avoid branching. I am always scared by the branching approach because far too often it results in semi-abandoned part of the code or in very difficult alignments.
TASK-016 review: complete remaining minor cleanup pass
TASK-027 review: majors+minors cleanup pass
TASK-050: Tier 6 minor review cleanup (48 items)