Modifications and cleanup for compilation on OS/X#42
Modifications and cleanup for compilation on OS/X#42waTeim wants to merge 4 commits intoetr:masterfrom
Conversation
Added various include directives in http_response.cpp and http_resource.cpp and modified http_endpoint copy constructor to make it public to satifify clang. Fixed typo/bug in http_request::set_requestor_port. Modified various forwards to match actual declarations. Modified configure to allow build in same directory.
|
Thank you for your help. I discuss here the changes. I am a lot interested in having details especially regarding the points 1 and 3.
|
|
|
Don't worry, I will probably anyway move the pull request manually. I installed a clang instance to test it and see if I can obtain something that actually works fine with both compilers. Regarding 7) yes, I really don't like compilations in the same folder - they usually end up in mixups between code and binaries that more than once have made me cry blood to be solved. Once I move the pull request (or part of it) inside the code, I will add you in the AUTHORS file as a collaborator in the format: FirstName FamilyName emailAddress let me know if for any reason you don't want to have your name or your e-mail address inserted there. |
|
Cool! Name: Jeff Waller Also, just to make it fun, XCode 5 clang is not the latest clang... clang --version
In file included from webserver.cpp:23: ... /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:1505:36: error: calling a private constructor of class 'httpserver::details::http_endpoint' |
|
I made some changes. It seems to compile on both my clang 3.2 installation and clang 3.3 on travis-ci. Let me know if the current trunk works on your clang 5.0. |
|
Hey I like the configure option! Unfortunately, I think OS/X std impl is going to insist that http_endpoint is publicly available it's essentially the same error as before. Which appears to arise from this one statement: Makes sense too because of this: webserver.hpp:232 if instead it was something like or this second option is probably going to force std=c++11 though, which is limiting, but maybe no big deal in 2014. that would probably satisfy the OS/X impl. mv -f .deps/string_utilities.Tpo .deps/string_utilities.Plo |
|
I think they are avoiding somehow the type erasure in new versions - since it compiles with 3.2 and 3.3. |
|
Oops, haha, you're right of course, but then same comments, but for the key not the value. |
|
I tried to block the possibilities for clang to build the partial object. I do not see why clang need to build a temporary copy of the key while inserting - this actually does not happen in linux implementation of LLVM 3.3 that matches your version on Mac OS/X. This make me strongly suspect that Mac implementation is just wrong. It would be long to explain the details of why I need an object and not a pointer as key and it is much more easy to explain why it is not possible to use C++11 costructs since there are guys compiling this library with gcc 4.1. I still think that having a public constructor for http_endpoint is not a good choice and I will not change this because they are not able to build a decent compiler. Said this, let me know if this new version works, otherwise I will add an #ifdef that moves to public the constructor in OS/X environments - at least this shame will be confined. |
I've made modifications mostly for compilation on OS/X to succeed. Found 1 generic bug and one bug that is OS/X specific. The resulting library have been tested by inclusion in a program compiled for both OS/X and Linux. The changes are limited in scope and can be verified readily by inspection. I also made some changes that are not strictly necessary, but suited to my development environment.
More specifically;
clang requires that the copy constructor of http_endpoint be public.
Changed to match declarations and forward declarations; class for class, struct for struct
clang may have further requirements needing fully defined types or there is a scenario not seen before that can give rise to use of a class not fully defined for which include file inclusion fixes the problem
OS/X AF_INET STREAM_SOCKETS do not support SOCK_CLOSEXEC; the system call returns EPROTONOSUPPORT not EINVAL.
Changed a getter to const as it doesn't change the class state and expands the scenarios in which a temporary object need not be created.
Fixed a typo in set_requestor_port.
Changed to allow building in the same directory, modified .gitignore to ignore the resulting targets